[clang][DR2621] using enum NAME lookup fix

[urnathan]

Summary

Although using-enum's grammar is 'using elaborated-enum-specifier',
the lookup for the enum is ordinary lookup (and not the tagged-type
lookup that normally occurs wth an tagged-type specifier). Thus (a)
we can find typedefs and (b) do not find enum tags hidden by a non-tag
name (the struct stat thing).

This reimplements that part of using-enum handling, to address DR2621,
where clang's behaviour does not match std intent (and other
compilers).

[aaron.ballman]

Thank you for working on this! The precommit CI failures on Windows look to be unrelated. Btw, when you generate the patch, if you could add some more context to the patch (with -U9999) that would be appreciated -- it makes it easier to review when we can see the surrounding code as well.

You should add a release note for the changes as well. It looks like we've not updated the C++ DR status page to have the latest round of DRs, so no need to regenerate that page. But you should still add a test case to clang/test/CXX/drs/dr26xx.cpp with the appropriate markup (see other examples in that file/test directory) so that we get the correct status when we do go to regenerate that page.

clang/include/clang/Basic/DiagnosticSemaKinds.td

607–608

I don't think we need this one; we can use err_unknown_typename.

clang/lib/Parse/ParseDeclCXX.cpp

722–723

Spell out the types because they're not spelled out in the initialization.

clang/lib/Sema/SemaDeclCXX.cpp

11852

I think you should consider using ClassifyName() instead, as that will not only get you the parsed type information, but it handles things like typo correction, etc. Then you shouldn't need to worry about the diagnostics immediately below.

11862

These days we use dyn_cast_if_present instead.

[urnathan]

I forgot I had a git abbriev for the -U9999 thing, sorry about that.

I tried ClassifyName, but although it claimed to have a type, its getType method returned a ParsedType object that seemed broken -- its 'get' methof produced a thing not recognized as valid QualType and whose dump method seg faulted when called.

[urnathan]

clang/lib/Sema/SemaDeclCXX.cpp

11852

See comments, tried & failed.

[aaron.ballman]

In D134283#3808446, @urnathan wrote:

I forgot I had a git abbriev for the -U9999 thing, sorry about that.

No worries!

I tried ClassifyName, but although it claimed to have a type, its getType method returned a ParsedType object that seemed broken -- its 'get' methof produced a thing not recognized as valid QualType and whose dump method seg faulted when called.

