[cfe-commits] r59196 - in /cfe/trunk: lib/AST/Expr.cpp lib/AST/ExprConstant.cpp test/Sema/const-eval.c
daniel at zuster.org
Wed Nov 12 20:26:11 PST 2008
Hmm, perhaps I did not do a good job explaining. However, I don't
understand why you reverted... can you explain what you think is wrong
with this approach? I have no problem doing it differently, I was just
trying to address regressions I was seeing...
The problem I was trying to solve was this:
>> - Evaluation of , operator used bogus assumption that LHS could be
>> evaluated as an integral expression even though its type is
This causes assertions in other parts of the code because it tries to
do silly things like get the bit width of void.
On Wed, Nov 12, 2008 at 5:19 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> Woah, this looks really strange.
Not sure why, the code is basically the same it just avoids the
traversal of the LHS in isICE. As I mentioned, this is overly
permissive, but it isn't clear how to resolve this in isICE.
> One, you're committing a change to isICE without a test, which is
> definitely a bad idea. What are you trying to accomplish there?
There was a test case added to const-eval.c.
> Second, this is somewhat tricky territory with tryEvaluate: we can
> definitively calculate the value of a comma operator without it
> actually being constant. I'm not sure how we want to deal with this
> case, though: I think a lot of the code using tryEvaluate is ignoring
> the possibility of side-effects.
The change wasn't about addressing side effects... the code for
tryEvaluate was very similar in effect (at least, that was my intent)
to what was there before. I will investigate why it was failing on the
test you added, however, note that now clang is asserting again on the
test I added. :)
I'm happy if you want to fix in a different fashion...
More information about the cfe-commits