rsmith added inline comments.
================
Comment at: lib/AST/ExprConstant.cpp:4469-4470
{ return StmtVisitorTy::Visit(E->getSubExpr()); }
+ bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E)
+ { return StmtVisitorTy::Visit(E->getSubExpr()); }
bool VisitChooseExpr(const ChooseExpr *E)
----------------
I believe this is unreachable: an `ObjCBoxedExpr` will always have pointer (or
dependent) type; the former will be handled below and the latter should never
be evaluated at all. (We might want a mode to recurse into non-dependent
subexpressions of value-dependent expressions, but that should probably be a
separate visitor.)
================
Comment at: lib/AST/ExprConstant.cpp:5487
+ }
+
+ case EvalInfo::EM_ConstantExpression:
----------------
Add `LLVM_FALLTHROUGH;` here, please.
================
Comment at: lib/AST/ExprConstant.cpp:5492
+ case EvalInfo::EM_OffsetFold:
+ return Success(E);
+ }
----------------
As far as I can see, this (from the pre-existing code) is very wrong: the
evaluation semantics of `ObjCBoxedExpr` are to evaluate the subexpression and
then pass that to a certain Objective-C method to box the result. We can't just
skip evaluating the subexpression! We miscompile this, for instance:
```
@interface NSNumber
+ (id)numberWithInt:(int)n;
@end
int n;
int m = (@(n++), 0);
```
... completely losing the increment of `n` because we incorrectly return
`Success` here without evaluating the subexpression.
Plus returning `Success` here seems like it's never likely to be useful, since
CGExprConstant can't actually emit an `APValue` of this form.
As a minimal fix that would still let us perform this weird evaluation, we
could unconditionally call `EvaluateIgnoredValue` here prior to returning
`Success(E)`.
https://reviews.llvm.org/D32410
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
