[PATCH] D141497: [clang][Interp] Record initialization via conditional operator

[Timm Bäder via Phabricator via cfe-commits]

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