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

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6dfe55569d88: [clang][Interp] Rework initializers (authored 
by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D156027?vs=548522=551816#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156027

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp
  clang/test/AST/Interp/lambda.cpp
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -795,3 +795,44 @@
 
 };
 #endif
+
+namespace CompositeDefaultArgs {
+  struct Foo {
+int a;
+int b;
+constexpr Foo() : a(12), b(13) {}
+  };
+
+  class Bar {
+  public:
+bool B = false;
+
+constexpr int someFunc(Foo F = Foo()) {
+  this->B = true;
+  return 5;
+}
+  };
+
+  constexpr bool testMe() {
+Bar B;
+B.someFunc();
+return B.B;
+  }
+  static_assert(testMe(), "");
+}
+
+constexpr bool BPand(BoolPair bp) {
+  return bp.first && bp.second;
+}
+static_assert(BPand(BoolPair{true, false}) == false, "");
+
+namespace TemporaryObjectExpr {
+  struct F {
+int a;
+constexpr F() : a(12) {}
+  };
+  constexpr int foo(F f) {
+return 0;
+  }
+  static_assert(foo(F()) == 0, "");
+}
Index: clang/test/AST/Interp/lambda.cpp
===
--- clang/test/AST/Interp/lambda.cpp
+++ clang/test/AST/Interp/lambda.cpp
@@ -103,8 +103,7 @@
 
 return a;
   }
-  /// FIXME: This should work in the new interpreter.
-  static_assert(foo() == 1); // expected-error {{not an integral constant expression}}
+  static_assert(foo() == 1);
 }
 
 namespace StaticInvoker {
@@ -136,10 +135,6 @@
   }
   static_assert(sv4(12) == 12);
 
-
-
-  /// FIXME: This is broken for lambda-unrelated reasons.
-#if 0
   constexpr int sv5(int i) {
 struct F { int a; float f; };
 auto l = [](int m, F f) { return m; };
@@ -147,7 +142,6 @@
 return fp(i, F{12, 14.0});
   }
   static_assert(sv5(12) == 12);
-#endif
 
   constexpr int sv6(int i) {
 struct F { int a;
@@ -162,3 +156,26 @@
   }
   static_assert(sv6(12) == 12);
 }
+
+namespace LambdasAsParams {
+  template
+  constexpr auto call(F f) {
+return f();
+  }
+  static_assert(call([](){ return 1;}) == 1);
+  static_assert(call([](){ return 2;}) == 2);
+
+
+  constexpr unsigned L = call([](){ return 12;});
+  static_assert(L == 12);
+
+
+  constexpr float heh() {
+auto a = []() {
+  return 1.0;
+};
+
+return static_cast(a());
+  }
+  static_assert(heh() == 1.0);
+}
Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -128,7 +128,7 @@
 return PT_Float;
 
   if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
-  T->isFunctionType())
+  T->isFunctionType() || T->isSpecificBuiltinType(BuiltinType::BoundMember))
 return PT_FnPtr;
 
   if (T->isReferenceType() || T->isPointerType())
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -83,6 +83,7 @@
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
   bool VisitMemberExpr(const MemberExpr *E);
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
+  bool VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
   bool VisitStringLiteral(const StringLiteral *E);
@@ -93,7 +94,6 @@
   bool VisitExprWithCleanups(const ExprWithCleanups *E);
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E);
-  bool VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
   bool VisitLambdaExpr(const LambdaExpr *E);
@@ -101,6 +101,7 @@
   bool VisitCXXThrowExpr(const CXXThrowExpr *E);
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E);
   bool VisitCXXNoexceptExpr(const CXXNoexceptExpr *E);
+  bool VisitCXXConstructExpr(const CXXConstructExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
@@ -136,17 +137,21 @@
 }
 llvm_unreachable("not a primitive type");
   }
-
-  /// Evaluates an expression for side effects and discards the result.
-  bool discard(const Expr *E);
-  /// Evaluates an expression and places 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:545-547
+  // TODO(perf): For int and bool types, we can probably just skip this
+  //   since we memset our Block*s to 0 and so we have the desired value
+  //   without this.

tbaeder wrote:
> aaron.ballman wrote:
> > We might be able to get away with this for floats as well (IIRC, all zero 
> > bits is +0 for any IEEE 754 float type, so at least some of the more common 
> > floats could probably do this). Same is true for any pointer type so long 
> > as it's not a member pointer type.
> IIRC doesn't work since floating point is represented using an `Floating`, 
> which uses `APFloat`.
Ah true!


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

