[clang] Implement CWG2877 "Type-only lookup for using-enum-declarator" #95399

Endilll · 2024-06-13T11:51:50Z

This patch implements 2024-05-31 resolution of a tentatively ready issue CWG2877 "Type-only lookup for using-enum-declarator", which supersedes earlier CWG2621 "Kind of lookup for using enum declarations".

Now we perform type-only lookup (not to be confused with type-only context) for elaborated-enum-declarator. This is the same kind of lookup that elaborated type specifiers and base specifiers undergo.

I also found out (and fixed) that one of our existing tests claimed that a dependent type can be used in elaborated-enum-declarator, but that's not the case:

The using-enum-declarator shall designate a non-dependent type with a reachable enum-specifier .

llvmbot · 2024-06-13T11:52:24Z

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

This patch implements 2024-05-31 resolution of a tentatively ready issue CWG2877 "Type-only lookup for using-enum-declarator", which supersedes earlier CWG2621 "Kind of lookup for using enum declarations".

Now we perform type-only lookup (not to be confused with type-only context) for elaborated-enum-declarator.

I also found out (and fixed) that one of our existing tests claimed that a dependent type can be used in elaborated-enum-declarator, but that's not the case:
> The using-enum-declarator shall designate a non-dependent type with a reachable enum-specifier[.](http://eel.is/c++draft/enum.udecl#1.sentence-2)

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

7 Files Affected:

(modified) clang/include/clang/Sema/Sema.h (+2-2)
(modified) clang/lib/Parse/ParseDeclCXX.cpp (+37-6)
(modified) clang/lib/Sema/SemaDeclCXX.cpp (+11-10)
(modified) clang/test/CXX/drs/cwg26xx.cpp (+2-3)
(modified) clang/test/CXX/drs/cwg28xx.cpp (+3-7)
(modified) clang/test/SemaCXX/cxx20-using-enum.cpp (+8-4)
(modified) clang/www/cxx_dr_status.html (+2-2)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 4d4579fcfd456..dc059f781dcb2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4031,8 +4031,8 @@ class Sema final : public SemaBase {
                               const ParsedAttributesView &AttrList);
   Decl *ActOnUsingEnumDeclaration(Scope *CurScope, AccessSpecifier AS,
                                   SourceLocation UsingLoc,
-                                  SourceLocation EnumLoc,
-                                  SourceLocation IdentLoc, IdentifierInfo &II,
+                                  SourceLocation EnumLoc, SourceRange TyLoc,
+                                  const IdentifierInfo &II, ParsedType Ty,
                                   CXXScopeSpec *SS = nullptr);
   Decl *ActOnAliasDeclaration(Scope *CurScope, AccessSpecifier AS,
                               MultiTemplateParamsArg TemplateParams,
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 9a4a777f575b2..51e065a6c6a86 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -692,7 +692,7 @@ bool Parser::ParseUsingDeclarator(DeclaratorContext Context,
 ///     using-enum-declaration: [C++20, dcl.enum]
 ///       'using' elaborated-enum-specifier ;
 ///       The terminal name of the elaborated-enum-specifier undergoes
-///       ordinary lookup
+///       type-only lookup
 ///
 ///     elaborated-enum-specifier:
 ///       'enum' nested-name-specifier[opt] identifier
@@ -738,16 +738,47 @@ Parser::DeclGroupPtrTy Parser::ParseUsingDeclaration(
       return nullptr;
     }
 
-    if (!Tok.is(tok::identifier)) {
+    Decl *UED = nullptr;
+
+    if (Tok.is(tok::identifier)) {
+      IdentifierInfo *IdentInfo = Tok.getIdentifierInfo();
+      SourceLocation IdentLoc = ConsumeToken();
+
+      ParsedType Type = Actions.getTypeName(
+          *IdentInfo, IdentLoc, getCurScope(), &SS, /*isClassName=*/true,
+          /*HasTrailingDot=*/false,
+          /*ObjectType=*/nullptr, /*IsCtorOrDtorName=*/false,
+          /*WantNontrivialTypeSourceInfo=*/true);
+
+      UED = Actions.ActOnUsingEnumDeclaration(
+          getCurScope(), AS, UsingLoc, UELoc, IdentLoc, *IdentInfo, Type, &SS);
+    } else if (Tok.is(tok::annot_template_id)) {
+      TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
+
+      if (TemplateId->mightBeType()) {
+        AnnotateTemplateIdTokenAsType(SS, ImplicitTypenameContext::No,
+                                      /*IsClassName=*/true);
+
+        assert(Tok.is(tok::annot_typename) && "template-id -> type failed");
+        TypeResult Type = getTypeAnnotation(Tok);
+        SourceRange Loc = Tok.getAnnotationRange();
+        ConsumeAnnotationToken();
+
+        UED = Actions.ActOnUsingEnumDeclaration(getCurScope(), AS, UsingLoc,
+                                                UELoc, Loc, *TemplateId->Name,
+                                                Type.get(), &SS);
+      } else {
+        Diag(Tok.getLocation(), diag::err_using_enum_not_enum)
+            << TemplateId->Name->getName()
+            << SourceRange(TemplateId->TemplateNameLoc, TemplateId->RAngleLoc);
+      }
+    } else {
       Diag(Tok.getLocation(), diag::err_using_enum_expect_identifier)
           << Tok.is(tok::kw_enum);
       SkipUntil(tok::semi);
       return nullptr;
     }
-    IdentifierInfo *IdentInfo = Tok.getIdentifierInfo();
-    SourceLocation IdentLoc = ConsumeToken();
-    Decl *UED = Actions.ActOnUsingEnumDeclaration(
-        getCurScope(), AS, UsingLoc, UELoc, IdentLoc, *IdentInfo, &SS);
+
     if (!UED) {
       SkipUntil(tok::semi);
       return nullptr;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 37f0df2a6a27d..d65c7df585bac 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -12394,23 +12394,24 @@ Decl *Sema::ActOnUsingDeclaration(Scope *S, AccessSpecifier AS,
 
 Decl *Sema::ActOnUsingEnumDeclaration(Scope *S, AccessSpecifier AS,
                                       SourceLocation UsingLoc,
-                                      SourceLocation EnumLoc,
-                                      SourceLocation IdentLoc,
-                                      IdentifierInfo &II, CXXScopeSpec *SS) {
+                                      SourceLocation EnumLoc, SourceRange TyLoc,
+                                      const IdentifierInfo &II, ParsedType Ty,
+                                      CXXScopeSpec *SS) {
   assert(!SS->isInvalid() && "ScopeSpec is invalid");
   TypeSourceInfo *TSI = nullptr;
-  QualType EnumTy = GetTypeFromParser(
-      getTypeName(II, IdentLoc, S, SS, /*isClassName=*/false,
-                  /*HasTrailingDot=*/false,
-                  /*ObjectType=*/nullptr, /*IsCtorOrDtorName=*/false,
-                  /*WantNontrivialTypeSourceInfo=*/true),
-      &TSI);
+  SourceLocation IdentLoc = TyLoc.getBegin();
+  QualType EnumTy = GetTypeFromParser(Ty, &TSI);
   if (EnumTy.isNull()) {
     Diag(IdentLoc, SS && isDependentScopeSpecifier(*SS)
                        ? diag::err_using_enum_is_dependent
                        : diag::err_unknown_typename)
         << II.getName()
-        << SourceRange(SS ? SS->getBeginLoc() : IdentLoc, IdentLoc);
+        << SourceRange(SS ? SS->getBeginLoc() : IdentLoc, TyLoc.getEnd());
+    return nullptr;
+  }
+
+  if (EnumTy->isDependentType()) {
+    Diag(IdentLoc, diag::err_using_enum_is_dependent);
     return nullptr;
   }
 
diff --git a/clang/test/CXX/drs/cwg26xx.cpp b/clang/test/CXX/drs/cwg26xx.cpp
index 2b17c8101438d..e180c93011a7e 100644
--- a/clang/test/CXX/drs/cwg26xx.cpp
+++ b/clang/test/CXX/drs/cwg26xx.cpp
@@ -7,7 +7,7 @@
 // RUN: %clang_cc1 -std=c++2c -triple x86_64-unknown-unknown -pedantic-errors %s -verify=expected,since-cxx11,since-cxx20,since-cxx23
 
 
-namespace cwg2621 { // cwg2621: 16
+namespace cwg2621 { // cwg2621: sup 2877
 #if __cplusplus >= 202002L
 enum class E { a };
 namespace One {
@@ -17,9 +17,8 @@ auto v = a;
 }
 namespace Two {
 using cwg2621::E;
-int E; // we see this
+int E; // ignored by type-only lookup
 using enum E;
-// since-cxx20-error@-1 {{unknown type name E}}
 }
 #endif
 }
diff --git a/clang/test/CXX/drs/cwg28xx.cpp b/clang/test/CXX/drs/cwg28xx.cpp
index da81eccc8dc22..67aa34484fe8b 100644
--- a/clang/test/CXX/drs/cwg28xx.cpp
+++ b/clang/test/CXX/drs/cwg28xx.cpp
@@ -110,22 +110,18 @@ struct A {
 
 } // namespace cwg2858
 
-namespace cwg2877 { // cwg2877: no tentatively ready 2024-05-31
+namespace cwg2877 { // cwg2877: 19 tentatively ready 2024-05-31
 #if __cplusplus >= 202002L
 enum E { x };
 void f() {
   int E;
-  // FIXME: OK, names ::E
-  using enum E;
-  // since-cxx20-error@-1 {{unknown type name E}}
+  using enum E;   // OK, names ::E
 }
 using F = E;
 using enum F;     // OK, designates ::E
 template<class T> using EE = T;
 void g() {
-  // FIXME: OK, designates ::E
-  using enum EE<E>;
-  // since-cxx20-error@-1 {{using enum requires an enum or typedef name}}
+  using enum EE<E>;  // OK, designates ::E
 }
 #endif
 } // namespace cwg2877
diff --git a/clang/test/SemaCXX/cxx20-using-enum.cpp b/clang/test/SemaCXX/cxx20-using-enum.cpp
index d16df4e1b9d3c..14ef4b29925a1 100644
--- a/clang/test/SemaCXX/cxx20-using-enum.cpp
+++ b/clang/test/SemaCXX/cxx20-using-enum.cpp
@@ -148,13 +148,10 @@ template <int I> struct C {
   enum class D { d,
                  e,
                  f };
-  using enum D;
-
-  static constexpr int W = int(f) + I;
+  using enum D; // expected-error {{using-enum cannot name a dependent type}}
 };
 
 static_assert(C<2>::V == 4);
-static_assert(C<20>::W == 22);
 
 } // namespace Seven
 
@@ -241,6 +238,13 @@ TPLa<int> a;
 
 } // namespace Thirteen
 
+namespace Fourteen {
+template<typename T>
+int A = T();
+
+using enum A<int>; // expected-error {{A is not an enumerated type}}
+} // namespace Fourteen
+
 namespace GH58057 {
 struct Wrap {
 enum Things {
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index a7b1e652330e4..31f5dbde05908 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -15534,7 +15534,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/2621.html">2621</a></td>
     <td>C++23</td>
     <td>Kind of lookup for <TT>using enum</TT> declarations</td>
-    <td class="full" align="center">Clang 16</td>
+    <td title="Clang 19 implements 2024-05-31 resolution" align="center">Superseded by <a href="#2877">2877</a></td>
   </tr>
   <tr id="2622">
     <td><a href="https://cplusplus.github.io/CWG/issues/2622.html">2622</a></td>
@@ -17071,7 +17071,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
     <td><a href="https://cplusplus.github.io/CWG/issues/2877.html">2877</a></td>
     <td>tentatively ready</td>
     <td>Type-only lookup for <I>using-enum-declarator</I></td>
-    <td title="Clang does not implement 2024-05-31 resolution" align="center">Not Resolved*</td>
+    <td title="Clang 19 implements 2024-05-31 resolution" align="center">Not Resolved*</td>
   </tr>
   <tr class="open" id="2878">
     <td><a href="https://cplusplus.github.io/CWG/issues/2878.html">2878</a></td>

Endilll · 2024-06-13T11:53:25Z

clang/lib/Parse/ParseDeclCXX.cpp

+      SourceLocation IdentLoc = ConsumeToken();
+
+      ParsedType Type = Actions.getTypeName(
+          *IdentInfo, IdentLoc, getCurScope(), &SS, /*isClassName=*/true,


Note that getTypeName is now called with /*isClassName=*/true. I believe this is how we spell type-only lookup when templates are not involved.

Endilll · 2024-06-13T11:58:05Z

clang/lib/Parse/ParseDeclCXX.cpp

+                                                Type.get(), &SS);
+      } else {
+        Diag(Tok.getLocation(), diag::err_using_enum_not_enum)
+            << TemplateId->Name->getName()


This leads to suboptimal diagnostic when e.g. variable template is put after using enum: using enum A<int>; // expected-error {{A is not an enumerated type}}. I believe we should tell the user what we see instead of a mildly cryptic "enumeration type is expected." Is there a way to get that out of TemplateIdAnnotation that we have here?

We don't do it for base specifier, so i think this is fine (I suspect improvements would be a bit difficult, maybe by calling ClassifyName)

maybe by calling ClassifyName

Looks like ClassifyName doesn't provide more information than I currently have.

cor3ntin

Thanks for the patch Vlad!
The general direction looks correct, but i have reservations about the parsing of templates

clang/lib/Parse/ParseDeclCXX.cpp

cor3ntin · 2024-06-13T12:23:14Z

clang/lib/Parse/ParseDeclCXX.cpp

+                                                Type.get(), &SS);
+      } else {
+        Diag(Tok.getLocation(), diag::err_using_enum_not_enum)
+            << TemplateId->Name->getName()


We don't do it for base specifier, so i think this is fine (I suspect improvements would be a bit difficult, maybe by calling ClassifyName)

…code should be factored out

…andles `using enum`

shafik · 2024-06-20T23:23:06Z

clang/test/SemaCXX/cxx20-using-enum.cpp

+int A = T();
+
+using enum A<int>; // expected-error {{A is not an enumerated type}}
+} // namespace Fourteen


What does Fourteen stand for here or is it just a unique name?

Yes. If you look at the rest of the file, you'll see tests One through Thirteen.

shafik · 2024-06-20T23:26:59Z

clang/test/SemaCXX/cxx20-using-enum.cpp

-  using enum D;
-
-  static constexpr int W = int(f) + I;
+  using enum D; // expected-error {{using-enum cannot name a dependent type}}


It looks like we always got this wrong, did this fix just fall out of the implementation?

Yes. I first implemented the DR, ran the tests, and this one failed. Then I realized that this test was incorrect.

llvm#95399) This patch implements 2024-05-31 resolution of a tentatively ready issue [CWG2877](https://cplusplus.github.io/CWG/issues/2877.html) "Type-only lookup for using-enum-declarator", which supersedes earlier [CWG2621](https://cplusplus.github.io/CWG/issues/2621.html) "Kind of lookup for `using enum` declarations". Now we perform type-only lookup (not to be confused with type-only context) for `elaborated-enum-declarator`. This is the same kind of lookup that elaborated type specifiers and base specifiers undergo. I also found out (and fixed) that one of our existing tests claimed that a dependent type can be used in `elaborated-enum-declarator`, but that's not the case: > The [using-enum-declarator](http://eel.is/c++draft/enum.udecl#nt:using-enum-declarator) shall designate a non-dependent type with a reachable [enum-specifier](http://eel.is/c++draft/dcl.enum#nt:enum-specifier)[.](http://eel.is/c++draft/enum.udecl#1.sentence-2)

Endilll added 3 commits June 13, 2024 14:35

[clang] Implement CWG2877 "Type-only lookup for using-enum-declarator"

a7b6a8b

Merge remote-tracking branch 'upstream/main' into cwg2877

38fd3ff

Run clang-format

40ad680

Endilll added c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 13, 2024

Endilll requested review from cor3ntin, AaronBallman, urnathan and Sirraide June 13, 2024 11:51

llvmbot added the clang Clang issues not falling into any other category label Jun 13, 2024

Endilll commented Jun 13, 2024

View reviewed changes

Add a release notes entry

d6868d3

cor3ntin reviewed Jun 13, 2024

View reviewed changes

Endilll added 2 commits June 20, 2024 00:01

Add fixmes that similarities between using enum and base specifier …

c19bc28

…code should be factored out

Pass /*IsTypename*/=true to ParseOptionalCXXScopeSpecifier that h…

2cc8a66

…andles `using enum`

cor3ntin approved these changes Jun 20, 2024

View reviewed changes

shafik approved these changes Jun 20, 2024

View reviewed changes

Endilll merged commit bc4d50f into llvm:main Jun 21, 2024
8 checks passed

Endilll deleted the cwg2877 branch June 21, 2024 09:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang] Implement CWG2877 "Type-only lookup for using-enum-declarator" #95399

[clang] Implement CWG2877 "Type-only lookup for using-enum-declarator" #95399

Uh oh!

Endilll commented Jun 13, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jun 13, 2024

Uh oh!

Endilll Jun 13, 2024

Uh oh!

Endilll Jun 13, 2024

Uh oh!

cor3ntin Jun 13, 2024

Uh oh!

Endilll Jun 19, 2024

Uh oh!

cor3ntin left a comment

Uh oh!

Uh oh!

cor3ntin Jun 13, 2024

Uh oh!

shafik Jun 20, 2024

Uh oh!

Endilll Jun 21, 2024

Uh oh!

shafik Jun 20, 2024

Uh oh!

Endilll Jun 21, 2024

Uh oh!

Uh oh!

Uh oh!

[clang] Implement CWG2877 "Type-only lookup for using-enum-declarator" #95399

[clang] Implement CWG2877 "Type-only lookup for using-enum-declarator" #95399

Uh oh!

Conversation

Endilll commented Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cor3ntin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Endilll commented Jun 13, 2024 •

edited

Loading