fix Issue 22929 - importC: extern array with unknown length gives bou… #13902
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#13902" |
src/dmd/expressionsem.d
Outdated
| @@ -8472,6 +8472,19 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor | |||
| // OR it in, because it might already be set for C array indexing | |||
| exp.indexIsInBounds |= bounds.contains(getIntRange(exp.e2)); | |||
| } | |||
| else if (auto ve = exp.e1.isVarExp()) | |||
| { | |||
| if (sc.flags & SCOPE.Cfile && t1b.ty == Tsarray) | |||
There was a problem hiding this comment.
Micro-optimization: swap this condition with the one above. Compiling C files occurs less frequent than compiling D files.
Also, I am a bit worried for the effect that all of these C file conditions have on the compilation of normal D code.
There was a problem hiding this comment.
ok.
I work to minimize these changes.
src/dmd/optimize.d
Outdated
| if (!se.var.isReference() && | ||
| !se.var.isImportedSymbol() && | ||
| se.var.isDataseg() && | ||
| se.var.isCsymbol()) |
There was a problem hiding this comment.
In this situation I have to mentally load myself with all of the previous conditions and then at the end I see that it applies only to C symbols. Why not start with this condition(preferably put it in the outer if)? That way, it becomes clear that what follows applies to C land. This should be a general rule.
There was a problem hiding this comment.
I put isCsymbol() at the end because it is slower.
There was a problem hiding this comment.
I realized the isCsymbol() isn't necessary and removed it.
WalterBright
force-pushed
the
fix22929
branch
2 times, most recently
from
2ea9776 to
3f11a41
Compare
|
This is blocking fixing https://issues.dlang.org/show_bug.cgi?id=22935 which is strongly related. |
WalterBright
added
the
Review:Blocking Other Work
…nds errors
This was actually two problems. The first test case is handled by lines optimize.c lines 555-561, the second test case the rest.