Skip to content

Commit

Permalink
[Clang][Sema] Diagnose friend declarations with enum elaborated-type-…
Browse files Browse the repository at this point in the history
…specifier in all language modes (#80171)

According to [dcl.type.elab] p4:
> If an _elaborated-type-specifier_ appears with the `friend` specifier
as an entire _member-declaration_, the _member-declaration_ shall have
one of the following forms:
>     `friend` _class-key_ _nested-name-specifier_(opt) _identifier_ `;`
>     `friend` _class-key_ _simple-template-id_ `;`
> `friend` _class-key_ _nested-name-specifier_ `template`(opt)
_simple-template-id_ `;`

Notably absent from this list is the `enum` form of an
_elaborated-type-specifier_ "`enum` _nested-name-specifier_(opt)
_identifier_", which appears to be intentional per the resolution of
CWG2363.

Most major implementations accept these declarations, so the diagnostic
is a pedantic warning across all C++ versions.

In addition to the trivial cases previously diagnosed in C++98, we now
diagnose cases where the _elaborated-type-specifier_ has a dependent
_nested-name-specifier_:
```
template<typename T>
struct A
{
    enum class E;
};

struct B
{
    template<typename T>
    friend enum A<T>::E; // pedantic warning: elaborated enumeration type cannot be a friend
};

template<typename T>
struct C
{
    friend enum T::E;  // pedantic warning: elaborated enumeration type cannot be a friend
};
```
  • Loading branch information
sdkrystian committed Feb 13, 2024
1 parent 1b65742 commit 3a48630
Show file tree
Hide file tree
Showing 19 changed files with 185 additions and 146 deletions.
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ Improvements to Clang's diagnostics
- The ``-Wshorten-64-to-32`` diagnostic is now grouped under ``-Wimplicit-int-conversion`` instead
of ``-Wconversion``. Fixes `#69444 <https://github.com/llvm/llvm-project/issues/69444>`_.

- Clang now diagnoses friend declarations with an ``enum`` elaborated-type-specifier in language modes after C++98.

Improvements to Clang's time-trace
----------------------------------

Expand Down
8 changes: 4 additions & 4 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -1637,10 +1637,10 @@ def err_inline_namespace_std : Error<
def err_unexpected_friend : Error<
"friends can only be classes or functions">;
def ext_enum_friend : ExtWarn<
"befriending enumeration type %0 is a C++11 extension">, InGroup<CXX11>;
def warn_cxx98_compat_enum_friend : Warning<
"befriending enumeration type %0 is incompatible with C++98">,
InGroup<CXX98Compat>, DefaultIgnore;
"elaborated enum specifier cannot be declared as a friend">,
InGroup<DiagGroup<"friend-enum">>;
def note_enum_friend : Note<
"remove 'enum%select{| struct| class}0' to befriend an enum">;
def ext_nonclass_type_friend : ExtWarn<
"non-class friend type %0 is a C++11 extension">, InGroup<CXX11>;
def warn_cxx98_compat_nonclass_type_friend : Warning<
Expand Down
13 changes: 6 additions & 7 deletions clang/include/clang/Sema/DeclSpec.h
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,7 @@ class DeclSpec {
// FIXME: Attributes should be included here.
};

enum FriendSpecified : bool {
No,
Yes,
};
enum FriendSpecified : bool { No, Yes };

private:
// storage-class-specifier
Expand Down Expand Up @@ -400,7 +397,7 @@ class DeclSpec {

// friend-specifier
LLVM_PREFERRED_TYPE(bool)
unsigned Friend_specified : 1;
unsigned FriendSpecifiedFirst : 1;

// constexpr-specifier
LLVM_PREFERRED_TYPE(ConstexprSpecKind)
Expand Down Expand Up @@ -491,7 +488,7 @@ class DeclSpec {
TypeSpecPipe(false), TypeSpecSat(false), ConstrainedAuto(false),
TypeQualifiers(TQ_unspecified), FS_inline_specified(false),
FS_forceinline_specified(false), FS_virtual_specified(false),
FS_noreturn_specified(false), Friend_specified(false),
FS_noreturn_specified(false), FriendSpecifiedFirst(false),
ConstexprSpecifier(
static_cast<unsigned>(ConstexprSpecKind::Unspecified)),
Attrs(attrFactory), writtenBS(), ObjCQualifiers(nullptr) {}
Expand Down Expand Up @@ -818,9 +815,11 @@ class DeclSpec {
const char *&PrevSpec, unsigned &DiagID);

FriendSpecified isFriendSpecified() const {
return static_cast<FriendSpecified>(Friend_specified);
return static_cast<FriendSpecified>(FriendLoc.isValid());
}

bool isFriendSpecifiedFirst() const { return FriendSpecifiedFirst; }

SourceLocation getFriendSpecLoc() const { return FriendLoc; }

bool isModulePrivateSpecified() const { return ModulePrivateLoc.isValid(); }
Expand Down
3 changes: 0 additions & 3 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -8039,9 +8039,6 @@ class Sema final {
SourceLocation RParenLoc, bool Failed);
void DiagnoseStaticAssertDetails(const Expr *E);

FriendDecl *CheckFriendTypeDecl(SourceLocation LocStart,
SourceLocation FriendLoc,
TypeSourceInfo *TSInfo);
Decl *ActOnFriendTypeDecl(Scope *S, const DeclSpec &DS,
MultiTemplateParamsArg TemplateParams);
NamedDecl *ActOnFriendFunctionDecl(Scope *S, Declarator &D,
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/Parse/ParseTentative.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ bool Parser::isCXXDeclarationStatement(
getCurScope(), *II, Tok.getLocation(), SS, /*Template=*/nullptr);
if (Actions.isCurrentClassName(*II, getCurScope(), &SS) ||
isDeductionGuide) {
if (isConstructorDeclarator(/*Unqualified=*/SS.isEmpty(),
isDeductionGuide,
DeclSpec::FriendSpecified::No))
if (isConstructorDeclarator(
/*Unqualified=*/SS.isEmpty(), isDeductionGuide,
/*IsFriend=*/DeclSpec::FriendSpecified::No))
return true;
} else if (SS.isNotEmpty()) {
// If the scope is not empty, it could alternatively be something like
Expand Down
9 changes: 2 additions & 7 deletions clang/lib/Sema/DeclSpec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1102,18 +1102,13 @@ bool DeclSpec::setFunctionSpecNoreturn(SourceLocation Loc,

bool DeclSpec::SetFriendSpec(SourceLocation Loc, const char *&PrevSpec,
unsigned &DiagID) {
if (Friend_specified) {
if (isFriendSpecified()) {
PrevSpec = "friend";
// Keep the later location, so that we can later diagnose ill-formed
// declarations like 'friend class X friend;'. Per [class.friend]p3,
// 'friend' must be the first token in a friend declaration that is
// not a function declaration.
FriendLoc = Loc;
DiagID = diag::warn_duplicate_declspec;
return true;
}

Friend_specified = true;
FriendSpecifiedFirst = isEmpty();
FriendLoc = Loc;
return false;
}
Expand Down
20 changes: 20 additions & 0 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17264,6 +17264,26 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc,
return true;
}

if (TUK == TUK_Friend && Kind == TagTypeKind::Enum) {
// C++23 [dcl.type.elab]p4:
// If an elaborated-type-specifier appears with the friend specifier as
// an entire member-declaration, the member-declaration shall have one
// of the following forms:
// friend class-key nested-name-specifier(opt) identifier ;
// friend class-key simple-template-id ;
// friend class-key nested-name-specifier template(opt)
// simple-template-id ;
//
// Since enum is not a class-key, so declarations like "friend enum E;"
// are ill-formed. Although CWG2363 reaffirms that such declarations are
// invalid, most implementations accept so we issue a pedantic warning.
Diag(KWLoc, diag::ext_enum_friend) << FixItHint::CreateRemoval(
ScopedEnum ? SourceRange(KWLoc, ScopedEnumKWLoc) : KWLoc);
assert(ScopedEnum || !ScopedEnumUsesClassTag);
Diag(KWLoc, diag::note_enum_friend)
<< (ScopedEnum + ScopedEnumUsesClassTag);
}

// Figure out the underlying type if this a enum declaration. We need to do
// this early, because it's needed to detect if this is an incompatible
// redeclaration.
Expand Down
137 changes: 38 additions & 99 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17545,79 +17545,6 @@ Decl *Sema::BuildStaticAssertDeclaration(SourceLocation StaticAssertLoc,
return Decl;
}

/// Perform semantic analysis of the given friend type declaration.
///
/// \returns A friend declaration that.
FriendDecl *Sema::CheckFriendTypeDecl(SourceLocation LocStart,
SourceLocation FriendLoc,
TypeSourceInfo *TSInfo) {
assert(TSInfo && "NULL TypeSourceInfo for friend type declaration");

QualType T = TSInfo->getType();
SourceRange TypeRange = TSInfo->getTypeLoc().getSourceRange();

// C++03 [class.friend]p2:
// An elaborated-type-specifier shall be used in a friend declaration
// for a class.*
//
// * The class-key of the elaborated-type-specifier is required.
if (!CodeSynthesisContexts.empty()) {
// Do not complain about the form of friend template types during any kind
// of code synthesis. For template instantiation, we will have complained
// when the template was defined.
} else {
if (!T->isElaboratedTypeSpecifier()) {
// If we evaluated the type to a record type, suggest putting
// a tag in front.
if (const RecordType *RT = T->getAs<RecordType>()) {
RecordDecl *RD = RT->getDecl();

SmallString<16> InsertionText(" ");
InsertionText += RD->getKindName();

Diag(TypeRange.getBegin(),
getLangOpts().CPlusPlus11 ?
diag::warn_cxx98_compat_unelaborated_friend_type :
diag::ext_unelaborated_friend_type)
<< (unsigned) RD->getTagKind()
<< T
<< FixItHint::CreateInsertion(getLocForEndOfToken(FriendLoc),
InsertionText);
} else {
Diag(FriendLoc,
getLangOpts().CPlusPlus11 ?
diag::warn_cxx98_compat_nonclass_type_friend :
diag::ext_nonclass_type_friend)
<< T
<< TypeRange;
}
} else if (T->getAs<EnumType>()) {
Diag(FriendLoc,
getLangOpts().CPlusPlus11 ?
diag::warn_cxx98_compat_enum_friend :
diag::ext_enum_friend)
<< T
<< TypeRange;
}

// C++11 [class.friend]p3:
// A friend declaration that does not declare a function shall have one
// of the following forms:
// friend elaborated-type-specifier ;
// friend simple-type-specifier ;
// friend typename-specifier ;
if (getLangOpts().CPlusPlus11 && LocStart != FriendLoc)
Diag(FriendLoc, diag::err_friend_not_first_in_declaration) << T;
}

// If the type specifier in a friend declaration designates a (possibly
// cv-qualified) class type, that class is declared as a friend; otherwise,
// the friend declaration is ignored.
return FriendDecl::Create(Context, CurContext,
TSInfo->getTypeLoc().getBeginLoc(), TSInfo,
FriendLoc);
}

/// Handle a friend tag declaration where the scope specifier was
/// templated.
DeclResult Sema::ActOnTemplatedFriendTag(
Expand Down Expand Up @@ -17755,6 +17682,7 @@ DeclResult Sema::ActOnTemplatedFriendTag(
Decl *Sema::ActOnFriendTypeDecl(Scope *S, const DeclSpec &DS,
MultiTemplateParamsArg TempParams) {
SourceLocation Loc = DS.getBeginLoc();
SourceLocation FriendLoc = DS.getFriendSpecLoc();

assert(DS.isFriendSpecified());
assert(DS.getStorageClassSpec() == DeclSpec::SCS_unspecified);
Expand All @@ -17766,9 +17694,10 @@ Decl *Sema::ActOnFriendTypeDecl(Scope *S, const DeclSpec &DS,
// friend simple-type-specifier ;
// friend typename-specifier ;
//
// Any declaration with a type qualifier does not have that form. (It's
// legal to specify a qualified type as a friend, you just can't write the
// keywords.)
// If the friend keyword isn't first, or if the declarations has any type
// qualifiers, then the declaration doesn't have that form.
if (getLangOpts().CPlusPlus11 && !DS.isFriendSpecifiedFirst())
Diag(FriendLoc, diag::err_friend_not_first_in_declaration);
if (DS.getTypeQualifiers()) {
if (DS.getTypeQualifiers() & DeclSpec::TQ_const)
Diag(DS.getConstSpecLoc(), diag::err_friend_decl_spec) << "const";
Expand All @@ -17795,24 +17724,35 @@ Decl *Sema::ActOnFriendTypeDecl(Scope *S, const DeclSpec &DS,
if (DiagnoseUnexpandedParameterPack(Loc, TSI, UPPC_FriendDeclaration))
return nullptr;

// This is definitely an error in C++98. It's probably meant to
// be forbidden in C++0x, too, but the specification is just
// poorly written.
//
// The problem is with declarations like the following:
// template <T> friend A<T>::foo;
// where deciding whether a class C is a friend or not now hinges
// on whether there exists an instantiation of A that causes
// 'foo' to equal C. There are restrictions on class-heads
// (which we declare (by fiat) elaborated friend declarations to
// be) that makes this tractable.
//
// FIXME: handle "template <> friend class A<T>;", which
// is possibly well-formed? Who even knows?
if (TempParams.size() && !T->isElaboratedTypeSpecifier()) {
Diag(Loc, diag::err_tagless_friend_type_template)
<< DS.getSourceRange();
return nullptr;
if (!T->isElaboratedTypeSpecifier()) {
if (TempParams.size()) {
// C++23 [dcl.pre]p5:
// In a simple-declaration, the optional init-declarator-list can be
// omitted only when declaring a class or enumeration, that is, when
// the decl-specifier-seq contains either a class-specifier, an
// elaborated-type-specifier with a class-key, or an enum-specifier.
//
// The declaration of a template-declaration or explicit-specialization
// is never a member-declaration, so this must be a simple-declaration
// with no init-declarator-list. Therefore, this is ill-formed.
Diag(Loc, diag::err_tagless_friend_type_template) << DS.getSourceRange();
return nullptr;
} else if (const RecordDecl *RD = T->getAsRecordDecl()) {
SmallString<16> InsertionText(" ");
InsertionText += RD->getKindName();

Diag(Loc, getLangOpts().CPlusPlus11
? diag::warn_cxx98_compat_unelaborated_friend_type
: diag::ext_unelaborated_friend_type)
<< (unsigned)RD->getTagKind() << T
<< FixItHint::CreateInsertion(getLocForEndOfToken(FriendLoc),
InsertionText);
} else {
Diag(FriendLoc, getLangOpts().CPlusPlus11
? diag::warn_cxx98_compat_nonclass_type_friend
: diag::ext_nonclass_type_friend)
<< T << DS.getSourceRange();
}
}

// C++98 [class.friend]p1: A friend of a class is a function
Expand All @@ -17828,12 +17768,11 @@ Decl *Sema::ActOnFriendTypeDecl(Scope *S, const DeclSpec &DS,

Decl *D;
if (!TempParams.empty())
D = FriendTemplateDecl::Create(Context, CurContext, Loc,
TempParams,
TSI,
DS.getFriendSpecLoc());
D = FriendTemplateDecl::Create(Context, CurContext, Loc, TempParams, TSI,
FriendLoc);
else
D = CheckFriendTypeDecl(Loc, DS.getFriendSpecLoc(), TSI);
D = FriendDecl::Create(Context, CurContext, TSI->getTypeLoc().getBeginLoc(),
TSI, FriendLoc);

if (!D)
return nullptr;
Expand Down
7 changes: 2 additions & 5 deletions clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1407,11 +1407,8 @@ Decl *TemplateDeclInstantiator::VisitFriendDecl(FriendDecl *D) {
if (!InstTy)
return nullptr;

FriendDecl *FD = SemaRef.CheckFriendTypeDecl(D->getBeginLoc(),
D->getFriendLoc(), InstTy);
if (!FD)
return nullptr;

FriendDecl *FD = FriendDecl::Create(
SemaRef.Context, Owner, D->getLocation(), InstTy, D->getFriendLoc());
FD->setAccess(AS_public);
FD->setUnsupportedFriend(D->isUnsupportedFriend());
Owner->addDecl(FD);
Expand Down
8 changes: 4 additions & 4 deletions clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ class A1 {
friend union A; // expected-error {{use of 'A' with tag type that does not match previous declaration}}

friend enum A; // expected-error {{use of 'A' with tag type that does not match previous declaration}}
friend enum E;
#if __cplusplus <= 199711L // C++03 or earlier modes
// expected-warning@-2 {{befriending enumeration type 'enum E' is a C++11 extension}}
#endif
// expected-warning@-1 {{cannot be declared as a friend}}
// expected-note@-2 {{remove 'enum' to befriend an enum}}
friend enum E; // expected-warning {{cannot be declared as a friend}}
// expected-note@-1 {{remove 'enum' to befriend an enum}}
};

template <class T> struct B { // expected-note {{previous use is here}}
Expand Down
40 changes: 40 additions & 0 deletions clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.elab/p4.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// RUN: %clang_cc1 -verify %s -std=c++11 -pedantic-errors

enum class E;

template<typename T>
struct A {
enum class F;
};

struct B {
template<typename T>
friend enum A<T>::F; // expected-error {{elaborated enum specifier cannot be declared as a friend}}
// expected-note@-1 {{remove 'enum' to befriend an enum}}

// FIXME: Per [temp.expl.spec]p19, a friend declaration cannot be an explicit specialization
template<>
friend enum A<int>::F; // expected-error {{elaborated enum specifier cannot be declared as a friend}}
// expected-note@-1 {{remove 'enum' to befriend an enum}}

enum class G;

friend enum E; // expected-error {{elaborated enum specifier cannot be declared as a friend}}
// expected-note@-1 {{remove 'enum' to befriend an enum}}
};

template<typename T>
struct C {
friend enum T::G; // expected-error {{elaborated enum specifier cannot be declared as a friend}}
// expected-note@-1 {{remove 'enum' to befriend an enum}}
friend enum A<T>::G; // expected-error {{elaborated enum specifier cannot be declared as a friend}}
// expected-note@-1 {{remove 'enum' to befriend an enum}}
};

struct D {
friend enum B::G; // expected-error {{elaborated enum specifier cannot be declared as a friend}}
// expected-note@-1 {{remove 'enum' to befriend an enum}}
friend enum class B::G; // expected-error {{elaborated enum specifier cannot be declared as a friend}}
// expected-note@-1 {{remove 'enum class' to befriend an enum}}
// expected-error@-2 {{reference to enumeration must use 'enum' not 'enum class'}}
};

0 comments on commit 3a48630

Please sign in to comment.