Wha? That seems rather concerning. What does getKind() return? (We assert that the kind is correct when trying to get the data out of the object, but perhaps you weren't using an asserts build?)

[urnathan]

Wha? That seems rather concerning. What does getKind() return? (We assert that the kind is correct when trying to get the data out of the object, but perhaps you weren't using an asserts build?)

getKind() says it's a type and getType() returns a thing that could be a (nonnull) ParsedType. That objects get call returns a QualType I am unable to inspect, and that got printed out in diagnostics as no characters (when it failed the getAsTaggedType call). I find the wrapped pointer objects clang seems to love hard to inspect. I tried tracing through ClassifyName to see how it was constructing the type, but got somewhat lost in a set of lambdas. FWIW ClassifyName seems to be mostly employed in figuring out tentative parse disambiguation.

I am using unoptimized assert builds :)

[aaron.ballman]

In D134283#3811198, @urnathan wrote:

Wha? That seems rather concerning. What does getKind() return? (We assert that the kind is correct when trying to get the data out of the object, but perhaps you weren't using an asserts build?)

getKind() says it's a type and getType() returns a thing that could be a (nonnull) ParsedType. That objects get call returns a QualType I am unable to inspect, and that got printed out in diagnostics as no characters (when it failed the getAsTaggedType call). I find the wrapped pointer objects clang seems to love hard to inspect. I tried tracing through ClassifyName to see how it was constructing the type, but got somewhat lost in a set of lambdas. FWIW ClassifyName seems to be mostly employed in figuring out tentative parse disambiguation.

I am using unoptimized assert builds :)

Well huh, TIL! :-D

I'm happy with the state of things, so LGTM, but I did spot the need for a comment to be added to a test case that you should fix when landing, so the DR scraping script works properly.

clang/test/CXX/drs/dr26xx.cpp

3

[aaron.ballman]

LGTM!

[junaire]

clang/lib/Parse/ParseDeclCXX.cpp

725

minor suggestion: will it be easier to read if you add comments like /*ParsedType=*/nullptr, /*ObjectHasErrors=*/false and etc?

[shafik]

clang/lib/Parse/ParseDeclCXX.cpp

725

We should be doing this it is part of the style guide.

This can also be verified via clang-tidy bugprone-argument-comment

[h-vetinari]

Seems DR2621 is not yet listed in https://github.com/llvm/llvm-project/blob/main/clang/www/cxx_dr_status.html (but if it were, it should be updated...)

[urnathan]

Here's an update, just to check I've got all the comments addressed. It doesn't appear this DR has made it to the ready status in cwg-index, so ./make_cxx_dr_status barfed.

(For avoidance of doubt, I was at the CWG mtg last week when we discussed this and moved it to Ready).

ETA: I'll commit in a few days, if there are not further comments.

[aaron.ballman]

Reaffirming my LG but there was a minor formatting thing for the release notes that you can fix when landing. Thank you! (Btw, I was also at that CWG and can confirm this matches the resolution of the DR as discussed.)

clang/docs/ReleaseNotes.rst

359

clang/docs/ReleaseNotes.rst

- Skip rebuilding lambda expressions in arguments of immediate invocations.
   `Issue 51695 <https://github.com/llvm/llvm-project/issues/51695>`_,
   `Issue 50455 <https://github.com/llvm/llvm-project/issues/50455>`_,
   `Issue 54872 <https://github.com/llvm/llvm-project/issues/54872>`_,
   `Issue 54587 <https://github.com/llvm/llvm-project/issues/54587>`_.
 - Clang now correctly delays the instantiation of function constraints until
   the time of checking, which should now allow the libstdc++ ranges implementation
   to work for at least trivial examples.  This fixes
   `Issue 44178 <https://github.com/llvm/llvm-project/issues/44178>`_.
+- Clang implements DR2621, correcting a defect in ``using enum`` handling.  The
+  name is found via ordinary lookup so typedefs are found.
 
 C++2b Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 - Support label at end of compound statement (`P2324 <https://wg21.link/p2324r2>`_).
 
 CUDA/HIP Language Changes in Clang
 ----------------------------------

clang/include/clang/Basic/DiagnosticParseKinds.td

 def err_using_namespace_in_class : Error<
   "'using namespace' is not allowed in classes">;
 def warn_cxx17_compat_using_enum_declaration : Warning<
   "using enum declaration is incompatible with C++ standards before C++20">,
   InGroup<CXXPre20Compat>, DefaultIgnore;
 def ext_using_enum_declaration : ExtWarn<
   "using enum declaration is a C++20 extension">,
   InGroup<CXX20>;
+def err_using_enum_expect_identifier : Error<
+  "using enum %select{requires an enum or typedef name|"
+  "does not permit an elaborated enum specifier}0">;
 def err_constructor_bad_name : Error<
   "missing return type for function %0; did you mean the constructor name %1?">;
 def err_destructor_tilde_identifier : Error<
   "expected a class name after '~' to name a destructor">;
 def err_destructor_tilde_scope : Error<
   "'~' in destructor name should be after nested name specifier">;
 def err_destructor_template_id : Error<
   "destructor name %0 does not refer to a template">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_using_decl_constructor_not_in_direct_base : Error<
   "%0 is not a direct base of %1, cannot inherit constructors">;
 def err_using_decl_can_not_refer_to_class_member : Error<
   "using declaration cannot refer to class member">;
 def warn_cxx17_compat_using_decl_class_member_enumerator : Warning<
   "member using declaration naming a non-member enumerator is incompatible "
   "with C++ standards before C++20">, InGroup<CXXPre20Compat>, DefaultIgnore;
 def err_using_enum_is_dependent : Error<
   "using-enum cannot name a dependent type">;
+def err_using_enum_not_enum : Error<
+  "%0 is not an enumerated type">;
 def err_ambiguous_inherited_constructor : Error<
   "constructor of %0 inherited from multiple base class subobjects">;
 def note_ambiguous_inherited_constructor_using : Note<
   "inherited from base class %0 here">;
 def note_using_decl_class_member_workaround : Note<
   "use %select{an alias declaration|a typedef declaration|a reference|"
   "a const variable|a constexpr variable}0 instead">;
 def err_using_decl_can_not_refer_to_namespace : Error<

 def err_using_decl_redeclaration_expansion : Error<
   "using declaration pack expansion at block scope produces multiple values">;
 def err_use_of_empty_using_if_exists : Error<
   "reference to unresolved using declaration">;
 def note_empty_using_if_exists_here : Note<
   "using declaration annotated with 'using_if_exists' here">;
 def err_using_if_exists_on_ctor : Error<
   "'using_if_exists' attribute cannot be applied to an inheriting constructor">;
 def err_using_enum_decl_redeclaration : Error<
   "redeclaration of using-enum declaration">;
 def note_using_enum_decl : Note<"%select{|previous }0using-enum declaration">;
 
 def warn_access_decl_deprecated : Warning<
   "access declarations are deprecated; use using declarations instead">,
   InGroup<Deprecated>;
 def err_access_decl : Error<
   "ISO C++11 does not allow access declarations; "
   "use using declarations instead">;

clang/include/clang/Sema/Sema.h

public:
 
   Decl *ActOnUsingDeclaration(Scope *CurScope, AccessSpecifier AS,
                               SourceLocation UsingLoc,
                               SourceLocation TypenameLoc, CXXScopeSpec &SS,
                               UnqualifiedId &Name, SourceLocation EllipsisLoc,
                               const ParsedAttributesView &AttrList);
   Decl *ActOnUsingEnumDeclaration(Scope *CurScope, AccessSpecifier AS,
                                   SourceLocation UsingLoc,
-                                  SourceLocation EnumLoc, const DeclSpec &);
+                                  SourceLocation EnumLoc,
+                                  SourceLocation IdentLoc, IdentifierInfo &II,
+                                  CXXScopeSpec *SS = nullptr);
   Decl *ActOnAliasDeclaration(Scope *CurScope, AccessSpecifier AS,
                               MultiTemplateParamsArg TemplateParams,
                               SourceLocation UsingLoc, UnqualifiedId &Name,
                               const ParsedAttributesView &AttrList,
                               TypeResult Type, Decl *DeclFromDeclSpec);
 
   /// BuildCXXConstructExpr - Creates a complete call to a constructor,
   /// including handling of its default argument expressions.

