[PATCH] D156027: [clang][Interp] Rework how initializers work

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:240
+    return Initializing ? this->visitInitializer(SubExpr)
+                        : this->visit(SubExpr);
 
----------------
tbaeder wrote:
> This pattern shows up a few times, so it might make sense to add a function that simply passes on all the flags and doesn't do anything else. I'm just not sure what to call it.
It's not clear to me when you should use this pattern to begin with.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2090
+      assert(Initializing);
+      if (!isa<CXXMemberCallExpr>(E)) {
+        if (!this->emitDupPtr(E))
----------------
And if it is a member call expression?


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:142
   }
-
-  /// Evaluates an expression for side effects and discards the result.
-  bool discard(const Expr *E);
-  /// Evaluates an expression and places result on stack.
+  /// Evaluates an expression and places result on stack. If the
+  /// expression is of composite type, a local variable will be created
----------------



================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:146
   bool visit(const Expr *E);
-  /// Compiles an initializer.
+  /// Compiles an initializer This is like visit() but it will never
+  /// create a variable and instead rely on a variable already having
----------------



================
Comment at: clang/lib/AST/Interp/Context.cpp:131
   if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
-      T->isFunctionType())
+      T->isFunctionType() || T->isSpecificBuiltinType(BuiltinType::BoundMember))
     return PT_FnPtr;
----------------
tbaeder wrote:
> I've removed this change in https://reviews.llvm.org/D144164 since it didn't  seem necessary, but it //is// necessary after applying this patch.
Which test case exercises this bit?


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

https://reviews.llvm.org/D156027