rsmith added inline comments.
================
Comment at: lib/AST/APValue.cpp:27
+ APValue::LValueBase Base;
+ bool IsOnePastTheEnd;
CharUnits Offset;
----------------
Move this to the end so it can share space with `IsNullPtr`.
================
Comment at: lib/AST/ExprConstant.cpp:589-598
+ /// Keep track of the version of MTEs that are used by CXXDefaultArgExpr.
+ /// The version number is updated every time VisitCXXDefaultArgExpr is
+ /// called.
+ unsigned getDefaultArgNum() const { return CurDefaultArgNum; }
+ void setDefaultArgNum() {
+ assert(CurDefaultArgNum == 0 && "CurDefaultArgNum hasn't been cleared");
+ CurDefaultArgNum = ++NextDefaultArgNum;
----------------
This seems extremely similar to the purpose of the existing `CallIndex`
parameter. Have you thought about ways the two might be unified?
If there's no reasonable path to unifying the two (which I suspect there may
not be), it would make more sense to me to store a current version number on
the `CallStackFrame` rather than globally in the `EvalInfo`, and to restart the
numbering in each new call frame. That also emphasizes something else: this
version number should be kept with the `CallIndex`. (We could achieve that by
either moving the `CallIndex` into the `LValueBase` or moving the `Version` out
of it.)
================
Comment at: lib/AST/ExprConstant.cpp:597
+ }
+ void clearDefaultArgNum() { CurDefaultArgNum = 0; }
+ unsigned CurDefaultArgNum = 0, NextDefaultArgNum = 0;
----------------
This is wrong: these scopes can nest, so you can't just reset the number to 0
when you're done. You should restore the prior number when you're done here, to
divide up evaluation into CXXDefaultArgExpr scopes. (Technically, I think it
would also be correct to leave the number alone when you leave one of these
scopes, but only because a scope for a particular parameter's default argument
can't nest within another scope for the same default argument expression -- and
even that might not be true in the presence of template instantiation.)
================
Comment at: lib/AST/ExprConstant.cpp:4595
+ }
bool VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *E) {
// The initializer may not have been parsed yet, or might be erroneous.
----------------
We need the same handling for `CXXDefaultInitExpr`, too.
https://reviews.llvm.org/D42776
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
