Skip to content

[Clang] Correctly finds subexpressions of immediate invocations #106055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 26, 2024

Conversation

cor3ntin
Copy link
Contributor

We were not correctly ignoring implicit casts.

Fixes #105558

We were not correctly ignoring implicit casts.

Fixes llvm#105558
@cor3ntin cor3ntin requested review from AaronBallman, Sirraide and tbaederr and removed request for AaronBallman and Sirraide August 26, 2024 10:33
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 26, 2024

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

We were not correctly ignoring implicit casts.

Fixes #105558


Full diff: https://github.com/llvm/llvm-project/pull/106055.diff

4 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+14-3)
  • (modified) clang/test/AST/ByteCode/new-delete.cpp (+5-11)
  • (modified) clang/test/SemaCXX/cxx2a-consteval.cpp (+23)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8f98167dff31ef..7d3eebb604a244 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -292,6 +292,7 @@ Bug Fixes to C++ Support
 - Correctly check constraints of explicit instantiations of member functions. (#GH46029)
 - Fixed an assertion failure about a constraint of a friend function template references to a value with greater
   template depth than the friend function template. (#GH98258)
+- Correctly handle subexpressions of an immediate invocation in the presence of implicit casts. (#GH105558)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index c67183df335dd5..1dd0f4bd717c3f 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -17464,11 +17464,22 @@ static void RemoveNestedImmediateInvocation(
     ExprResult TransformInitializer(Expr *Init, bool NotCopyInit) {
       if (!Init)
         return Init;
+
+      // We cannot use IgnoreImpCasts because we need to preserve
+      // full expressions.
+      while (true) {
+        if (auto *ICE = dyn_cast<ImplicitCastExpr>(Init))
+          Init = ICE->getSubExpr();
+        else if (auto *ICE = dyn_cast<MaterializeTemporaryExpr>(Init))
+          Init = ICE->getSubExpr();
+        else
+          break;
+      }
       /// ConstantExpr are the first layer of implicit node to be removed so if
       /// Init isn't a ConstantExpr, no ConstantExpr will be skipped.
-      if (auto *CE = dyn_cast<ConstantExpr>(Init))
-        if (CE->isImmediateInvocation())
-          RemoveImmediateInvocation(CE);
+      if (auto *CE = dyn_cast<ConstantExpr>(Init);
+          CE && CE->isImmediateInvocation())
+        RemoveImmediateInvocation(CE);
       return Base::TransformInitializer(Init, NotCopyInit);
     }
     ExprResult TransformDeclRefExpr(DeclRefExpr *E) {
diff --git a/clang/test/AST/ByteCode/new-delete.cpp b/clang/test/AST/ByteCode/new-delete.cpp
index 6bb30bc19f110c..b90897d66795f3 100644
--- a/clang/test/AST/ByteCode/new-delete.cpp
+++ b/clang/test/AST/ByteCode/new-delete.cpp
@@ -404,7 +404,7 @@ constexpr typename std::remove_reference<T>::type&& move(T &&t) noexcept {
 
 namespace cxx2a {
 struct A {
-  int* p = new int(42); // both-note 7{{heap allocation performed here}}
+  int* p = new int(42); // both-note 3{{heap allocation performed here}}
   consteval int ret_i() const { return p ? *p : 0; }
   consteval A ret_a() const { return A{}; }
   constexpr ~A() { delete p; }
@@ -433,9 +433,7 @@ void test() {
   { A k = to_lvalue_ref(A()); } // both-error {{'cxx2a::to_lvalue_ref' is not a constant expression}} \
                                 // both-note {{reference to temporary is not a constant expression}} \
                                 // both-note {{temporary created here}}
-  { A k = to_lvalue_ref(A().ret_a()); } // both-error {{'cxx2a::A::ret_a' is not a constant expression}} \
-                                        // both-note {{heap-allocated object is not a constant expression}} \
-                                        // both-error {{'cxx2a::to_lvalue_ref' is not a constant expression}} \
+  { A k = to_lvalue_ref(A().ret_a()); } // both-error {{'cxx2a::to_lvalue_ref' is not a constant expression}} \
                                         // both-note {{reference to temporary is not a constant expression}} \
                                         // both-note {{temporary created here}}
   { int k = A().ret_a().ret_i(); } // both-error {{'cxx2a::A::ret_a' is not a constant expression}} \
@@ -445,19 +443,15 @@ void test() {
   { int k = const_a_ref(a); }
   { int k = rvalue_ref(A()); }
   { int k = rvalue_ref(std::move(a)); }
-  { int k = const_a_ref(A().ret_a()); } // both-error {{'cxx2a::A::ret_a' is not a constant expression}} \
-                                        // both-note {{is not a constant expression}}
-  { int k = const_a_ref(to_lvalue_ref(A().ret_a())); } // both-error {{'cxx2a::A::ret_a' is not a constant expression}} \
-                                                       // both-note {{is not a constant expression}}
+  { int k = const_a_ref(A().ret_a()); }
+  { int k = const_a_ref(to_lvalue_ref(A().ret_a())); }
   { int k = const_a_ref(to_lvalue_ref(std::move(a))); }
   { int k = by_value_a(A().ret_a()); }
   { int k = by_value_a(to_lvalue_ref(static_cast<const A&&>(a))); }
   { int k = (A().ret_a(), A().ret_i()); } // both-error {{'cxx2a::A::ret_a' is not a constant expression}} \
                                           // both-note {{is not a constant expression}} \
                                           // both-warning {{left operand of comma operator has no effect}}
-  { int k = (const_a_ref(A().ret_a()), A().ret_i()); }  // both-error {{'cxx2a::A::ret_a' is not a constant expression}} \
-                                                        // both-note {{is not a constant expression}} \
-                                                        // both-warning {{left operand of comma operator has no effect}}
+  { int k = (const_a_ref(A().ret_a()), A().ret_i()); } // both-warning {{left operand of comma operator has no effect}}
 }
 }
 
diff --git a/clang/test/SemaCXX/cxx2a-consteval.cpp b/clang/test/SemaCXX/cxx2a-consteval.cpp
index 6b0609a26c5882..f96be6cf0424a9 100644
--- a/clang/test/SemaCXX/cxx2a-consteval.cpp
+++ b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -1232,4 +1232,27 @@ consteval void immediate() {
 }
 
 
+}
+
+namespace GH105558 {
+
+consteval int* alloc() { return new int(0); }
+consteval void f(int* p) { delete p; }
+consteval void g1(int*&& p) { delete p; }
+consteval void g2(const int* p) { delete p; }
+consteval void g3(int*const& p) { delete p; }
+struct X {
+  int* p;
+  explicit(false) constexpr X(int* p) : p(p) {}
+};
+consteval void g4(X x) { delete x.p; }
+
+void test() {
+  f(alloc());
+  g1(alloc());
+  g2(alloc());
+  g3(alloc());
+  g4(alloc());
+}
+
 }

Copy link
Member

@Sirraide Sirraide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cor3ntin cor3ntin merged commit c821cc3 into llvm:main Aug 26, 2024
7 of 9 checks passed
@cor3ntin cor3ntin deleted the corentin/gh105558 branch August 26, 2024 20:55
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 26, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-amdgpu-runtime running on omp-vega20-0 while building clang at step 6 "test-openmp".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/4816

Here is the relevant piece of the build log for the reference
Step 6 (test-openmp) failure: test (failure)
******************** TEST 'libomp :: tasking/issue-94260-2.c' FAILED ********************
Exit Code: -11

Command Output (stdout):
--
# RUN: at line 1
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp   -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -fno-omit-frame-pointer -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test/ompt /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic && /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -fno-omit-frame-pointer -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test/ompt /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/openmp/runtime/test/tasking/issue-94260-2.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp -lm -latomic
# note: command had no output on stdout or stderr
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/test/tasking/Output/issue-94260-2.c.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11

--

********************


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clang fails to compile correct program with consteval
4 participants