reboot fix Issue 23058 - importC: cannot take address inside multi-di… #14094
Conversation
|
Thanks for your pull request, @WalterBright! 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#14094" |
…mensional array at compile time
|
|
||
| void test1() | ||
| { | ||
| int *p1 = &arr[4][2]; |
There was a problem hiding this comment.
This code is technically correct code from the perspective of the C specification. Is it ok to turn it into an error?
There was a problem hiding this comment.
It's length+1, which is not technically correct as I recall.
There was a problem hiding this comment.
I compiled this code with gcc and there's no error. Also, the actual code you are introducing treats C and D code the same.
There was a problem hiding this comment.
You're right that gcc doesn't give an error for:
int a[10];
int* f() { return &a[100]; }
but clang does with -Wpedantic -std=c11
The C11 6.5.6-8 spec lists such constructs as "undefined behavior". I'm good with diagnosing an error at compile time.
There was a problem hiding this comment.
There's no reason it should be different for D.
| @@ -57,7 +57,7 @@ fail_compilation/fail13902.d(88): Error: Using the result of a comma expression | |||
| fail_compilation/fail13902.d(75): Error: returning `& x` escapes a reference to parameter `x` | |||
| fail_compilation/fail13902.d(76): Error: returning `&s1.v` escapes a reference to parameter `s1` | |||
| fail_compilation/fail13902.d(81): Error: returning `& sa1` escapes a reference to parameter `sa1` | |||
| fail_compilation/fail13902.d(82): Error: returning `&sa2[0][0]` escapes a reference to parameter `sa2` | |||
| fail_compilation/fail13902.d(82): Error: returning `& sa2` escapes a reference to parameter `sa2` | |||
There was a problem hiding this comment.
off topic: why is & formatted sometimes with a trailing space and sometimes not ?
There was a problem hiding this comment.
The parser generates an AddrExp for &x, which gets printed as &x as well. Semantic analysis turn an AddrExp into a SymOffExp with offset 0, which gets printed as & x. (See dmd.hdrgen.d for the relevant source code)
There was a problem hiding this comment.
shouldn't this be considered as an issue ? What's the point ?
There was a problem hiding this comment.
I think printing a SymOff expression in user-facing error messages is a bug in itself, since when the offset isn't 0, it prints (& ptr+byteoffset) which doesn't account for pointer arithmetic.
There was a problem hiding this comment.
I don't know why the space was added, but from a compiler developer perspective, it is useful to distinguish an AddrExp from a SymOff expression.
…mensional array at compile time (dlang#14094)
…mensional array at compile time (dlang#14094)
|
This PR introduced weird codegen for D array indexing. Before this, frontend passed to the code generator: With this change, we now see: |
| Expression ex = new AddrExp(ae1.loc, ae1); // &a[i] | ||
| ex.type = ae1.type.pointerTo(); | ||
|
|
||
| Expression add = new AddExp(ae.loc, ex, new IntegerExp(ae.loc, offset, e.type)); |
There was a problem hiding this comment.
I assume this is the problem - the integer expression is given a pointer type.
…mensional array at compile time
I was forced to abandon #14044 as it capsized and sank. I wish git had an undo. Sure would be nice.