https://reviews.llvm.org/D156027

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

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

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:545-547
+  // TODO(perf): For int and bool types, we can probably just skip this
+  //   since we memset our Block*s to 0 and so we have the desired value
+  //   without this.

aaron.ballman wrote:
> We might be able to get away with this for floats as well (IIRC, all zero 
> bits is +0 for any IEEE 754 float type, so at least some of the more common 
> floats could probably do this). Same is true for any pointer type so long as 
> it's not a member pointer type.
IIRC doesn't work since floating point is represented using an `Floating`, 
which uses `APFloat`.


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

https://reviews.llvm.org/D156027

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

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

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

tbaeder updated this revision to Diff 548522.
tbaeder marked an inline comment as done.

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

https://reviews.llvm.org/D156027

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp
  clang/test/AST/Interp/lambda.cpp
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -856,3 +856,44 @@
 
 };
 #endif
+
+namespace CompositeDefaultArgs {
+  struct Foo {
+int a;
+int b;
+constexpr Foo() : a(12), b(13) {}
+  };
+
+  class Bar {
+  public:
+bool B = false;
+
+constexpr int someFunc(Foo F = Foo()) {
+  this->B = true;
+  return 5;
+}
+  };
+
+  constexpr bool testMe() {
+Bar B;
+B.someFunc();
+return B.B;
+  }
+  static_assert(testMe(), "");
+}
+
+constexpr bool BPand(BoolPair bp) {
+  return bp.first && bp.second;
+}
+static_assert(BPand(BoolPair{true, false}) == false, "");
+
+namespace TemporaryObjectExpr {
+  struct F {
+int a;
+constexpr F() : a(12) {}
+  };
+  constexpr int foo(F f) {
+return 0;
+  }
+  static_assert(foo(F()) == 0, "");
+}
Index: clang/test/AST/Interp/lambda.cpp
===
--- clang/test/AST/Interp/lambda.cpp
+++ clang/test/AST/Interp/lambda.cpp
@@ -135,10 +135,6 @@
   }
   static_assert(sv4(12) == 12);
 
-
-
-  /// FIXME: This is broken for lambda-unrelated reasons.
-#if 0
   constexpr int sv5(int i) {
 struct F { int a; float f; };
 auto l = [](int m, F f) { return m; };
@@ -146,7 +142,6 @@
 return fp(i, F{12, 14.0});
   }
   static_assert(sv5(12) == 12);
-#endif
 
   constexpr int sv6(int i) {
 struct F { int a;
@@ -182,3 +177,27 @@
   static_assert(F.a == 33, "");
   static_assert(F.Aplus2() == (33 + 2), "");
 }
+
+namespace LambdasAsParams {
+  template
+  constexpr auto call(F f) {
+return f();
+  }
+  static_assert(call([](){ return 1;}) == 1);
+  static_assert(call([](){ return 2;}) == 2);
+
+
+  constexpr unsigned L = call([](){ return 12;});
+  static_assert(L == 12);
+
+
+  constexpr float heh() {
+auto a = []() {
+  return 1.0;
+};
+
+return static_cast(a());
+  }
+  static_assert(heh() == 1.0);
+}
+
Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -128,7 +128,7 @@
 return PT_Float;
 
   if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
-  T->isFunctionType())
+  T->isFunctionType() || T->isSpecificBuiltinType(BuiltinType::BoundMember))
 return PT_FnPtr;
 
   if (T->isReferenceType() || T->isPointerType())
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -83,6 +83,7 @@
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
   bool VisitMemberExpr(const MemberExpr *E);
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
+  bool VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
   bool VisitStringLiteral(const StringLiteral *E);
@@ -93,7 +94,6 @@
   bool VisitExprWithCleanups(const ExprWithCleanups *E);
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E);
-  bool VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
   bool VisitLambdaExpr(const LambdaExpr *E);
@@ -101,6 +101,7 @@
   bool VisitCXXThrowExpr(const CXXThrowExpr *E);
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E);
   bool VisitCXXNoexceptExpr(const CXXNoexceptExpr *E);
+  bool VisitCXXConstructExpr(const CXXConstructExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
@@ -136,17 +137,21 @@
 }
 llvm_unreachable("not a primitive type");
   }
-
-  /// 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 the result on the stack. If the
+  /// expression is of composite type, a local variable will be created
+  /// and a pointer to said variable will be placed on the stack.
   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
+  /// been created. 

