[Clang] Ensure the method scope at the late parsing of noexcept specifiers

[zyn0217]

Previously, we only pushed the function scope once we entered the function definition, whereas tryCaptureVariable() requires at least one function scope available when ParmVarDecls being captured have been owned by a function. This led to problems parsing the noexcept specifiers, as the DeclRefExprs inside them were improperly computed.

Fixes #97453

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

Previously, we only pushed the method scope once we entered the function definition, whereas tryCaptureVariable() requires at least one function scope available when being called. This led to problems parsing the noexcept specifiers, as the DeclRefs inside them were improperly computed.

As a bonus, this also fixes an implementation glitch of C++11 [expr.prim.general]p3: we shouldn't add an implicit 'this' to expressions inside noexcept specifiers.

Fixes #97453

---

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

5 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1-1)
• (modified) clang/lib/Parse/ParseCXXInlineMethods.cpp (+13-2)
• (modified) clang/test/CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp (+3-2)
• (modified) clang/test/CodeGenCXX/mangle-exception-spec.cpp (+11)
• (modified) clang/test/SemaCXX/cxx0x-noexcept-expression.cpp (+8)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 838cb69f647ee2..b521de4515e1f4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -958,7 +958,7 @@ Bug Fixes to C++ Support
 - Fixed a type constraint substitution issue involving a generic lambda expression. (#GH93821)
 - Fix a crash caused by improper use of ``__array_extent``. (#GH80474)
 - Fixed several bugs in capturing variables within unevaluated contexts. (#GH63845), (#GH67260), (#GH69307),
-  (#GH88081), (#GH89496), (#GH90669) and (#GH91633).
+  (#GH88081), (#GH89496), (#GH90669), (#GH91633) and (#GH97453).
 - Fixed handling of brace ellison when building deduction guides. (#GH64625), (#GH83368).
 - Clang now instantiates local constexpr functions eagerly for constant evaluators. (#GH35052), (#GH94849)
 - Fixed a failed assertion when attempting to convert an integer representing the difference
diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index 943ce0fdde3a38..faab9c6b19caac 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -511,11 +511,13 @@ void Parser::ParseLexedMethodDeclaration(LateParsedMethodDeclaration &LM) {
     //   and the end of the function-definition, member-declarator, or
     //   declarator.
     CXXMethodDecl *Method;
+    FunctionDecl *FunctionToPush;
     if (FunctionTemplateDecl *FunTmpl
           = dyn_cast<FunctionTemplateDecl>(LM.Method))
-      Method = dyn_cast<CXXMethodDecl>(FunTmpl->getTemplatedDecl());
+      FunctionToPush = FunTmpl->getTemplatedDecl();
     else
-      Method = dyn_cast<CXXMethodDecl>(LM.Method);
+      FunctionToPush = cast<FunctionDecl>(LM.Method);
+    Method = dyn_cast<CXXMethodDecl>(FunctionToPush);
 
     Sema::CXXThisScopeRAII ThisScope(
         Actions, Method ? Method->getParent() : nullptr,
@@ -529,6 +531,15 @@ void Parser::ParseLexedMethodDeclaration(LateParsedMethodDeclaration &LM) {
     ExprResult NoexceptExpr;
     CachedTokens *ExceptionSpecTokens;
 
+    // Push a function scope so that tryCaptureVariable() can properly visit
+    // function scopes involving function parameters that are referenced inside
+    // the noexcept specifier e.g. through a lambda expression.
+    // Example:
+    // struct X {
+    //   void ICE(int val) noexcept(noexcept([val]{}));
+    // };
+    Sema::SynthesizedFunctionScope Scope(Actions, FunctionToPush);
+
     ExceptionSpecificationType EST
       = tryParseExceptionSpecification(/*Delayed=*/false, SpecificationRange,
                                        DynamicExceptions,
diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp b/clang/test/CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp
index b59cc175a1976f..d50d05d17ab7a1 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp
@@ -113,8 +113,9 @@ namespace Static {
     static auto g() -> decltype(this->m); // expected-error{{'this' cannot be used in a static member function declaration}}
 
     static int h();
-    
-    static int i() noexcept(noexcept(m + 2)); // expected-error{{'this' cannot be implicitly used in a static member function declaration}}
+
+    // The use of 'm' doesn't constitute an ODR use, so we don't have a reason to reject it.
+    static int i() noexcept(noexcept(m + 2));
   };
 
   auto X1::h() -> decltype(m) { return 0; } // expected-error{{'this' cannot be implicitly used in a static member function declaration}}
diff --git a/clang/test/CodeGenCXX/mangle-exception-spec.cpp b/clang/test/CodeGenCXX/mangle-exception-spec.cpp
index 15f7a8b6cb5048..1bc268e1b6ca44 100644
--- a/clang/test/CodeGenCXX/mangle-exception-spec.cpp
+++ b/clang/test/CodeGenCXX/mangle-exception-spec.cpp
@@ -47,3 +47,14 @@ template auto i<>(int()) -> int (*)();
 // CHECK-CXX11: define {{.*}} @_Z1iIJfEEPDwiDpT_EFivEPS2_(
 // CHECK-CXX17: define {{.*}} @_Z1iIJfEEPDwiDpT_EFivES3_(
 template auto i<float>(int()) -> int (*)();
+
+template <class T>
+struct X1 {
+  T m;
+  static void j(int (X1::*)() noexcept(noexcept(m + 2)));
+};
+
+void foo() {
+  // CHECK-CXX17: call {{.*}} @_ZN2X1IiE1jEMS0_DoFivE(
+  X1<int>::j(nullptr);
+}
diff --git a/clang/test/SemaCXX/cxx0x-noexcept-expression.cpp b/clang/test/SemaCXX/cxx0x-noexcept-expression.cpp
index c616a77f36619a..53d23580d02a28 100644
--- a/clang/test/SemaCXX/cxx0x-noexcept-expression.cpp
+++ b/clang/test/SemaCXX/cxx0x-noexcept-expression.cpp
@@ -157,3 +157,11 @@ void f5() {
   // expected-error@-1 {{no member named 'non_existent' in 'typeid_::X<true>'}}
 }
 } // namespace typeid_
+
+namespace GH97453 {
+
+struct X {
+  void ICE(int that) noexcept(noexcept([that]() {}));
+};
+
+} // namespace GH97453

[zyn0217]

All major compilers (except for us) accept the code: https://gcc.godbolt.org/z/ffxK4aKTa

[cor3ntin]

I would be great to find the corresponding wording

[zyn0217]

Done. I didn't even know we had relevant wordings about such scenarios until @frederick-vs-ja (thanks!) told me about [expr.prim.id.general]/4.

[cor3ntin]

I would be great to find the corresponding wording

[zyn0217]

Looks like this caused a regression in std/utilities/tuple/tuple.tuple/tuple.cnstr/PR23256_constrain_UTypes_ctor.pass.cpp... I will look into that.

[zyn0217]

So the previous problem was that we ended up exercising IsInFnTryBlockHandler() on constructors with noexcept specifiers. That function assumes that if we're in a function DeclContext (which is brought by SynthesizedFunctionScope, which also pushes a DeclContext), we would also have had a function Scope pushed. So, a solution is to pull in a Scope as well.

Regarding the conformance of [expr.prim.id.general]p4, I think overriding the ThisType is a bit suboptimal. While it affects ClassifyImplicitMemberAccess() so that we don't need an implicit this in such situations, I think a probably more optimal way is to fix the algorithm in ClassifyImplicitMemberAccess() to consider unevaluated contexts - that could presumably resolve all conforming cases in clang/test/CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp. So, I reverted these changes at the moment.

[cor3ntin]

LGTM