tbaeder added inline comments.

================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:920
+
+  Condition->dump();
+  if (!this->visit(Condition))
----------------
erichkeane wrote:
> This meant to be here?
Nope! :(


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:921
+  Condition->dump();
+  if (!this->visit(Condition))
+    return false;
----------------
erichkeane wrote:
> Is this condition supposed to be just a 'visit', or should this be using the 
> functor?
The visit is correct here, the functor is just for the true/false expr.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:200
+  using ExprVisitorFunc = std::function<bool(const Expr *)>;
+  bool visitConditional(const AbstractConditionalOperator *E,
+                        ExprVisitorFunc VisitFunc);
----------------
erichkeane wrote:
> I'd probably rather make this a template taking a functor, rather than 
> bringing in the atrocities that come with std::function.
Alright, will look into that


================
Comment at: clang/test/AST/Interp/records.cpp:339
+
+namespace ConditionalInit {
+  struct S { int a; };
----------------
erichkeane wrote:
> Did this not work before?  I don't see what part of teh code changes makes 
> this work?
We call `visitInitializer` also when the return value is constructed in the 
calling function, so the return statement in `getS()` exercises the code added.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141497/new/

https://reviews.llvm.org/D141497

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to