[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:545-547
+  // TODO(perf): For int and bool types, we can probably just skip this
+  //   since we memset our Block*s to 0 and so we have the desired value
+  //   without this.

We might be able to get away with this for floats as well (IIRC, all zero bits 
is +0 for any IEEE 754 float type, so at least some of the more common floats 
could probably do this). Same is true for any pointer type so long as it's not 
a member pointer type.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1515-1519
+  if (std::optional T = classify(E->getType())) {
+return visit(E);
+  } else {
+return this->bail(E);
+  }




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

https://reviews.llvm.org/D156027

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

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

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

tbaeder updated this revision to Diff 545929.
tbaeder added a comment.

merge `visitConditional()` into its only caller, 
`VisitAbstractConditionalOperator()`. This works now sine the initializer and 
non-initializer code paths are the same.


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

https://reviews.llvm.org/D156027

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp
  clang/test/AST/Interp/lambda.cpp
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -856,3 +856,44 @@
 
 };
 #endif
+
+namespace CompositeDefaultArgs {
+  struct Foo {
+int a;
+int b;
+constexpr Foo() : a(12), b(13) {}
+  };
+
+  class Bar {
+  public:
+bool B = false;
+
+constexpr int someFunc(Foo F = Foo()) {
+  this->B = true;
+  return 5;
+}
+  };
+
+  constexpr bool testMe() {
+Bar B;
+B.someFunc();
+return B.B;
+  }
+  static_assert(testMe(), "");
+}
+
+constexpr bool BPand(BoolPair bp) {
+  return bp.first && bp.second;
+}
+static_assert(BPand(BoolPair{true, false}) == false, "");
+
+namespace TemporaryObjectExpr {
+  struct F {
+int a;
+constexpr F() : a(12) {}
+  };
+  constexpr int foo(F f) {
+return 0;
+  }
+  static_assert(foo(F()) == 0, "");
+}
Index: clang/test/AST/Interp/lambda.cpp
===
--- clang/test/AST/Interp/lambda.cpp
+++ clang/test/AST/Interp/lambda.cpp
@@ -135,10 +135,6 @@
   }
   static_assert(sv4(12) == 12);
 
-
-
-  /// FIXME: This is broken for lambda-unrelated reasons.
-#if 0
   constexpr int sv5(int i) {
 struct F { int a; float f; };
 auto l = [](int m, F f) { return m; };
@@ -146,7 +142,6 @@
 return fp(i, F{12, 14.0});
   }
   static_assert(sv5(12) == 12);
-#endif
 
   constexpr int sv6(int i) {
 struct F { int a;
@@ -182,3 +177,27 @@
   static_assert(F.a == 33, "");
   static_assert(F.Aplus2() == (33 + 2), "");
 }
+
+namespace LambdasAsParams {
+  template
+  constexpr auto call(F f) {
+return f();
+  }
+  static_assert(call([](){ return 1;}) == 1);
+  static_assert(call([](){ return 2;}) == 2);
+
+
+  constexpr unsigned L = call([](){ return 12;});
+  static_assert(L == 12);
+
+
+  constexpr float heh() {
+auto a = []() {
+  return 1.0;
+};
+
+return static_cast(a());
+  }
+  static_assert(heh() == 1.0);
+}
+
Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -128,7 +128,7 @@
 return PT_Float;
 
   if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
-  T->isFunctionType())
+  T->isFunctionType() || T->isSpecificBuiltinType(BuiltinType::BoundMember))
 return PT_FnPtr;
 
   if (T->isReferenceType() || T->isPointerType())
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -83,6 +83,7 @@
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
   bool VisitMemberExpr(const MemberExpr *E);
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
+  bool VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
   bool VisitStringLiteral(const StringLiteral *E);
@@ -93,7 +94,6 @@
   bool VisitExprWithCleanups(const ExprWithCleanups *E);
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E);
-  bool VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
   bool VisitLambdaExpr(const LambdaExpr *E);
@@ -101,6 +101,7 @@
   bool VisitCXXThrowExpr(const CXXThrowExpr *E);
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E);
   bool VisitCXXNoexceptExpr(const CXXNoexceptExpr *E);
+  bool VisitCXXConstructExpr(const CXXConstructExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
@@ -136,17 +137,21 @@
 }
 llvm_unreachable("not a primitive type");
   }
-
-  /// 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 the result on the stack. If the
+  /// expression is of composite type, a local variable will be created
+  /// and a pointer to said variable will be placed on the stack.
   bool visit(const Expr *E);
-  /// Compiles an initializer.
+  /// Compiles 

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

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