clang/lib/Parse/ParseDeclCXX.cpp

 ///     using-declarator-list: [C++98-14]
 ///       using-declarator
 ///
 ///     alias-declaration: C++11 [dcl.dcl]p1
 ///       'using' identifier attribute-specifier-seq[opt] = type-id ;
 ///
 ///     using-enum-declaration: [C++20, dcl.enum]
 ///       'using' elaborated-enum-specifier ;
+///       The terminal name of the elaborated-enum-specifier undergoes
+///       ordinary lookup
 ///
 ///     elaborated-enum-specifier:
 ///       'enum' nested-name-specifier[opt] identifier
 Parser::DeclGroupPtrTy Parser::ParseUsingDeclaration(
     DeclaratorContext Context, const ParsedTemplateInfo &TemplateInfo,
     SourceLocation UsingLoc, SourceLocation &DeclEnd,
     ParsedAttributes &PrefixAttrs, AccessSpecifier AS) {
   SourceLocation UELoc;
   bool InInitStatement = Context == DeclaratorContext::SelectionInit ||
                          Context == DeclaratorContext::ForInit;
 
   if (TryConsumeToken(tok::kw_enum, UELoc) && !InInitStatement) {
     // C++20 using-enum
     Diag(UELoc, getLangOpts().CPlusPlus20
                     ? diag::warn_cxx17_compat_using_enum_declaration
                     : diag::ext_using_enum_declaration);
 
     DiagnoseCXX11AttributeExtension(PrefixAttrs);
 
-    DeclSpec DS(AttrFactory);
-    ParseEnumSpecifier(UELoc, DS, TemplateInfo, AS,
-                       // DSC_trailing has the semantics we desire
-                       DeclSpecContext::DSC_trailing);
-
     if (TemplateInfo.Kind) {
       SourceRange R = TemplateInfo.getSourceRange();
       Diag(UsingLoc, diag::err_templated_using_directive_declaration)
           << 1 /* declaration */ << R << FixItHint::CreateRemoval(R);
+      SkipUntil(tok::semi);
+      return nullptr;
+    }
+    CXXScopeSpec SS;
+    if (ParseOptionalCXXScopeSpecifier(SS, /*ParsedType=*/nullptr,
+                                       /*ObectHasErrors=*/false,
+                                       /*EnteringConttext=*/false,
+                                       /*MayBePseudoDestructor=*/nullptr,
+                                       /*IsTypename=*/false,
+                                       /*IdentifierInfo=*/nullptr,
+                                       /*OnlyNamespace=*/false,
+                                       /*InUsingDeclaration=*/true)) {
+      SkipUntil(tok::semi);
+      return nullptr;
+    }
 
+    if (Tok.is(tok::code_completion)) {
+      cutOffParsing();
+      Actions.CodeCompleteUsing(getCurScope());
+      return nullptr;
+    }
+
+    if (!Tok.is(tok::identifier)) {
+      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;
     }
 
-    Decl *UED = Actions.ActOnUsingEnumDeclaration(getCurScope(), AS, UsingLoc,
-                                                  UELoc, DS);
     DeclEnd = Tok.getLocation();
     if (ExpectAndConsume(tok::semi, diag::err_expected_after,
                          "using-enum declaration"))
       SkipUntil(tok::semi);
 
     return Actions.ConvertDeclToDeclGroup(UED);
   }
 

clang/lib/Sema/SemaDeclCXX.cpp

NamedDecl *UD =
                             AttrList.hasAttribute(ParsedAttr::AT_UsingIfExists));
   if (UD)
     PushOnScopeChains(UD, S, /*AddToContext*/ false);
 
   return UD;
 }
 
 Decl *Sema::ActOnUsingEnumDeclaration(Scope *S, AccessSpecifier AS,
                                       SourceLocation UsingLoc,
                                       SourceLocation EnumLoc,
-                                      const DeclSpec &DS) {
-  switch (DS.getTypeSpecType()) {
-  case DeclSpec::TST_error:
-    // This will already have been diagnosed
+                                      SourceLocation IdentLoc,
+                                      IdentifierInfo &II, CXXScopeSpec *SS) {
+  assert(!SS->isInvalid() && "ScopeSpec is invalid");
+  ParsedType TypeRep = getTypeName(II, IdentLoc, S, SS);
+  if (!TypeRep) {
+    Diag(IdentLoc, SS && isDependentScopeSpecifier(*SS)
+                       ? diag::err_using_enum_is_dependent
+                       : diag::err_unknown_typename)
+        << II.getName()
+        << SourceRange(SS ? SS->getBeginLoc() : IdentLoc, IdentLoc);
     return nullptr;
+  }
 
-  case DeclSpec::TST_enum:
-    break;
-
-  case DeclSpec::TST_typename:
-    Diag(DS.getTypeSpecTypeLoc(), diag::err_using_enum_is_dependent);
+  auto *Enum = dyn_cast_if_present<EnumDecl>(TypeRep.get()->getAsTagDecl());
+  if (!Enum) {
+    Diag(IdentLoc, diag::err_using_enum_not_enum) << TypeRep.get();
     return nullptr;
-
-  default:
-    llvm_unreachable("unexpected DeclSpec type");
   }
 
-  // As with enum-decls, we ignore attributes for now.
-  auto *Enum = cast<EnumDecl>(DS.getRepAsDecl());
   if (auto *Def = Enum->getDefinition())
     Enum = Def;
 
-  auto *UD = BuildUsingEnumDeclaration(S, AS, UsingLoc, EnumLoc,
-                                       DS.getTypeSpecTypeNameLoc(), Enum);
+  auto *UD =
+      BuildUsingEnumDeclaration(S, AS, UsingLoc, EnumLoc, IdentLoc, Enum);
   if (UD)
     PushOnScopeChains(UD, S, /*AddToContext*/ false);
 
   return UD;
 }
 
 /// Determine whether a using declaration considers the given
 /// declarations as "equivalent", e.g., if they are redeclarations of

clang/test/CXX/drs/dr26xx.cpp

 // RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify
 
+namespace dr2621 { // dr2621: yes
+enum class E { a };
+namespace One {
+using E_t = E;
+using enum E_t; // typedef ok
+auto v = a;
+}
+namespace Two {
+using dr2621::E;
+int E; // we see this
+using enum E; // expected-error {{unknown type name E}}
+}
+}
+
 namespace dr2628 { // dr2628: yes
 
 template <bool A = false, bool B = false>
 struct foo {
   constexpr foo() requires (!A && !B) = delete; // #DR2628_CTOR
   constexpr foo() requires (A || B) = delete;
 };
 
 void f() {
   foo fooable; // expected-error {{call to deleted}}
   // expected-note@#DR2628_CTOR {{marked deleted here}}
 }
 
 }

clang/test/CodeCompletion/using-enum.cpp

 enum class AAA { X, Y, Z };
 
 namespace N2 {
   using enum AAA;
-  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:4:14 %s | FileCheck -check-prefix=CHECK-CC1 %s
+  // RUN: %clang_cc1 -std=c++20 -fsyntax-only -code-completion-at=%s:4:14 %s | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1: COMPLETION: AAA
 };

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

 // RUN: %clang_cc1 -std=c++20 -verify %s
 
 namespace GH57347 {
 namespace A {}
 
 void f() {
-  using enum A::+; // expected-error {{expected identifier}}
-  using enum; // expected-error {{expected identifier or '{'}}
-  using enum class; // expected-error {{expected identifier or '{'}}
-  using enum : blah; // expected-error {{unknown type name 'blah'}} expected-error {{unnamed enumeration must be a definition}}
+  using enum A::+; // expected-error {{using enum requires an enum or typedef name}}
+  using enum; // expected-error {{using enum requires an enum or typedef name}}
+  using enum class; // expected-error {{using enum requires an enum or typedef name}}
+  using enum enum q; // expected-error {{using enum does not permit an elaborated enum specifier}}
 }
 }

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

 // RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
 
 // p1099 'using enum ELABORATED-ENUM-SPECIFIER ;'
 
 namespace One {
 namespace Bob {
 enum A { a, // expected-note{{declared here}}
          b,
          c };
-class C; // expected-note{{previous use}}
+class C;
 enum class D : int;
 enum class D { d,
                e,
                f };
 enum class D : int;
 } // namespace Bob
 
 using enum Bob::A;
 #if __cplusplus < 202002
 // expected-warning@-2{{is a C++20 extension}}
 #endif
-using enum Bob::B; // expected-error{{no enum named 'B'}}
+using enum Bob::B; // expected-error{{unknown type name B}}
 #if __cplusplus < 202002
 // expected-warning@-2{{is a C++20 extension}}
 #endif
-using enum Bob::C; // expected-error{{tag type that does not match}}
+using enum Bob::C; // expected-error{{'Bob::C' is not an enumerated type}}
 #if __cplusplus < 202002
 // expected-warning@-2{{is a C++20 extension}}
 #endif
 auto v = a;
 
 A g; // expected-error{{unknown type name 'A'}}
 
 int A;
 
 using enum Bob::D;
 #if __cplusplus < 202002
 // expected-warning@-2{{is a C++20 extension}}
 #endif
+
+void DR2621() {
+  using A_t = Bob::A;
+  using enum A_t;
+#if __cplusplus < 202002
+// expected-warning@-2{{is a C++20 extension}}
+#endif
+  A_t x = a;
+}
+
 } // namespace One
 
 namespace Two {
 namespace Kevin {
 enum class B { d,
                e,
                f };
 }