[cfe-commits] r59884 - in /cfe/trunk: include/clang/AST/Expr.h lib/AST/ExprConstant.cpp lib/Sema/SemaStmt.cpp test/Sema/switch.c
andersca at mac.com
Mon Nov 24 19:53:39 PST 2008
24 nov 2008 kl. 15.18 skrev Daniel Dunbar:
> Hi Anders,
> Some comments,
> On Sat, Nov 22, 2008 at 1:50 PM, Anders Carlsson <andersca at mac.com>
>> + // FIXME: We should come up with a better API for the
>> isEvaluated case.
>> + bool Evaluate(APValue& Result, ASTContext &Ctx, bool
>> *isEvaluated = 0) const;
> I agree with the FIXME. :)
> I personally think that Evaluate should return a simple variant class
> which wraps all the information Evaluate computed about the
> expression. This forces clients to explicitly deal with the variant
> which increases code size (lines) a bit but I think for this case this
> is Good Thing.
Another option would be to pass in the constraints to Evaluate, like
"this must be fully evaluated", "this must be an ICE" etc.
> This name is quite confusing (esp. given the addition of the
> isEvaluatable method which computes something else) and is not, I
> think, what we want to know. For the sole current use case, what we
> really want to know is 'must-be-executed', where false means that
> replacing the expression by the evaluated constant does not change the
> semantics of the program. The current flag is, I believe, just a
> coarse approximation of this. It is safe to replace (0,1) by 1, for
>> /// isEvaluatable - Call Evaluate to see if this expression can be
>> /// folded, but discard the result.
>> bool isEvaluatable(ASTContext &Ctx) const;
> This can be inline? I can't say I'm fond of the name evaluatable
> because I don't know what it means, but in this case I presume it
> "means" Evaluate returns true for the expression.
Yeah - canEvaluate would be better - or just have an Evaluate overload
that wouldn't take an APValue reference.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 2415 bytes
Desc: not available
More information about the cfe-commits