tbaeder updated this revision to Diff 545928.

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

https://reviews.llvm.org/D156027

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp
  clang/test/AST/Interp/lambda.cpp
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -856,3 +856,44 @@
 
 };
 #endif
+
+namespace CompositeDefaultArgs {
+  struct Foo {
+int a;
+int b;
+constexpr Foo() : a(12), b(13) {}
+  };
+
+  class Bar {
+  public:
+bool B = false;
+
+constexpr int someFunc(Foo F = Foo()) {
+  this->B = true;
+  return 5;
+}
+  };
+
+  constexpr bool testMe() {
+Bar B;
+B.someFunc();
+return B.B;
+  }
+  static_assert(testMe(), "");
+}
+
+constexpr bool BPand(BoolPair bp) {
+  return bp.first && bp.second;
+}
+static_assert(BPand(BoolPair{true, false}) == false, "");
+
+namespace TemporaryObjectExpr {
+  struct F {
+int a;
+constexpr F() : a(12) {}
+  };
+  constexpr int foo(F f) {
+return 0;
+  }
+  static_assert(foo(F()) == 0, "");
+}
Index: clang/test/AST/Interp/lambda.cpp
===
--- clang/test/AST/Interp/lambda.cpp
+++ clang/test/AST/Interp/lambda.cpp
@@ -135,10 +135,6 @@
   }
   static_assert(sv4(12) == 12);
 
-
-
-  /// FIXME: This is broken for lambda-unrelated reasons.
-#if 0
   constexpr int sv5(int i) {
 struct F { int a; float f; };
 auto l = [](int m, F f) { return m; };
@@ -146,7 +142,6 @@
 return fp(i, F{12, 14.0});
   }
   static_assert(sv5(12) == 12);
-#endif
 
   constexpr int sv6(int i) {
 struct F { int a;
@@ -182,3 +177,27 @@
   static_assert(F.a == 33, "");
   static_assert(F.Aplus2() == (33 + 2), "");
 }
+
+namespace LambdasAsParams {
+  template
+  constexpr auto call(F f) {
+return f();
+  }
+  static_assert(call([](){ return 1;}) == 1);
+  static_assert(call([](){ return 2;}) == 2);
+
+
+  constexpr unsigned L = call([](){ return 12;});
+  static_assert(L == 12);
+
+
+  constexpr float heh() {
+auto a = []() {
+  return 1.0;
+};
+
+return static_cast(a());
+  }
+  static_assert(heh() == 1.0);
+}
+
Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -128,7 +128,7 @@
 return PT_Float;
 
   if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
-  T->isFunctionType())
+  T->isFunctionType() || T->isSpecificBuiltinType(BuiltinType::BoundMember))
 return PT_FnPtr;
 
   if (T->isReferenceType() || T->isPointerType())
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -83,6 +83,7 @@
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
   bool VisitMemberExpr(const MemberExpr *E);
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
+  bool VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
   bool VisitStringLiteral(const StringLiteral *E);
@@ -93,7 +94,6 @@
   bool VisitExprWithCleanups(const ExprWithCleanups *E);
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E);
-  bool VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
   bool VisitLambdaExpr(const LambdaExpr *E);
@@ -101,6 +101,7 @@
   bool VisitCXXThrowExpr(const CXXThrowExpr *E);
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E);
   bool VisitCXXNoexceptExpr(const CXXNoexceptExpr *E);
+  bool VisitCXXConstructExpr(const CXXConstructExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
@@ -136,17 +137,21 @@
 }
 llvm_unreachable("not a primitive type");
   }
-
-  /// 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 the result on the stack. If the
+  /// expression is of composite type, a local variable will be created
+  /// and a pointer to said variable will be placed on the stack.
   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
+  /// been created. visitInitializer() then relies on a pointer to 

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

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

tbaeder updated this revision to Diff 545926.
tbaeder added a comment.

Add `::delegate(const Expr *E)` and squash previous commits into this one.


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

https://reviews.llvm.org/D156027

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp
  clang/test/AST/Interp/lambda.cpp
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -856,3 +856,51 @@
 
 };
 #endif
