[clang] f061a39 - [Clang][Sema][Parse] Delay parsing of noexcept-specifiers in friend function declarations (#90517)

via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 30 11:23:07 PDT 2024


Author: Krystian Stasiowski
Date: 2024-04-30T14:23:02-04:00
New Revision: f061a395ffb78215a23e0f503e8ea121ee3b13ad

URL: https://github.com/llvm/llvm-project/commit/f061a395ffb78215a23e0f503e8ea121ee3b13ad
DIFF: https://github.com/llvm/llvm-project/commit/f061a395ffb78215a23e0f503e8ea121ee3b13ad.diff

LOG: [Clang][Sema][Parse] Delay parsing of noexcept-specifiers in friend function declarations (#90517)

According to [class.mem.general] p8:
> A complete-class context of a class (template) is a
> - function body,
> - default argument,
> - default template argument,
> - _noexcept-specifier_, or
> - default member initializer
>
> within the member-specification of the class or class template.

When testing #90152, it came to my attention that we do _not_ consider
the _noexcept-specifier_ of a friend function declaration to be a
complete-class context (something which the Microsoft standard library
depends on). Although a comment states that this is "consistent with
what other implementations do", the only other implementation that
exhibits this behavior is GCC (MSVC and EDG both late-parse the
_noexcept-specifier_).

This patch changes _noexcept-specifiers_ of friend function declarations
to be late parsed, which is in agreement with the standard & majority of
implementations. Pre-#90152, our existing implementation falls "in
between" the implementation consensus: within non-template classes, we
would not find latter declared members (qualified and unqualified),
while within class templates we would not find latter declared member
when named with a unqualified name, we would find members named with a
qualified name (even when lookup context is the current instantiation).
Therefore, this _shouldn't_ be a breaking change -- any code that didn't
compile will continue to not compile (since a _noexcept-specifier_ is
not part of the deduction substitution
loci (see [temp.deduct.general] p7), and any code which
did compile should continue to do so.

Added: 
    clang/test/CXX/class/class.mem/class.mem.general/p8.cpp
    clang/test/CXX/except/except.spec/p13-friend.cpp

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Sema/Sema.h
    clang/lib/Parse/ParseDecl.cpp
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/lib/Sema/SemaExceptionSpec.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a90df458705736..7458ddc99d4eb5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -614,6 +614,7 @@ Bug Fixes to C++ Support
 - Fix a bug on template partial specialization whose template parameter is `decltype(auto)`.
 - Fix a bug on template partial specialization with issue on deduction of nontype template parameter
   whose type is `decltype(auto)`. Fixes (#GH68885).
+- Clang now correctly treats the noexcept-specifier of a friend function to be a complete-class context.
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 809b9c4498f697..72181c9ddae7c5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4097,12 +4097,12 @@ class Sema final : public SemaBase {
                                    SmallVectorImpl<QualType> &Exceptions,
                                    FunctionProtoType::ExceptionSpecInfo &ESI);
 
-  /// Add an exception-specification to the given member function
-  /// (or member function template). The exception-specification was parsed
-  /// after the method itself was declared.
+  /// Add an exception-specification to the given member or friend function
+  /// (or function template). The exception-specification was parsed
+  /// after the function itself was declared.
   void actOnDelayedExceptionSpecification(
-      Decl *Method, ExceptionSpecificationType EST,
-      SourceRange SpecificationRange, ArrayRef<ParsedType> DynamicExceptions,
+      Decl *D, ExceptionSpecificationType EST, SourceRange SpecificationRange,
+      ArrayRef<ParsedType> DynamicExceptions,
       ArrayRef<SourceRange> DynamicExceptionRanges, Expr *NoexceptExpr);
 
   class InheritedConstructorInfo;

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 7431c256d2c135..69fabf6acb84e6 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -7416,12 +7416,20 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
       std::optional<Sema::CXXThisScopeRAII> ThisScope;
       InitCXXThisScopeForDeclaratorIfRelevant(D, DS, ThisScope);
 
-      // Parse exception-specification[opt].
-      // FIXME: Per [class.mem]p6, all exception-specifications at class scope
-      // should be delayed, including those for non-members (eg, friend
-      // declarations). But only applying this to member declarations is
-      // consistent with what other implementations do.
-      bool Delayed = D.isFirstDeclarationOfMember() &&
+      // C++ [class.mem.general]p8:
+      //   A complete-class context of a class (template) is a
+      //     - function body,
+      //     - default argument,
+      //     - default template argument,
+      //     - noexcept-specifier, or
+      //     - default member initializer
+      //   within the member-specification of the class or class template.
+      //
+      // Parse exception-specification[opt]. If we are in the
+      // member-specification of a class or class template, this is a
+      // complete-class context and parsing of the noexcept-specifier should be
+      // delayed (even if this is a friend declaration).
+      bool Delayed = D.getContext() == DeclaratorContext::Member &&
                      D.isFunctionDeclaratorAFunctionDeclaration();
       if (Delayed && Actions.isLibstdcxxEagerExceptionSpecHack(D) &&
           GetLookAheadToken(0).is(tok::kw_noexcept) &&

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 338b0ec1e099c0..83155c3216916e 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -19172,40 +19172,40 @@ void Sema::checkExceptionSpecification(
   }
 }
 
-void Sema::actOnDelayedExceptionSpecification(Decl *MethodD,
-             ExceptionSpecificationType EST,
-             SourceRange SpecificationRange,
-             ArrayRef<ParsedType> DynamicExceptions,
-             ArrayRef<SourceRange> DynamicExceptionRanges,
-             Expr *NoexceptExpr) {
-  if (!MethodD)
+void Sema::actOnDelayedExceptionSpecification(
+    Decl *D, ExceptionSpecificationType EST, SourceRange SpecificationRange,
+    ArrayRef<ParsedType> DynamicExceptions,
+    ArrayRef<SourceRange> DynamicExceptionRanges, Expr *NoexceptExpr) {
+  if (!D)
     return;
 
-  // Dig out the method we're referring to.
-  if (FunctionTemplateDecl *FunTmpl = dyn_cast<FunctionTemplateDecl>(MethodD))
-    MethodD = FunTmpl->getTemplatedDecl();
+  // Dig out the function we're referring to.
+  if (FunctionTemplateDecl *FTD = dyn_cast<FunctionTemplateDecl>(D))
+    D = FTD->getTemplatedDecl();
 
-  CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(MethodD);
-  if (!Method)
+  FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
+  if (!FD)
     return;
 
   // Check the exception specification.
   llvm::SmallVector<QualType, 4> Exceptions;
   FunctionProtoType::ExceptionSpecInfo ESI;
-  checkExceptionSpecification(/*IsTopLevel*/true, EST, DynamicExceptions,
+  checkExceptionSpecification(/*IsTopLevel=*/true, EST, DynamicExceptions,
                               DynamicExceptionRanges, NoexceptExpr, Exceptions,
                               ESI);
 
   // Update the exception specification on the function type.
-  Context.adjustExceptionSpec(Method, ESI, /*AsWritten*/true);
+  Context.adjustExceptionSpec(FD, ESI, /*AsWritten=*/true);
 
-  if (Method->isStatic())
-    checkThisInStaticMemberFunctionExceptionSpec(Method);
+  if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D)) {
+    if (MD->isStatic())
+      checkThisInStaticMemberFunctionExceptionSpec(MD);
 
-  if (Method->isVirtual()) {
-    // Check overrides, which we previously had to delay.
-    for (const CXXMethodDecl *O : Method->overridden_methods())
-      CheckOverridingFunctionExceptionSpec(Method, O);
+    if (MD->isVirtual()) {
+      // Check overrides, which we previously had to delay.
+      for (const CXXMethodDecl *O : MD->overridden_methods())
+        CheckOverridingFunctionExceptionSpec(MD, O);
+    }
   }
 }
 

diff  --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index c9dd6bb2413e38..41bf273d12f2f3 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -258,13 +258,14 @@ Sema::UpdateExceptionSpec(FunctionDecl *FD,
 }
 
 static bool exceptionSpecNotKnownYet(const FunctionDecl *FD) {
-  auto *MD = dyn_cast<CXXMethodDecl>(FD);
-  if (!MD)
+  ExceptionSpecificationType EST =
+      FD->getType()->castAs<FunctionProtoType>()->getExceptionSpecType();
+  if (EST == EST_Unparsed)
+    return true;
+  else if (EST != EST_Unevaluated)
     return false;
-
-  auto EST = MD->getType()->castAs<FunctionProtoType>()->getExceptionSpecType();
-  return EST == EST_Unparsed ||
-         (EST == EST_Unevaluated && MD->getParent()->isBeingDefined());
+  const DeclContext *DC = FD->getLexicalDeclContext();
+  return DC->isRecord() && cast<RecordDecl>(DC)->isBeingDefined();
 }
 
 static bool CheckEquivalentExceptionSpecImpl(

diff  --git a/clang/test/CXX/class/class.mem/class.mem.general/p8.cpp b/clang/test/CXX/class/class.mem/class.mem.general/p8.cpp
new file mode 100644
index 00000000000000..8cc9b41eaca919
--- /dev/null
+++ b/clang/test/CXX/class/class.mem/class.mem.general/p8.cpp
@@ -0,0 +1,78 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+namespace N0 {
+  struct A {
+    void f0() noexcept(x);
+    void g0() noexcept(y); // expected-error {{use of undeclared identifier 'y'}}
+
+    void f1() noexcept(A::x);
+    void g1() noexcept(A::y); // expected-error {{no member named 'y' in 'N0::A'}}
+
+    template<typename T>
+    void f2() noexcept(x);
+    template<typename T>
+    void g2() noexcept(y); // expected-error {{use of undeclared identifier 'y'}}
+
+    template<typename T>
+    void f3() noexcept(A::x);
+    template<typename T>
+    void g3() noexcept(A::y); // expected-error {{no member named 'y' in 'N0::A'}}
+
+    friend void f4() noexcept(x);
+    friend void g4() noexcept(y); // expected-error {{use of undeclared identifier 'y'}}
+
+    friend void f5() noexcept(A::x);
+    friend void g5() noexcept(A::y); // expected-error {{no member named 'y' in 'N0::A'}}
+
+    template<typename T>
+    friend void f6() noexcept(x);
+    template<typename T>
+    friend void g6() noexcept(y); // expected-error {{use of undeclared identifier 'y'}}
+
+    template<typename T>
+    friend void f7() noexcept(A::x);
+    template<typename T>
+    friend void g7() noexcept(A::y); // expected-error {{no member named 'y' in 'N0::A'}}
+
+    static constexpr bool x = true;
+  };
+} // namespace N0
+
+namespace N1 {
+  template<typename T>
+  struct A {
+    void f0() noexcept(x);
+    void g0() noexcept(y); // expected-error {{use of undeclared identifier 'y'}}
+
+    void f1() noexcept(A::x);
+    void g1() noexcept(A::y); // expected-error {{no member named 'y' in 'A<T>'}}
+
+    template<typename U>
+    void f2() noexcept(x);
+    template<typename U>
+    void g2() noexcept(y); // expected-error {{use of undeclared identifier 'y'}}
+
+    template<typename U>
+    void f3() noexcept(A::x);
+    template<typename U>
+    void g3() noexcept(A::y); // expected-error {{no member named 'y' in 'A<T>'}}
+
+    friend void f4() noexcept(x);
+    friend void g4() noexcept(y); // expected-error {{use of undeclared identifier 'y'}}
+
+    friend void f5() noexcept(A::x);
+    friend void g5() noexcept(A::y); // expected-error {{no member named 'y' in 'A<T>'}}
+
+    template<typename U>
+    friend void f6() noexcept(x);
+    template<typename U>
+    friend void g6() noexcept(y); // expected-error {{use of undeclared identifier 'y'}}
+
+    template<typename U>
+    friend void f7() noexcept(A::x);
+    template<typename U>
+    friend void g7() noexcept(A::y); // expected-error {{no member named 'y' in 'A<T>'}}
+
+    static constexpr bool x = true;
+  };
+} // namespace N1

diff  --git a/clang/test/CXX/except/except.spec/p13-friend.cpp b/clang/test/CXX/except/except.spec/p13-friend.cpp
new file mode 100644
index 00000000000000..7f73a4ff431aa9
--- /dev/null
+++ b/clang/test/CXX/except/except.spec/p13-friend.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fexceptions -fcxx-exceptions -fsyntax-only -verify %s
+
+namespace N0 {
+  void f() noexcept;
+  void g() noexcept;
+
+  struct A {
+    friend void f() noexcept;
+    friend void g() noexcept(x);
+
+    static constexpr bool x = true;
+  };
+} // namespace N0
+
+namespace N1 {
+  void f() noexcept;
+  void g();
+
+  template<typename T>
+  struct A {
+    friend void f() noexcept;
+    // FIXME: This error is emitted if no other errors occured (i.e. Sema::hasUncompilableErrorOccurred() is false).
+    friend void g() noexcept(x); // expected-error {{no member 'x' in 'N1::A<int>'; it has not yet been instantiated}}
+                                 // expected-note at -1 {{in instantiation of exception specification}}
+    static constexpr bool x = false; // expected-note {{not-yet-instantiated member is declared here}}
+  };
+
+  template struct A<int>; // expected-note {{in instantiation of template class}}
+} // namespace N1


        


More information about the cfe-commits mailing list