[Clang][Sema] Implement approved resolution for CWG2858

[sdkrystian]

The approved resolution for CWG2858 changes [expr.prim.id.qual] p2 sentence 2 to read:

A declarative nested-name-specifier shall not have a computed-type-specifier.

This patch implements the approved resolution. Since we don't consider nested-name-specifiers in friend declarations to be declarative (yet), it currently isn't possible to write a test that would produce this diagnostic (diagnoseQualifiedDeclaration is never called if the DeclContext can't be computed). Nevertheless, tests were added which will produce the diagnostic once we start calling diagnoseQualifiedDeclaration for friend declarations.

(note: this still needs a release note)

[llvmbot]

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

The approved resolution for CWG2858 changes [[expr.prim.id.qual] p2 sentence 2](https://eel.is/c++draft/expr.prim.id.qual#2) to read:
> A declarative nested-name-specifier shall not have a computed-type-specifier.

This patch implements the approved resolution. Since we don't consider nested-name-specifiers in friend declarations to be declarative (yet), it currently isn't possible to write a test that would produce this diagnostic (diagnoseQualifiedDeclaration is never called if the DeclContext can't be computed). Nevertheless, tests were added which will produce the diagnostic once we start calling diagnoseQualifiedDeclaration for friend declarations.

---

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

3 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3-4)
• (modified) clang/lib/Sema/SemaDecl.cpp (+6-7)
• (modified) clang/test/CXX/expr/expr.prim/expr.prim.id/expr.prim.id.qual/p3.cpp (+16)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a1dda2d2461c31..2b9077173aa426 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2402,10 +2402,6 @@ def err_selected_explicit_constructor : Error<
 def note_explicit_ctor_deduction_guide_here : Note<
   "explicit %select{constructor|deduction guide}0 declared here">;
 
-// C++11 decltype
-def err_decltype_in_declarator : Error<
-    "'decltype' cannot be used to name a declaration">;
-
 // C++11 auto
 def warn_cxx98_compat_auto_type_specifier : Warning<
   "'auto' type specifier is incompatible with C++98">,
@@ -8302,6 +8298,9 @@ def ext_template_after_declarative_nns : ExtWarn<
 def ext_alias_template_in_declarative_nns : ExtWarn<
   "a declarative nested name specifier cannot name an alias template">,
   InGroup<DiagGroup<"alias-template-in-declaration-name">>;
+def err_computed_type_in_declarative_nns  : Error<
+  "%select{a pack indexing type|'decltype'}0 cannot be used in "
+  "a declarative nested name specifier">;
 
 def err_no_typeid_with_fno_rtti : Error<
   "use of typeid requires -frtti">;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index c790dab72dd721..1ba6b3beb1c758 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6335,16 +6335,15 @@ bool Sema::diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC,
         if (TST->isDependentType() && TST->isTypeAlias())
           Diag(Loc, diag::ext_alias_template_in_declarative_nns)
               << SpecLoc.getLocalSourceRange();
-      } else if (T->isDecltypeType()) {
+      } else if (T->isDecltypeType() || T->getAsAdjusted<PackIndexingType>()) {
         // C++23 [expr.prim.id.qual]p2:
         //   [...] A declarative nested-name-specifier shall not have a
-        //   decltype-specifier.
+        //   computed-type-specifier.
         //
-        // FIXME: This wording appears to be defective as it does not forbid
-        // declarative nested-name-specifiers with pack-index-specifiers.
-        // See https://github.com/cplusplus/CWG/issues/499.
-        Diag(Loc, diag::err_decltype_in_declarator)
-            << SpecLoc.getTypeLoc().getSourceRange();
+        // CWG2858 changed this from 'decltype-specifier' to
+        // 'computed-type-specifier'.
+        Diag(Loc, diag::err_computed_type_in_declarative_nns)
+            << T->isDecltypeType() << SpecLoc.getTypeLoc().getSourceRange();
       }
     }
   } while ((SpecLoc = SpecLoc.getPrefix()));
diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.id/expr.prim.id.qual/p3.cpp b/clang/test/CXX/expr/expr.prim/expr.prim.id/expr.prim.id.qual/p3.cpp
index c73ffa55a26a31..5da13cc22abef2 100644
--- a/clang/test/CXX/expr/expr.prim/expr.prim.id/expr.prim.id.qual/p3.cpp
+++ b/clang/test/CXX/expr/expr.prim/expr.prim.id/expr.prim.id.qual/p3.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -std=c++2c -verify %s
 
 template<typename T>
 struct A {
@@ -27,3 +28,18 @@ namespace N {
 
 template<typename T>
 void N::E<T>::f() { } // expected-warning {{a declarative nested name specifier cannot name an alias template}}
+
+#if __cplusplus > 202302L
+template<typename... Ts>
+struct A {
+  // FIXME: The nested-name-specifier in the following friend declarations are declarative,
+  // but we don't treat them as such (yet).
+  friend void Ts...[0]::f();
+  template<typename U>
+  friend void Ts...[0]::g();
+
+  friend struct Ts...[0]::B;
+  template<typename U>
+  friend struct Ts...[0]::C; // expected-warning{{is not supported; ignoring this friend declaration}}
+};
+#endif

[erichkeane]

This seems right to me, thanks!

[cor3ntin]

This needs a release note and a test in test/CXX/drs/dr28xx.cpp (and a rerun of www/make_cxx_dr_status )

[sdkrystian]

@cor3ntin Should I just move the test from test/CXX/expr/expr.prim/expr.prim.id/expr.prim.id.qual/p3.cpp to test/CXX/drs/dr28xx.cpp?

[cor3ntin]

@cor3ntin Should I just move the test from test/CXX/expr/expr.prim/expr.prim.id/expr.prim.id.qual/p3.cpp to test/CXX/drs/dr28xx.cpp?

Sounds reasonable, yes!

[Endilll]

This looks much better now, thank you!
As Corentin suggested earlier, run clang/www/make_cxx_dr_status to update the DR page, and this should be good to go.

[sdkrystian]

@Endilll clang/www/make_cxx_dr_status queries core issues from https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_index.html. Since a new revision of the index has not yet been published since CWG2858 was opened, clang/www/make_cxx_dr_status doesn't add an entry for the issue.

[erichkeane]

@Endilll clang/www/make_cxx_dr_status queries core issues from https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_index.html. Since a new revision of the index has not yet been published since CWG2858 was opened, clang/www/make_cxx_dr_status doesn't add an entry for the issue.

Yeah, those don't get published consistently. You have to pick them up from the latest committee wiki (see the 'attachments' list here: https://wiki.edg.com/bin/view/Wg21tokyo2024/CoreWorkingGroup).

[sdkrystian]

Could also get them from the CWG GitHub pages repository (https://github.com/cplusplus/CWG/tree/gh-pages/issues)... anyways, am I good to merge this?

[erichkeane]

Could also get them from the CWG GitHub pages repository (https://github.com/cplusplus/CWG/tree/gh-pages/issues)... anyways, am I good to merge this?

I believe we would still like you to re-generate with : www/make_cxx_dr_status

[sdkrystian]

Oh, sorry. I'll do that :)

[erichkeane]

@Endilll says 'Good to go' with the dr-status updated, so I'll approve.

[Endilll]

make_cxx_dr_status is not going to do anything here, since the DR indeed didn't make it into public list.
Merge as-is. 2858 is going to be picked up when someone runs the script for the first time after a new revision of core issues list is published.

[sdkrystian]

@Endilll I ran make_cxx_dr_status with the index from the WG21 wiki... should I drop the commit?

[Endilll]

Please do. This is going to be reverted by the next person fetching from public index page anyway, because we ask people to run the script every time they have changes for cxx_dr_status.html. In any case, WG21 wikis are of restricted access, and we shouldn't make their content public like this.