[Clang][Sema][Parse] Delay parsing of noexcept-specifiers in friend function declarations

[sdkrystian]

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), and any code which did compile should continue to do so.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

According to [[class.mem.general] p8](http://eel.is/c++draft/class.mem.general#8):
> 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](http://eel.is/c++draft/temp.deduct.general#7)), and any code which did compile should continue to do so.

---

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

1 Files Affected:

• (modified) clang/lib/Parse/ParseDecl.cpp (+14-6)

diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index a7846e102a43c7..93950e27a08f35 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -7388,12 +7388,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) &&

[sdkrystian]

Note: This still needs a release note + test updates

[sdkrystian]

This actually requires a little more work... (the delayed exception specification parsing code seems to only expect CXXMethodDecls). I'll take care of that tomorrow.

[erichkeane]

Missing test + release note.

In the future, please use 'draft pull request' when your patch is not ready for review/to be committed.

[github-actions]

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

[sdkrystian]

PR updated with full implementation, tests, and release note

[cor3ntin]

a nit but otherwise LGTM
I'd like @erichkeane to look at it too

[erichkeane]

Corentin's request for a comment makes sense to me, or perhaps a better name th an just 'D'? So please take care of his request, but otherwise LGTM.