fix issue 23565 - Allow the use of $ as index on .ptr expressions
#14716
Conversation
|
Thanks for your pull request and interest in making D better, @SixthDot! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#14716" |
We can use `.ptr` in slice and indexes expressions to bypass bounds verification but then we loose the opportunity to use `$` in the expression that give the subscripts, even if an array and hence a `.length` is possible. This commit fix the problem as long as the `.ptr` LHS is a (optionally dotted) variable The implementation is based on a front-end lowering as the existing system was not designed for this case (it finalize the substitution in the mid-end function `resolveLengthVar()`)
thewilsonator
added
Review:Needs Changelog
|
@thewilsonator, a changelog entry is there |
|
How can this work ? I am sure a more complicated example would fail. |
| if (auto tp = ce.type.isTypePointer()) | ||
| { | ||
| const cty = ce.e1.type.ty; | ||
| const cop = ce.e1.op; |
There was a problem hiding this comment.
why cop and cty ?
because of the const?
generally we don't use Hungarian notation.
| a.ptr[0 .. $] = [1,2]; | ||
| assert(a == [1,2]); | ||
|
|
||
| s.a.ptr[0 .. $] = [1,2]; |
There was a problem hiding this comment.
You use the same arrays with the same lengths.
Use another array with different length.
Then use a struct where opDollar is overloaded.
The current tests can easily hide unwanted interaction.
No. Check a few lines before. The scope |
| @@ -13790,3 +13796,23 @@ Expression toBoolean(Expression exp, Scope* sc) | |||
| return e; | |||
| } | |||
| } | |||
|
|
|||
| /* Push the necessary scope to solve `$` in `e1.ptr[$]` or `e1.ptr[0 .. $]` */ | |||
|
This appears to have the potential to break existing code. Consider this currently-legal example: auto a = [1,2,3,4,5,6,0];
auto b = [0,1,2,3,4,5,6,7];
writeln(b[a.ptr[$ - 2]]);This is legal code today, and it prints 0. With this change, will it print 0 or 6? Furthermore, why is .ptr treated specially here? Why does this not extend to other properties, or even UFCS'd functions? And what of array-like structs that have both a .ptr and opDollar? do they get to take advantage of this feature? |
|
I don't understand the point of this PR. I know what it does, but I don't know why. It shouldn't be common to do this, and the "to bypass bounds checks" is an interesting phrase to use here. Indexing into a pointer that came from a slice/dynamic array is, at the very least, weird. |
If nothing else it makes it more plastic to use .ptr in existing code (primarily for guaranteeing there being no bounds check locally) |
|
As per @Herringway comment, this is actually a breaking change in nested array expressions. |
ghost
closed this
We can use
.ptrin slice and indexes expressions to bypass bounds verification but then we loose the opportunity to use$in the expression that give the subscripts, even if an array is present and hence a.lengthis possible.This commit fix the problem as long as the
.ptrLHS is a (optionally dotted) variableThe implementation is based on a front-end lowering as the existing system was not designed for this case (it finalizes the substitution in the mid-end function
resolveLengthVar())