+
+namespace CompositeDefaultArgs {
+  struct Foo {
+int a;
+int b;
+constexpr Foo() : a(12), b(13) {}
+  };
+
+  class Bar {
+  public:
+bool B = false;
+
+constexpr int someFunc(Foo F = Foo()) {
+  this->B = true;
+  return 5;
+}
+  };
+
+  constexpr bool testMe() {
+Bar B;
+B.someFunc();
+return B.B;
+  }
+  static_assert(testMe(), "");
+}
+
+constexpr bool BPand(BoolPair bp) {
+  return bp.first && bp.second;
+}
+static_assert(BPand(BoolPair{true, false}) == false, "");
+
+namespace TemporaryObjectExpr {
+  struct F {
+int a;
+constexpr F() : a(12) {}
+  };
+  constexpr int foo(F f) {
+return 0;
+  }
+  static_assert(foo(F()) == 0, "");
+}
+
+#if 0
+constexpr bool BPand(BoolPair bp) {
+  return bp.first && bp.second;
+}
+static_assert(BPand(BoolPair{true, false}) == false, "");
+#endif
Index: clang/test/AST/Interp/lambda.cpp
===
--- clang/test/AST/Interp/lambda.cpp
+++ clang/test/AST/Interp/lambda.cpp
@@ -135,10 +135,6 @@
   }
   static_assert(sv4(12) == 12);
 
-
-
-  /// FIXME: This is broken for lambda-unrelated reasons.
-#if 0
   constexpr int sv5(int i) {
 struct F { int a; float f; };
 auto l = [](int m, F f) { return m; };
@@ -146,7 +142,6 @@
 return fp(i, F{12, 14.0});
   }
   static_assert(sv5(12) == 12);
-#endif
 
   constexpr int sv6(int i) {
 struct F { int a;
@@ -182,3 +177,27 @@
   static_assert(F.a == 33, "");
   static_assert(F.Aplus2() == (33 + 2), "");
 }
+
+namespace LambdasAsParams {
+  template
+  constexpr auto call(F f) {
+return f();
+  }
+  static_assert(call([](){ return 1;}) == 1);
+  static_assert(call([](){ return 2;}) == 2);
+
+
+  constexpr unsigned L = call([](){ return 12;});
+  static_assert(L == 12);
+
+
+  constexpr float heh() {
+auto a = []() {
+  return 1.0;
+};
+
+return static_cast(a());
+  }
+  static_assert(heh() == 1.0);
+}
+
Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -128,7 +128,7 @@
 return PT_Float;
 
   if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
-  T->isFunctionType())
+  T->isFunctionType() || T->isSpecificBuiltinType(BuiltinType::BoundMember))
 return PT_FnPtr;
 
   if (T->isReferenceType() || T->isPointerType())
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -83,6 +83,7 @@
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
   bool VisitMemberExpr(const MemberExpr *E);
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
+  bool VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
   bool VisitStringLiteral(const StringLiteral *E);
@@ -93,7 +94,6 @@
   bool VisitExprWithCleanups(const ExprWithCleanups *E);
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E);
-  bool VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
   bool VisitLambdaExpr(const LambdaExpr *E);
@@ -101,6 +101,7 @@
   bool VisitCXXThrowExpr(const CXXThrowExpr *E);
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E);
   bool VisitCXXNoexceptExpr(const CXXNoexceptExpr *E);
+  bool VisitCXXConstructExpr(const CXXConstructExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
@@ -136,17 +137,21 @@
 }
 llvm_unreachable("not a primitive type");
   }
