Skip to content
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

[clang]use correct this scope to evaluate noexcept expr #77416

Conversation

HerrCai0907
Copy link
Contributor

Fixes: #77411
When substituting deduced type, noexcept expr in method should be instantiated and evaluated.
ThisScrope should be switched to method context instead of origin sema context

…emplate

Fixes: #77411
When substituting deduced type, noexcept expr in function decl should be instantiated and evaluated.
ThisScrope should be switched to method context
@HerrCai0907 HerrCai0907 linked an issue Jan 9, 2024 that may be closed by this pull request
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 9, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 9, 2024

@llvm/pr-subscribers-clang

Author: Congcong Cai (HerrCai0907)

Changes

Fixes: #77411
When substituting deduced type, noexcept expr in method should be instantiated and evaluated.
ThisScrope should be switched to method context instead of origin sema context


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2-1)
  • (modified) clang/lib/Sema/TreeTransform.h (+6)
  • (modified) clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp (+15)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 803eb2f7c74cf6..2695a0e30f58e0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -610,7 +610,8 @@ Bug Fixes in This Version
   of template classes. Fixes
   (`#68543 <https://github.com/llvm/llvm-project/issues/68543>`_,
   `#42496 <https://github.com/llvm/llvm-project/issues/42496>`_,
-  `#77071 <https://github.com/llvm/llvm-project/issues/77071>`_)
+  `#77071 <https://github.com/llvm/llvm-project/issues/77071>`_,
+  `#77411 <https://github.com/llvm/llvm-project/issues/77411>`_)
 - Fixed an issue when a shift count larger than ``__INT64_MAX__``, in a right
   shift operation, could result in missing warnings about
   ``shift count >= width of type`` or internal compiler error.
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index c8c5a51bf9f94e..fb7e15b5bcbb02 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -6192,6 +6192,12 @@ bool TreeTransform<Derived>::TransformExceptionSpec(
 
   // Instantiate a dynamic noexcept expression, if any.
   if (isComputedNoexcept(ESI.Type)) {
+    // Update this scrope because ContextDecl in Sema will be used in TransformExpr.
+    auto *Method = dyn_cast_or_null<CXXMethodDecl>(ESI.SourceTemplate);
+    Sema::CXXThisScopeRAII ThisScope(
+        SemaRef, Method ? Method->getParent() : nullptr,
+        Method ? Method->getMethodQualifiers() : Qualifiers{},
+        Method != nullptr);
     EnterExpressionEvaluationContext Unevaluated(
         getSema(), Sema::ExpressionEvaluationContext::ConstantEvaluated);
     ExprResult NoexceptExpr = getDerived().TransformExpr(ESI.NoexceptExpr);
diff --git a/clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp b/clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
index 11b1093f9064f6..5e56f19477d6ca 100644
--- a/clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
+++ b/clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
@@ -64,6 +64,21 @@ namespace DependentDefaultCtorExceptionSpec {
   struct A { multimap<int> Map; } a;
 
   static_assert(noexcept(A()));
+
+  template <class> struct NoexceptWithThis {
+    int ca;
+    template <class T> auto foo(T) noexcept(ca) { return true; }
+    // expected-error@-1 {{noexcept specifier argument is not a constant expression}}
+    // expected-note@-2 {{in instantiation of exception specification}}
+    // expected-note@-3 {{implicit use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function}}
+  };
+  struct InstantiateFromAnotherClass {
+    template <class B, class T = decltype(static_cast<bool (B::*)(int)>(&B::foo))> // expected-note {{in instantiation of function template specialization}}
+    InstantiateFromAnotherClass(B *) {} // expected-note {{in instantiation of default argument}}
+  };
+  NoexceptWithThis<int> f{};
+  // Don't crash here.
+  InstantiateFromAnotherClass b{&f}; // expected-note {{while substituting deduced template arguments into function template}}
 }
 
 #endif

Copy link

github-actions bot commented Jan 9, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -6192,6 +6192,12 @@ bool TreeTransform<Derived>::TransformExceptionSpec(

// Instantiate a dynamic noexcept expression, if any.
if (isComputedNoexcept(ESI.Type)) {
// Update this scrope because ContextDecl in Sema will be used in TransformExpr.
auto *Method = dyn_cast_or_null<CXXMethodDecl>(ESI.SourceTemplate);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
auto *Method = dyn_cast_or_null<CXXMethodDecl>(ESI.SourceTemplate);
auto *Method = dyn_cast_if_present<CXXMethodDecl>(ESI.SourceTemplate);

@HerrCai0907 HerrCai0907 merged commit 4b7e861 into main Jan 9, 2024
5 checks passed
@HerrCai0907 HerrCai0907 deleted the users/ccc/77411-cannot-resolve-this-correctly-in-transformexceptionspec branch January 9, 2024 15:48
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Fixes: llvm#77411
When substituting deduced type, noexcept expr in method should be
instantiated and evaluated.
ThisScrope should be switched to method context instead of origin sema
context
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.

cannot resolve this correctly in TransformExceptionSpec
4 participants