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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


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


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


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


================
Comment at: clang/test/AST/Interp/records.cpp:339
+
+namespace ConditionalInit {
+  struct S { int a; };
----------------
Did this not work before?  I don't see what part of teh code changes makes this work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141497