[PATCH] D38483: [ExprConstant] Allow constexpr ctor to modify non static data members in body

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/AST/ExprConstant.cpp:576
 
+    typedef std::pair<APValue::LValueBase, unsigned> EvaluatingObject;
+
----------------
Please add a comment explaining what the two fields mean.


================
Comment at: lib/AST/ExprConstant.cpp:588
+          : EI(EI), Object(Object) {
+        DidInsert = EI.EvaluatingConstructors.insert(Object).second;
+      }
----------------
Can the `DidInsert == false` case actually happen?


================
Comment at: lib/AST/ExprConstant.cpp:596
+    bool isEvaluatingConstructor(APValue::LValueBase Decl, unsigned CallIndex) {
+      return Decl == EvaluatingDecl ||
+        EvaluatingConstructors.count(EvaluatingObject(Decl, CallIndex));
----------------
Hmm, should we check the `CallIndex` is 0 in this case? I think we technically don't need to, but only because we happen to only evaluate the initializer of local variables before the enclosing function has its body attached.

Perhaps we could insert `EvaluatingDecl` into the `EvaluatingConstructors` list and simplify this check to just the `DenseSet` lookup?


https://reviews.llvm.org/D38483