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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D38483: [... Erik Pilkington via Phabricator via cfe-commits
    • [PATCH] D384... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D384... Erik Pilkington via Phabricator via cfe-commits
    • [PATCH] D384... Erik Pilkington via Phabricator via cfe-commits
    • [PATCH] D384... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D384... Phabricator via Phabricator via cfe-commits

Reply via email to