-
-  /// 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 the result on the stack. If the
+  /// expression is of composite type, a local variable will be created
+  /// and a pointer to said variable will be placed on the stack.
   bool visit(const 

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D156027#4544346 , @tbaeder wrote:

> Do you want me to squash the patches I abandoned into this one?

I think that could help us to see the bigger picture; I'm assuming that only 
adds a bit of code rather than three full patches' worth of code.




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:240
+return Initializing ? this->visitInitializer(SubExpr)
+: this->visit(SubExpr);
 

tbaeder wrote:
> aaron.ballman wrote:
> > 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.
> Whenever you just pass on the visit, essentially ignoring the current node. 
> Another good example is `ConstantExpr`s, where we don't do anything for the 
> node and then just pass move on to the `SubExpr`.
Ah, I see, so it's basically when you want to delegate the visit to a child 
node? If so, perhaps that's a reasonable name (`delegateVisit()`)?



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2090
+  assert(Initializing);
+  if (!isa(E)) {
+if (!this->emitDupPtr(E))

tbaeder wrote:
> aaron.ballman wrote:
> > And if it is a member call expression?
> For member calls, the layout at this point is:
> 
> ```
> ThisPtr
> RVOPtr
> RVOPtr
> ```
> (top of the stack is where we're at). The dup here is supposed to dup the RVO 
> ptr, but we can't do that for member calls because the top is currently the 
> instance pointer. Tha'ts why `VisitCXXMemberCallExpr()` handles this 
> separately.
Thank you for the explanation!



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:
> aaron.ballman wrote:
> > 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?
> We have this line in `records.cpp`:
> 
> ```
>   constexpr int value = (s.*foo)();
> ```
> and its type is:
> 
> ```
> BuiltinType 0x6211fbe0 ''
> ```
> 
> it just so happens that we _now_ call `classify()` on this earlier in the 
> code path while before we didn't do that, so we need to know that the bound 
> member function type is now some composite type.
Ah, thank you! That makes sense.


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

https://reviews.llvm.org/D156027

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

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

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

tbaeder added a comment.

Do you want me to squash the patches I abandoned into this one?


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

https://reviews.llvm.org/D156027

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

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

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

tbaeder added inline comments.



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;

aaron.ballman wrote:
> 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?
We have this line in `records.cpp`:

```
  constexpr int value = (s.*foo)();
```
and its type is:

```
BuiltinType 0x6211fbe0 ''
```

it just so happens that we _now_ call `classify()` on this earlier in the code 
path while before we didn't do that, so we need to know that the bound member 
function type is now some composite type.


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

https://reviews.llvm.org/D156027

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

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

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:240
+return Initializing ? this->visitInitializer(SubExpr)
+: this->visit(SubExpr);
 

aaron.ballman wrote:
> 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.
Whenever you just pass on the visit, essentially ignoring the current node. 
Another good example is `ConstantExpr`s, where we don't do anything for the 
node and then just pass move on to the `SubExpr`.


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

https://reviews.llvm.org/D156027

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

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

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

tbaeder updated this revision to Diff 545336.
tbaeder marked 3 inline comments as done.

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

https://reviews.llvm.org/D156027

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp

Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -128,7 +128,7 @@
 return PT_Float;
 
   if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
-  T->isFunctionType())
+  T->isFunctionType() || T->isSpecificBuiltinType(BuiltinType::BoundMember))
 return PT_FnPtr;
 
   if (T->isReferenceType() || T->isPointerType())
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -83,6 +83,7 @@
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
   bool VisitMemberExpr(const MemberExpr *E);
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
+  bool VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
   bool VisitStringLiteral(const StringLiteral *E);
@@ -93,7 +94,6 @@
   bool VisitExprWithCleanups(const ExprWithCleanups *E);
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E);
-  bool VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
   bool VisitLambdaExpr(const LambdaExpr *E);
@@ -137,17 +137,18 @@
 }
 llvm_unreachable("not a primitive type");
   }
-
-  /// 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 the result on the stack. If the
+  /// expression is of composite type, a local variable will be created
+  /// and a pointer to said variable will be placed on the stack.
   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
+  /// been created. visitInitializer() then relies on a pointer to this
+  /// variable being on top of the stack.
   bool visitInitializer(const Expr *E);
-  /// Compiles an array initializer.
-  bool visitArrayInitializer(const Expr *Initializer);
-  /// Compiles a record initializer.
-  bool visitRecordInitializer(const Expr *Initializer);
+  /// Evaluates an expression for side effects and discards the result.
+  bool discard(const Expr *E);
+
   /// Creates and initializes a variable from the given decl.
   bool visitVarDecl(const VarDecl *VD);
 
@@ -285,6 +286,10 @@
 
   /// Flag indicating if return value is to be discarded.
   bool DiscardResult = false;
+
+  /// Flag inidicating if we're initializing an already created
+  /// variable. This is set in visitInitializer().
+  bool Initializing = false;
 };
 
 extern template class ByteCodeExprGen;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -43,18 +43,25 @@
 template  class OptionScope final {
 public:
   /// Root constructor, compiling or discarding primitives.
-  OptionScope(ByteCodeExprGen *Ctx, bool NewDiscardResult)
-  : Ctx(Ctx), OldDiscardResult(Ctx->DiscardResult) {
+  OptionScope(ByteCodeExprGen *Ctx, bool NewDiscardResult,
+  bool NewInitializing)
+  : Ctx(Ctx), OldDiscardResult(Ctx->DiscardResult),
+OldInitializing(Ctx->Initializing) {
 Ctx->DiscardResult = NewDiscardResult;
+Ctx->Initializing = NewInitializing;
   }
 
-  ~OptionScope() { Ctx->DiscardResult = OldDiscardResult; }
+  ~OptionScope() {
+Ctx->DiscardResult = OldDiscardResult;
+Ctx->Initializing = OldInitializing;
+  }
 
 private:
   /// Parent context.
   ByteCodeExprGen *Ctx;
   /// Old discard flag to restore.
   bool OldDiscardResult;
+  bool OldInitializing;
 };
 
 } // namespace interp
