[PATCH] D150040: [clang][Interp] Call invalid destructors

[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 rG10020791b18f: [clang][Interp] Call invalid destructors 
(authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D150040?vs=520092=544211#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150040

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.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
@@ -566,6 +566,25 @@
 return i;
   }
   static_assert(test() == 1);
+
+  struct S {
+constexpr S() {}
+constexpr ~S() { // expected-error {{never produces a constant 
expression}} \
+ // ref-error {{never produces a constant expression}}
+  int i = 1 / 0; // expected-warning {{division by zero}} \
+ // expected-note {{division by zero}} \
+ // ref-warning {{division by zero}} \
+ // ref-note 2{{division by zero}}
+}
+  };
+  constexpr int testS() {
+S{}; // ref-note {{in call to 'S{}.~S()'}}
+return 1;
+  // FIXME: ^ Wrong line
+  }
+  static_assert(testS() == 1); // expected-error {{not an integral constant 
expression}} \
+   // ref-error {{not an integral constant 
expression}} \
+   // ref-note {{in call to 'testS()'}}
 }
 
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -2158,8 +2158,7 @@
   // Now emit the destructor and recurse into base classes.
   if (const CXXDestructorDecl *Dtor = R->getDestructor();
   Dtor && !Dtor->isTrivial()) {
-const Function *DtorFunc = getFunction(Dtor);
-if (DtorFunc && DtorFunc->isConstexpr()) {
+if (const Function *DtorFunc = getFunction(Dtor)) {
   assert(DtorFunc->hasThisPointer());
   assert(DtorFunc->getNumParams() == 1);
   if (!this->emitDupPtr(SourceInfo{}))


Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -566,6 +566,25 @@
 return i;
   }
   static_assert(test() == 1);
+
+  struct S {
+constexpr S() {}
+constexpr ~S() { // expected-error {{never produces a constant expression}} \
+ // ref-error {{never produces a constant expression}}
+  int i = 1 / 0; // expected-warning {{division by zero}} \
+ // expected-note {{division by zero}} \
+ // ref-warning {{division by zero}} \
+ // ref-note 2{{division by zero}}
+}
+  };
+  constexpr int testS() {
+S{}; // ref-note {{in call to 'S{}.~S()'}}
+return 1;
+  // FIXME: ^ Wrong line
+  }
+  static_assert(testS() == 1); // expected-error {{not an integral constant expression}} \
+   // ref-error {{not an integral constant expression}} \
+   // ref-note {{in call to 'testS()'}}
 }
 
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -2158,8 +2158,7 @@
   // Now emit the destructor and recurse into base classes.
   if (const CXXDestructorDecl *Dtor = R->getDestructor();
   Dtor && !Dtor->isTrivial()) {
-const Function *DtorFunc = getFunction(Dtor);
-if (DtorFunc && DtorFunc->isConstexpr()) {
+if (const Function *DtorFunc = getFunction(Dtor)) {
   assert(DtorFunc->hasThisPointer());
   assert(DtorFunc->getNumParams() == 1);
   if (!this->emitDupPtr(SourceInfo{}))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150040: [clang][Interp] Call invalid destructors

[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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150040

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

[PATCH] D150040: [clang][Interp] Call invalid destructors

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150040

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

[PATCH] D150040: [clang][Interp] Call invalid destructors

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

tbaeder added inline comments.



Comment at: clang/test/AST/Interp/records.cpp:606
+S{}; // ref-note {{in call to '{}->~S()'}}
+return 1; // expected-note {{in call to '{}->~S()'}}
+  // FIXME: ^ Wrong line

erichkeane wrote:
> Which DTOR is happening here that changes behavior in this patch?  You 
> removed the `isConstexpr` test, but it seems to me that the `S` destructor is 
> constexpr, right?
`Function::isConstexpr()` doesn't return the same as 
`FunctionDecl::isConstexpr()` - it's just `return IsValid` right now, so if the 
compilation of the function aborts at any point, `Function::isConstexpr()` 
returns `false` (even if the function that was compiled is `constexpr`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150040

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

[PATCH] D150040: [clang][Interp] Call invalid destructors

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/AST/Interp/records.cpp:606
+S{}; // ref-note {{in call to '{}->~S()'}}
+return 1; // expected-note {{in call to '{}->~S()'}}
+  // FIXME: ^ Wrong line

Which DTOR is happening here that changes behavior in this patch?  You removed 
the `isConstexpr` test, but it seems to me that the `S` destructor is 
constexpr, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150040

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

[PATCH] D150040: [clang][Interp] Call invalid destructors

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

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

  We need to call them like any other function, so we can generate proper
  diagnostics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150040

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.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
@@ -590,6 +590,26 @@
 return i;
   }
   static_assert(test() == 1);
+
+  struct S {
+constexpr S() {}
+constexpr ~S() { // expected-error {{never produces a constant 
expression}} \
+ // ref-error {{never produces a constant expression}}
+  int i = 1 / 0; // expected-warning {{division by zero}} \
+ // expected-note 2{{division by zero}} \
+ // ref-warning {{division by zero}} \
+ // ref-note 2{{division by zero}}
+}
+  };
+  constexpr int testS() {
+S{}; // ref-note {{in call to '{}->~S()'}}
+return 1; // expected-note {{in call to '{}->~S()'}}
+  // FIXME: ^ Wrong line
+  }
+  static_assert(testS() == 1); // expected-error {{not an integral constant 
expression}} \
+   // expected-note {{in call to 'testS()'}} \
+   // ref-error {{not an integral constant 
expression}} \
+   // ref-note {{in call to 'testS()'}}
 }
 
 namespace BaseToDerived {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -2290,8 +2290,7 @@
   // Now emit the destructor and recurse into base classes.
   if (const CXXDestructorDecl *Dtor = R->getDestructor();
   Dtor && !Dtor->isTrivial()) {
-const Function *DtorFunc = getFunction(Dtor);
-if (DtorFunc && DtorFunc->isConstexpr()) {
+if (const Function *DtorFunc = getFunction(Dtor)) {
   assert(DtorFunc->hasThisPointer());
   assert(DtorFunc->getNumParams() == 1);
   if (!this->emitDupPtr(SourceInfo{}))


Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -590,6 +590,26 @@
 return i;
   }
   static_assert(test() == 1);
+
+  struct S {
+constexpr S() {}
+constexpr ~S() { // expected-error {{never produces a constant expression}} \
+ // ref-error {{never produces a constant expression}}
+  int i = 1 / 0; // expected-warning {{division by zero}} \
+ // expected-note 2{{division by zero}} \
+ // ref-warning {{division by zero}} \
+ // ref-note 2{{division by zero}}
+}
+  };
+  constexpr int testS() {
+S{}; // ref-note {{in call to '{}->~S()'}}
+return 1; // expected-note {{in call to '{}->~S()'}}
+  // FIXME: ^ Wrong line
+  }
+  static_assert(testS() == 1); // expected-error {{not an integral constant expression}} \
+   // expected-note {{in call to 'testS()'}} \
+   // ref-error {{not an integral constant expression}} \
+   // ref-note {{in call to 'testS()'}}
 }
 
 namespace BaseToDerived {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -2290,8 +2290,7 @@
   // Now emit the destructor and recurse into base classes.
   if (const CXXDestructorDecl *Dtor = R->getDestructor();
   Dtor && !Dtor->isTrivial()) {
-const Function *DtorFunc = getFunction(Dtor);
-if (DtorFunc && DtorFunc->isConstexpr()) {
+if (const Function *DtorFunc = getFunction(Dtor)) {
   assert(DtorFunc->hasThisPointer());
   assert(DtorFunc->getNumParams() == 1);
   if (!this->emitDupPtr(SourceInfo{}))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits