I figured I should get this out into the open, since my holding onto
it isn't very useful. It's essentially a patch replacing the whole of
initialization-list processing sema with a version written from
scratch.
I'm not really asking for review of this, because it has quite a few weaknesses.
1. It rewrites the whole thing from scratch, which is probably not the
best idea, even if it is only a few hundred lines.
2. I'm not really satisfied with the way this patch is written; it has
a lot of code duplication. Some of the logic is a bit tricky, and both
the duplicate loops both within the method and the inablility to reuse
the logic for codegen/analysis are serious issues.
3. It's not complete: I haven't gone over any of the logic required for vectors.
That said, the code does work, and I believe it implements C99 rules
correctly, so it might be useful source of ideas for Steve or whoever
else touches this code.
-Eli
rewritewipdiff.txt (10.9 KB)
Thanks for sending this out.
If you believe it implements C99 correctly, what test cases are you using to verify this?
The test cases I added to test/Sema are useful, but minimal. If you believe this implements C99 rules correctly, I'm sure you've used/developed additional test cases.
Since this is a particularly tricky area, it would be great if you could include the test cases this patch implements correctly.
snaroff
Okay, I've attached some stuff. I think this covers most of the
interesting stuff I tested, although I didn't save everything.
-Eli
checktests.c (1.45 KB)
Eli,
When I integrate your patch, I get a significant number of failures.
Since you said the code works, I was surprised by the scope of the breakage...
Did you ever run these tests?
Curious,
snaroff
[snaroff:tools/clang/test] snarofflocal% grep FAILED LOG
******************** TEST 'CodeGen/global-with-initialiser.c' FAILED!
Hmm, I can't check right now, but I did run the tests, and I'm pretty
sure there weren't that many failures... I'll have to try against a
clean tree to know for sure. Off the top of my head, though, I'm
pretty sure this patch depends on my string patch because it assumes
strings are arrays.
-Eli
Thanks for the reply.
I've started making a series of "small" changes (some of them influenced by your patch).
Given the complexity of initializers, I think it's best to keep everything working as we go.
snaroff
All right; that sounds like a good approach.
-Eli