@@ -229,7 +236,8 @@
   case CK_BitCast:
 if (DiscardResult)
   return this->discard(SubExpr);
-return this->visit(SubExpr);
+return Initializing ? this->visitInitializer(SubExpr)
+: this->visit(SubExpr);
 
   case CK_IntegralToBoolean:
   case CK_IntegralCast: {
@@ -326,7 +334,8 @@
   return this->discard(RHS);
 
 // Otherwise, visit RHS and optionally discard its value.
-return Discard(this->visit(RHS));
+return 

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

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

tbaeder marked an inline comment as done.
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2090
+  assert(Initializing);
+  if (!isa(E)) {
+if (!this->emitDupPtr(E))

aaron.ballman wrote:
> And if it is a member call expression?
For member calls, the layout at this point is:

```
ThisPtr
RVOPtr
RVOPtr
```
(top of the stack is where we're at). The dup here is supposed to dup the RVO 
ptr, but we can't do that for member calls because the top is currently the 
instance pointer. Tha'ts why `VisitCXXMemberCallExpr()` handles this separately.


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

https://reviews.llvm.org/D156027

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

[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(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

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

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

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:240
+return Initializing ? this->visitInitializer(SubExpr)
+: this->visit(SubExpr);
 

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.


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

https://reviews.llvm.org/D156027

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

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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D156027#4534090 , @tbaeder wrote:

> Note that this patch also supercedes:
>
> 1. https://reviews.llvm.org/D153616
> 2. https://reviews.llvm.org/D153653
> 3. https://reviews.llvm.org/D147591
>
> since it handles those cases more generally.

Can you abandon the revisions you are no longer pursuing? Thanks!


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

https://reviews.llvm.org/D156027

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

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

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

tbaeder added a comment.

Note that this patch also supercedes:

1. https://reviews.llvm.org/D153616
2. https://reviews.llvm.org/D153653
3. https://reviews.llvm.org/D147591

since it handles those cases more generally.




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;

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.


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

https://reviews.llvm.org/D156027

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

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

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

tbaeder updated this revision to Diff 543255.

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

https://reviews.llvm.org/D156027

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp

Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -128,7 +128,7 @@
 return PT_Float;
 
   if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
-  T->isFunctionType())
+  T->isFunctionType() || T->isSpecificBuiltinType(BuiltinType::BoundMember))
 return PT_FnPtr;
 
   if (T->isReferenceType() || T->isPointerType())
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -84,6 +84,7 @@
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
   bool VisitMemberExpr(const MemberExpr *E);
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
+  bool VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
   bool VisitStringLiteral(const StringLiteral *E);
@@ -94,7 +95,6 @@
   bool VisitExprWithCleanups(const ExprWithCleanups *E);
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E);
-  bool VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
   bool VisitLambdaExpr(const LambdaExpr *E);
@@ -139,17 +139,18 @@
 }
 llvm_unreachable("not a primitive type");
   }
-
-  /// 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
+  /// and a pointer to said variable will be placed on the stack.
   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
+  /// been created. visitInitializer() then relies on a pointer to this
+  /// variable being on top of the stack.
   bool visitInitializer(const Expr *E);
-  /// Compiles an array initializer.
-  bool visitArrayInitializer(const Expr *Initializer);
-  /// Compiles a record initializer.
-  bool visitRecordInitializer(const Expr *Initializer);
+  /// Evaluates an expression for side effects and discards the result.
+  bool discard(const Expr *E);
+
   /// Creates and initializes a variable from the given decl.
   bool visitVarDecl(const VarDecl *VD);
   /// Visit an APValue.
@@ -295,6 +296,10 @@
 
   /// Flag indicating if return value is to be discarded.
   bool DiscardResult = false;
