Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 21, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a7b6a8b
[clang] Implement CWG2877 "Type-only lookup for using-enum-declarator"
Endilll Jun 13, 2024
38fd3ff
Merge remote-tracking branch 'upstream/main' into cwg2877
Endilll Jun 13, 2024
40ad680
Run clang-format
Endilll Jun 13, 2024
d6868d3
Add a release notes entry
Endilll Jun 13, 2024
c19bc28
Add fixmes that similarities between `using enum` and base specifier …
Endilll Jun 19, 2024
2cc8a66
Pass `/*IsTypename*/=true` to `ParseOptionalCXXScopeSpecifier` that h…
Endilll Jun 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,9 @@ Resolutions to C++ Defect Reports
- P0522 implementation is enabled by default in all language versions, and
provisional wording for CWG2398 is implemented.

- Clang now performs type-only lookup for the name in ``using enum`` declaration.
(`CWG2877: Type-only lookup for using-enum-declarator <https://cplusplus.github.io/CWG/issues/2877.html>`_).

- Clang now requires a template argument list after a template keyword.
(`CWG96: Syntactic disambiguation using the template keyword <https://cplusplus.github.io/CWG/issues/96.html>`_).

Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
49 changes: 42 additions & 7 deletions clang/lib/Parse/ParseDeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -724,7 +724,7 @@ Parser::DeclGroupPtrTy Parser::ParseUsingDeclaration(
/*ObectHasErrors=*/false,
/*EnteringConttext=*/false,
/*MayBePseudoDestructor=*/nullptr,
/*IsTypename=*/false,
/*IsTypename=*/true,
/*IdentifierInfo=*/nullptr,
/*OnlyNamespace=*/false,
/*InUsingDeclaration=*/true)) {
Expand All @@ -738,16 +738,49 @@ Parser::DeclGroupPtrTy Parser::ParseUsingDeclaration(
return nullptr;
}

if (!Tok.is(tok::identifier)) {
Decl *UED = nullptr;

// FIXME: identifier and annot_template_id handling is very similar to
// ParseBaseTypeSpecifier. It should be factored out into a function.
if (Tok.is(tok::identifier)) {
IdentifierInfo *IdentInfo = Tok.getIdentifierInfo();
SourceLocation IdentLoc = ConsumeToken();

ParsedType Type = Actions.getTypeName(
*IdentInfo, IdentLoc, getCurScope(), &SS, /*isClassName=*/true,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

/*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()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe by calling ClassifyName

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

<< 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;
Expand Down Expand Up @@ -1403,6 +1436,8 @@ TypeResult Parser::ParseBaseTypeSpecifier(SourceLocation &BaseLoc,
}

// Check whether we have a template-id that names a type.
// FIXME: identifier and annot_template_id handling in ParseUsingDeclaration
// work very similarly. It should be refactored into a separate function.
if (Tok.is(tok::annot_template_id)) {
TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
if (TemplateId->mightBeType()) {
Expand Down
21 changes: 11 additions & 10 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 2 additions & 3 deletions clang/test/CXX/drs/cwg26xx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
10 changes: 3 additions & 7 deletions clang/test/CXX/drs/cwg28xx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions clang/test/SemaCXX/cxx20-using-enum.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

};

static_assert(C<2>::V == 4);
static_assert(C<20>::W == 22);

} // namespace Seven

Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


namespace GH58057 {
struct Wrap {
enum Things {
Expand Down
4 changes: 2 additions & 2 deletions clang/www/cxx_dr_status.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
Loading