Fixes for two important CTFE regressions #37
Conversation
Now that $ is constant-folded correctly during the optimize step, the limited constant-folding in dsymbol can be removed. It had the effect of creating __dollar=0 in zombie cases where constant folding wasn't quite possible. A big mess. Here's a very simple new scheme: __dollar is still initialized for tuples, but for arrays, it is ALWAYS void-initialized. Later, it might be turned into an integer in the optimize step; otherwise, it becomes a runtime value.
The implementation now shares most of the same code as array variable assignment.
|
Maybe it's something with the setup on my machine, but when I dmd with this patch on Linux, I get this error: g++ -m32 -c -Wno-deprecated -Wstrict-aliasing -D__near= -D__pascal= -fno-exceptions -O2 -Iroot -D__I86__=1 -DMARS=1 -DTARGET_LINUX=1 -D_DH interpret.c |
|
That's weird. So g++ thinks this isn't an lvalue (line 2681) ? |
|
Some compilers don't like a cast on the lvalue. |
|
Also, what happened to the "merge help" button? |
|
what's the git pull command for this? Damn merge help button is gone. |
This caused a foreach CTFE regression, and also refused code like int[]a=null; int [] b = a[0..$];
|
I've found that this doesn't fix all cases. For example, this fails with another "__dollar cannot be read at compile time" message. |
|
No, additional parens don't fix the problem on line 2687 of interpret.c, whether I put them around the lvalue or around both the lvalue and the cast. I'm guessing that you're going to have to cast the value on the right rather than the left. Also, that last check in seems to make the compilation of interpret.c fail even more on my machine: g++ -m32 -c -Wno-deprecated -Wstrict-aliasing -D__near= -D__pascal= -fno-exceptions -O2 -Iroot -D__I86__=1 -DMARS=1 -DTARGET_LINUX=1 -D_DH interpret.c I don't know what version of gcc/g++ that you're using, but it may be that mine is more recent and pickier. It claims to be g++ (GCC) 4.5.2 20110127 (prerelease) Or perhaps you just built it on Windows with dmc and dmc isn't as picky about this particular case. In either case, it's failing to build on my machine. |
|
The pull help is linked just above the text entry box for comments: Comment on this pull request (Help) <-- it's not help for text entry, it's help for the whole pull process. |
|
Jim, Walter already answered it. Bizarrely, g++ can't handle casts on lvalues, even when it's a no-op. You can remove the cast entirely. |
Fixes for two important CTFE regressions
|
I would point out that the error that I listed before with the gotos is still there, and not only is it failing on my box, it's now failing on most of the autotesters. So, this merge broke the dmd build. |
|
Things definitely are a mess right now. FreeBSD's unit tests don't pass datetime, and as far as I know never passed. Druntime won't build on Linux due to core.atomic changes. Unit tests on the latest Phobos hang on Windows. Now this (though this should be easily fixable, as g++ cries wolf a lot about branching past initializations). |
|
Hmm. I thought that std.datetime had been passing on FreeBSD. If not, it would be good to know what's broken about it so that I can fix it (assuming that it's not a compiler error). In either case, there are definitely several issues which need to be ironed out before we can really think about doing another release. |
|
Pushed a fix for the goto problem. |
|
jmdavis: see: http://d.puremagic.com/issues/show_bug.cgi?id=5629 |
|
@WalterBright dmd does now build on my box thanks to your goto fix. @braddr Oh yes. I'd forgotten about that. I never saw it in any of my searches because it doesn't mention std.datetime. The code's fine, but the test needs to be fixed. So, it's a problem but far from critical. And it's going to be entertaining to find the appropriate test there, given the variations in behaviour. Bleh. Well, at least it's on my radar again. |
|
It is a problem for me because I cannot run my build/test scripts on FreeBSD. |
|
Why? I wouldn't have thought that whether core.time's unit tests ran would affect anything else. Regardless, I can just disable the test for now if it's causing problems for you and then re-enable it once I've got it figured out. |
|
Can we move this off github onto the mailing list? It has nothing to do with this pull request and is just cluttering things up. |
The first patch entirely removes the hackish, limited constant folding for $ from the initial semantic pass, into the constant folding pass where it belongs. (Incidentally, the comments for __dollar in the code were completely wrong -- it isn't always a constant!)
The second patch allows array reference assignment to members of structs, which in 2.052 would always compile, and occasionally generate correct code . Now it should actually work.