+
+  /// Flag inidicating if we're initializing an already created
+  /// variable. This is set in visitInitializer().
+  bool Initializing = false;
 };
 
 extern template class ByteCodeExprGen;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -43,18 +43,25 @@
 template  class OptionScope final {
 public:
   /// Root constructor, compiling or discarding primitives.
-  OptionScope(ByteCodeExprGen *Ctx, bool NewDiscardResult)
-  : Ctx(Ctx), OldDiscardResult(Ctx->DiscardResult) {
+  OptionScope(ByteCodeExprGen *Ctx, bool NewDiscardResult,
+  bool NewInitializing)
+  : Ctx(Ctx), OldDiscardResult(Ctx->DiscardResult),
+OldInitializing(Ctx->Initializing) {
 Ctx->DiscardResult = NewDiscardResult;
+Ctx->Initializing = NewInitializing;
   }
 
-  ~OptionScope() { Ctx->DiscardResult = OldDiscardResult; }
+  ~OptionScope() {
+Ctx->DiscardResult = OldDiscardResult;
+Ctx->Initializing = OldInitializing;
+  }
 
 private:
   /// Parent context.
   ByteCodeExprGen *Ctx;
   /// Old discard flag to restore.
   bool OldDiscardResult;
+  bool OldInitializing;
 };
 
 } // namespace interp
@@ -229,7 +236,8 @@
   case CK_BitCast:
 if (DiscardResult)
   return this->discard(SubExpr);
-return this->visit(SubExpr);
+return Initializing ? this->visitInitializer(SubExpr)
+: this->visit(SubExpr);
 
   case CK_IntegralToBoolean:
   case CK_IntegralCast: {
@@ -326,7 +334,8 @@
   return this->discard(RHS);
 
 // Otherwise, visit RHS and optionally discard its value.
-return Discard(this->visit(RHS));
+return Discard(Initializing ? 

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

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

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Before this patch, we had `visitRecordInitializer()` and 
`visitArrayInitializer()`, which were different from the regular `visit()` in 
that they expected a pointer on the top of the stack, which they initialized. 
For example, `visitArrayInitializer` handled `InitListExprs` by looping over 
the members and initializing the elements of that pointer.

However, this had a few corner cases and problems. For example, in 
`visitLambdaExpr()` (a lambda is always of record type), it was not clear 
whether we should always create a new local variable to save the lambda to, or 
not. This is why https://reviews.llvm.org/D153616 changed things around.

There is also a long-standing problem with fucnction calls, because we always 
visit the call arguments like:

  for (const auto *Arg : CtorExpr->arguments()) {
if (!this->visit(Arg))
  return false;
  }

but if the argument returns a composite type, where does the storage for it 
come from? Before this patch, it sometimes works and sometimes doesn't.

This patch changes the visiting functions to:

1. `visit()`: Always leaves a new value on the stack. If the expression can be 
mapped to a primitive type, it's just visited and the value is put on the 
stack. If it's of composite type, this function will create a local variable 
for the expression value and call `visitInitializer()`. The pointer to the 
local variable will stay on the stack.
2. `visitInitializer()`: Visits the given expression, assuming there is a 
pointer on top of the stack that will be initialized by it.
3. `discard()`: Visit the expression for side-effects, but don't leave a value 
on the stack.

It also adds an additional `Initializing` flag to differentiate between the 
initializing and non-initializing case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156027

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp

Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -128,7 +128,7 @@
 return PT_Float;
 
   if (T->isFunctionPointerType() || T->isFunctionReferenceType() ||
-  T->isFunctionType())
+  T->isFunctionType() || T->isSpecificBuiltinType(BuiltinType::BoundMember))
 return PT_FnPtr;
 
   if (T->isReferenceType() || T->isPointerType())
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -84,6 +84,7 @@
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
   bool VisitMemberExpr(const MemberExpr *E);
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
+  bool VisitArrayInitLoopExpr(const ArrayInitLoopExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
   bool VisitStringLiteral(const StringLiteral *E);
@@ -94,7 +95,6 @@
   bool VisitExprWithCleanups(const ExprWithCleanups *E);
   bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E);
   bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E);
-  bool VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E);
   bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   bool VisitTypeTraitExpr(const TypeTraitExpr *E);
   bool VisitLambdaExpr(const LambdaExpr *E);
@@ -139,17 +139,18 @@
 }
 llvm_unreachable("not a primitive type");
   }
-
-  /// 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
+  /// and a pointer to said variable will be placed on the stack.
   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
+  /// been created. visitInitializer() then relies on a pointer to this
+  /// variable being on top of the stack.
   bool visitInitializer(const Expr *E);
-  /// Compiles an array initializer.
-  bool visitArrayInitializer(const Expr *Initializer);
-  /// Compiles a record initializer.
-  bool visitRecordInitializer(const Expr *Initializer);
+  /// Evaluates an expression for side effects and discards the result.
+  bool discard(const Expr *E);
+
   /// Creates and initializes a variable from the given decl.
   bool visitVarDecl(const VarDecl *VD);
   ///