diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -67,7 +67,8 @@ New Compiler Flags ------------------ -- ... +- ``-Wreserved-identifier`` emits warning when user code uses reserved + identifiers. Deprecated Compiler Flags ------------------------- diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -356,6 +356,10 @@ /// a C++ class. bool isCXXInstanceMember() const; + /// Determine if the declaration obeys the reserved identifier rules of the + /// given language. + ReservedIdentifierStatus isReserved(const LangOptions &LangOpts) const; + /// Determine what kind of linkage this entity has. /// /// This is not the linkage as defined by the standard or the codegen notion diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -638,7 +638,8 @@ // Preprocessor warnings. def AmbiguousMacro : DiagGroup<"ambiguous-macro">; def KeywordAsMacro : DiagGroup<"keyword-macro">; -def ReservedIdAsMacro : DiagGroup<"reserved-id-macro">; +def ReservedIdAsMacro : DiagGroup<"reserved-macro-identifier">; +def ReservedIdAsMacroAlias : DiagGroup<"reserved-id-macro", [ReservedIdAsMacro]>; // Just silence warnings about -Wstrict-aliasing for now. def : DiagGroup<"strict-aliasing=0">; @@ -801,6 +802,9 @@ def DuplicateArgDecl : DiagGroup<"duplicate-method-arg">; def SignedEnumBitfield : DiagGroup<"signed-enum-bitfield">; +def ReservedIdentifier : DiagGroup<"reserved-identifier", + [ReservedIdAsMacro]>; + // Unreachable code warning groups. // // The goal is make -Wunreachable-code on by default, in -Wall, or at diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -379,6 +379,15 @@ "%select{used|required to be captured for this use}1">, InGroup, DefaultIgnore; +def warn_reserved_extern_symbol: Warning< + "identifier %0 is reserved because %select{" + "|" // ReservedIdentifierStatus::NotReserved + "it starts with '_' at global scope|" + "it starts with '__'|" + "it starts with '_' followed by a capital letter|" + "it contains '__'}1">, + InGroup, DefaultIgnore; + def warn_parameter_size: Warning< "%0 is a large (%1 bytes) pass-by-value argument; " "pass it by reference instead ?">, InGroup; diff --git a/clang/include/clang/Basic/IdentifierTable.h b/clang/include/clang/Basic/IdentifierTable.h --- a/clang/include/clang/Basic/IdentifierTable.h +++ b/clang/include/clang/Basic/IdentifierTable.h @@ -40,6 +40,14 @@ class MultiKeywordSelector; class SourceLocation; +enum class ReservedIdentifierStatus { + NotReserved = 0, + StartsWithUnderscoreAtGlobalScope, + StartsWithDoubleUnderscore, + StartsWithUnderscoreFollowedByCapitalLetter, + ContainsDoubleUnderscore, +}; + /// A simple pair of identifier info and location. using IdentifierLocPair = std::pair; @@ -385,14 +393,7 @@ /// Determine whether \p this is a name reserved for the implementation (C99 /// 7.1.3, C++ [lib.global.names]). - bool isReservedName(bool doubleUnderscoreOnly = false) const { - if (getLength() < 2) - return false; - const char *Name = getNameStart(); - return Name[0] == '_' && - (Name[1] == '_' || - (Name[1] >= 'A' && Name[1] <= 'Z' && !doubleUnderscoreOnly)); - } + ReservedIdentifierStatus isReserved(const LangOptions &LangOpts) const; /// Provide less than operator for lexicographical sorting. bool operator<(const IdentifierInfo &RHS) const { diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2590,6 +2590,8 @@ SourceLocation Less, SourceLocation Greater); + void warnOnReservedIdentifier(const NamedDecl *D); + Decl *ActOnDeclarator(Scope *S, Declarator &D); NamedDecl *HandleDeclarator(Scope *S, Declarator &D, diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -1078,6 +1078,29 @@ return L == getCachedLinkage(); } +ReservedIdentifierStatus +NamedDecl::isReserved(const LangOptions &LangOpts) const { + const IdentifierInfo *II = getIdentifier(); + if (!II) + if (const auto *FD = dyn_cast(this)) + II = FD->getLiteralIdentifier(); + + if (!II) + return ReservedIdentifierStatus::NotReserved; + + ReservedIdentifierStatus Status = II->isReserved(LangOpts); + if (Status == ReservedIdentifierStatus::StartsWithUnderscoreAtGlobalScope) { + // Check if we're at TU level or not. + if (isa(this) || isTemplateParameter()) + return ReservedIdentifierStatus::NotReserved; + const DeclContext *DC = getDeclContext()->getRedeclContext(); + if (!DC->isTranslationUnit()) + return ReservedIdentifierStatus::NotReserved; + } + + return Status; +} + ObjCStringFormatFamily NamedDecl::getObjCFStringFormattingFamily() const { StringRef name = getName(); if (name.empty()) return SFF_None; diff --git a/clang/lib/Basic/IdentifierTable.cpp b/clang/lib/Basic/IdentifierTable.cpp --- a/clang/lib/Basic/IdentifierTable.cpp +++ b/clang/lib/Basic/IdentifierTable.cpp @@ -273,6 +273,39 @@ return !isKeyword(LangOptsNoCPP); } +ReservedIdentifierStatus +IdentifierInfo::isReserved(const LangOptions &LangOpts) const { + StringRef Name = getName(); + + // '_' is a reserved identifier, but its use is so common (e.g. to store + // ignored values) that we don't warn on it. + if (Name.size() <= 1) + return ReservedIdentifierStatus::NotReserved; + + // [lex.name] p3 + if (Name[0] == '_') { + + // Each name that begins with an underscore followed by an uppercase letter + // or another underscore is reserved. + if (Name[1] == '_') + return ReservedIdentifierStatus::StartsWithDoubleUnderscore; + + if ('A' <= Name[1] && Name[1] <= 'Z') + return ReservedIdentifierStatus:: + StartsWithUnderscoreFollowedByCapitalLetter; + + // This is a bit misleading: it actually means it's only reserved if we're + // at global scope because it starts with an underscore. + return ReservedIdentifierStatus::StartsWithUnderscoreAtGlobalScope; + } + + // Each name that contains a double underscore (__) is reserved. + if (LangOpts.CPlusPlus && Name.contains("__")) + return ReservedIdentifierStatus::ContainsDoubleUnderscore; + + return ReservedIdentifierStatus::NotReserved; +} + tok::PPKeywordKind IdentifierInfo::getPPKeywordID() const { // We use a perfect hash function here involving the length of the keyword, // the first and third character. For preprocessor ID's there are no diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -4058,9 +4058,9 @@ if (CalleeDecl->getBuiltinID() != 0 || CalleeDecl->hasAttr() || getCallSiteRelatedAttrs() == llvm::DINode::FlagZero) return; - if (const auto *Id = CalleeDecl->getIdentifier()) - if (Id->isReservedName()) - return; + if (CalleeDecl->isReserved(CGM.getLangOpts()) != + ReservedIdentifierStatus::NotReserved) + return; // If there is no DISubprogram attached to the function being called, // create the one describing the function in order to have complete diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -739,18 +739,17 @@ // Filter out names reserved for the implementation if they come from a // system header. static bool shouldIgnoreDueToReservedName(const NamedDecl *ND, Sema &SemaRef) { - const IdentifierInfo *Id = ND->getIdentifier(); - if (!Id) - return false; - + ReservedIdentifierStatus Status = ND->isReserved(SemaRef.getLangOpts()); // Ignore reserved names for compiler provided decls. - if (Id->isReservedName() && ND->getLocation().isInvalid()) + if ((Status != ReservedIdentifierStatus::NotReserved) && + (Status != ReservedIdentifierStatus::StartsWithUnderscoreAtGlobalScope) && + ND->getLocation().isInvalid()) return true; // For system headers ignore only double-underscore names. // This allows for system headers providing private symbols with a single // underscore. - if (Id->isReservedName(/*doubleUnderscoreOnly=*/true) && + if (Status == ReservedIdentifierStatus::StartsWithDoubleUnderscore && SemaRef.SourceMgr.isInSystemHeader( SemaRef.SourceMgr.getSpellingLoc(ND->getLocation()))) return true; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -1516,6 +1516,7 @@ } else { IdResolver.AddDecl(D); } + warnOnReservedIdentifier(D); } bool Sema::isDeclInScope(NamedDecl *D, DeclContext *Ctx, Scope *S, @@ -5558,6 +5559,18 @@ return false; } +void Sema::warnOnReservedIdentifier(const NamedDecl *D) { + // Avoid warning twice on the same identifier, and don't warn on redeclaration + // of system decl. + if (D->getPreviousDecl() || D->isImplicit()) + return; + ReservedIdentifierStatus Status = D->isReserved(getLangOpts()); + if (Status != ReservedIdentifierStatus::NotReserved && + !Context.getSourceManager().isInSystemHeader(D->getLocation())) + Diag(D->getLocation(), diag::warn_reserved_extern_symbol) + << D << static_cast(Status); +} + Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D) { D.setFunctionDefinitionKind(FunctionDefinitionKind::Declaration); Decl *Dcl = HandleDeclarator(S, D, MultiTemplateParamsArg()); diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -16778,6 +16778,8 @@ } } + warnOnReservedIdentifier(ND); + return ND; } diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -542,6 +542,12 @@ return SubStmt; } + ReservedIdentifierStatus Status = TheDecl->isReserved(getLangOpts()); + if (Status != ReservedIdentifierStatus::NotReserved && + !Context.getSourceManager().isInSystemHeader(IdentLoc)) + Diag(IdentLoc, diag::warn_reserved_extern_symbol) + << TheDecl << static_cast(Status); + // Otherwise, things are good. Fill in the declaration and return it. LabelStmt *LS = new (Context) LabelStmt(IdentLoc, TheDecl, SubStmt); TheDecl->setStmt(LS); diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -1677,6 +1677,9 @@ if (ExportLoc.isValid()) Diag(ExportLoc, diag::warn_template_export_unsupported); + for (NamedDecl *P : Params) + warnOnReservedIdentifier(P); + return TemplateParameterList::Create( Context, TemplateLoc, LAngleLoc, llvm::makeArrayRef(Params.data(), Params.size()), diff --git a/clang/test/Preprocessor/macro-reserved.c b/clang/test/Preprocessor/macro-reserved.c --- a/clang/test/Preprocessor/macro-reserved.c +++ b/clang/test/Preprocessor/macro-reserved.c @@ -46,7 +46,7 @@ #define volatile // expected-warning {{keyword is hidden by macro definition}} #undef volatile -#pragma clang diagnostic warning "-Wreserved-id-macro" +#pragma clang diagnostic warning "-Wreserved-macro-identifier" #define switch if // expected-warning {{keyword is hidden by macro definition}} #define final 1 diff --git a/clang/test/Preprocessor/macro-reserved.cpp b/clang/test/Preprocessor/macro-reserved.cpp --- a/clang/test/Preprocessor/macro-reserved.cpp +++ b/clang/test/Preprocessor/macro-reserved.cpp @@ -47,8 +47,7 @@ #define volatile // expected-warning {{keyword is hidden by macro definition}} #undef volatile - -#pragma clang diagnostic warning "-Wreserved-id-macro" +#pragma clang diagnostic warning "-Wreserved-macro-identifier" #define switch if // expected-warning {{keyword is hidden by macro definition}} #define final 1 diff --git a/clang/test/Sema/reserved-identifier.c b/clang/test/Sema/reserved-identifier.c new file mode 100644 --- /dev/null +++ b/clang/test/Sema/reserved-identifier.c @@ -0,0 +1,65 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wreserved-identifier -Wno-visibility %s + +#define __oof foo__ // expected-warning {{macro name is a reserved identifier}} + +int foo__bar() { return 0; } // no-warning +static int _bar() { return 0; } // expected-warning {{identifier '_bar' is reserved because it starts with '_' at global scope}} +static int _Bar() { return 0; } // expected-warning {{identifier '_Bar' is reserved because it starts with '_' followed by a capital letter}} +int _foo() { return 0; } // expected-warning {{identifier '_foo' is reserved because it starts with '_' at global scope}} + +// This one is explicitly skipped by -Wreserved-identifier +void *_; // no-warning + +void foo(unsigned int _Reserved) { // expected-warning {{identifier '_Reserved' is reserved because it starts with '_' followed by a capital letter}} + unsigned int __1 = // expected-warning {{identifier '__1' is reserved because it starts with '__'}} + _Reserved; // no-warning + goto __reserved; // expected-warning {{identifier '__reserved' is reserved because it starts with '__'}} +__reserved: // expected-warning {{identifier '__reserved' is reserved because it starts with '__'}} + ; +} + +void foot(unsigned int _not_reserved) {} // no-warning + +enum __menu { // expected-warning {{identifier '__menu' is reserved because it starts with '__'}} + __some, // expected-warning {{identifier '__some' is reserved because it starts with '__'}} + _Other, // expected-warning {{identifier '_Other' is reserved because it starts with '_' followed by a capital letter}} + _other // expected-warning {{identifier '_other' is reserved because it starts with '_' at global scope}} +}; + +struct __babar { // expected-warning {{identifier '__babar' is reserved because it starts with '__'}} +}; + +struct _Zebulon; // expected-warning {{identifier '_Zebulon' is reserved because it starts with '_' followed by a capital letter}} +struct _Zebulon2 { // expected-warning {{identifier '_Zebulon2' is reserved because it starts with '_' followed by a capital letter}} +} * p; +struct _Zebulon3 *pp; // expected-warning {{identifier '_Zebulon3' is reserved because it starts with '_' followed by a capital letter}} + +typedef struct { + int _Field; // expected-warning {{identifier '_Field' is reserved because it starts with '_' followed by a capital letter}} + int _field; // no-warning +} _Typedef; // expected-warning {{identifier '_Typedef' is reserved because it starts with '_' followed by a capital letter}} + +int foobar() { + return foo__bar(); // no-warning +} + +struct _reserved { // expected-warning {{identifier '_reserved' is reserved because it starts with '_' at global scope}} + int a; +} cunf(void) { + return (struct _reserved){1}; +} + +// FIXME: According to clang declaration context layering, _preserved belongs to +// the translation unit, so we emit a warning. It's unclear that's what the +// standard mandate, but it's such a corner case we can live with it. +void func(struct _preserved { int a; } r) {} // expected-warning {{identifier '_preserved' is reserved because it starts with '_' at global scope}} + +extern char *_strdup(const char *); // expected-warning {{identifier '_strdup' is reserved because it starts with '_' at global scope}} + +// Don't warn on redecleration +extern char *_strdup(const char *); // no-warning + +void ok() { + void _ko(); // expected-warning {{identifier '_ko' is reserved because it starts with '_' at global scope}} + extern int _ko_again; // expected-warning {{identifier '_ko_again' is reserved because it starts with '_' at global scope}} +} diff --git a/clang/test/Sema/reserved-identifier.cpp b/clang/test/Sema/reserved-identifier.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Sema/reserved-identifier.cpp @@ -0,0 +1,91 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -Wreserved-identifier %s + +int foo__bar() { return 0; } // expected-warning {{identifier 'foo__bar' is reserved because it contains '__'}} +static int _bar() { return 0; } // expected-warning {{identifier '_bar' is reserved because it starts with '_' at global scope}} +static int _Bar() { return 0; } // expected-warning {{identifier '_Bar' is reserved because it starts with '_' followed by a capital letter}} +int _barbouille() { return 0; } // expected-warning {{identifier '_barbouille' is reserved because it starts with '_' at global scope}} + +void foo(unsigned int _Reserved) { // expected-warning {{identifier '_Reserved' is reserved because it starts with '_' followed by a capital letter}} + unsigned int __1 = // expected-warning {{identifier '__1' is reserved because it starts with '__'}} + _Reserved; // no-warning +} + +// This one is explicitly skipped by -Wreserved-identifier +void *_; // no-warning + +template constexpr bool __toucan = true; // expected-warning {{identifier '__toucan' is reserved because it starts with '__'}} + +template +concept _Barbotine = __toucan; // expected-warning {{identifier '_Barbotine' is reserved because it starts with '_' followed by a capital letter}} + +template // expected-warning {{'__' is reserved because it starts with '__'}} +struct BarbeNoire {}; + +template // no-warning +struct BarbeJaune {}; + +template // expected-warning {{'__' is reserved because it starts with '__'}} +void BarbeRousse() {} + +namespace _Barbidur { // expected-warning {{identifier '_Barbidur' is reserved because it starts with '_' followed by a capital letter}} + +struct __barbidou {}; // expected-warning {{identifier '__barbidou' is reserved because it starts with '__'}} +struct _barbidou {}; // no-warning + +int __barbouille; // expected-warning {{identifier '__barbouille' is reserved because it starts with '__'}} +int _barbouille; // no-warning + +int __babar() { return 0; } // expected-warning {{identifier '__babar' is reserved because it starts with '__'}} +int _babar() { return 0; } // no-warning + +} // namespace _Barbidur + +class __barbapapa { // expected-warning {{identifier '__barbapapa' is reserved because it starts with '__'}} + void _barbabelle() {} // no-warning + int _Barbalala; // expected-warning {{identifier '_Barbalala' is reserved because it starts with '_' followed by a capital letter}} +}; + +enum class __menu { // expected-warning {{identifier '__menu' is reserved because it starts with '__'}} + __some, // expected-warning {{identifier '__some' is reserved because it starts with '__'}} + _Other, // expected-warning {{identifier '_Other' is reserved because it starts with '_' followed by a capital letter}} + _other // no-warning +}; + +enum _Menu { // expected-warning {{identifier '_Menu' is reserved because it starts with '_' followed by a capital letter}} + _OtheR_, // expected-warning {{identifier '_OtheR_' is reserved because it starts with '_' followed by a capital letter}} + _other_ // expected-warning {{identifier '_other_' is reserved because it starts with '_' at global scope}} +}; + +enum { + __some, // expected-warning {{identifier '__some' is reserved because it starts with '__'}} + _Other, // expected-warning {{identifier '_Other' is reserved because it starts with '_' followed by a capital letter}} + _other // expected-warning {{identifier '_other' is reserved because it starts with '_' at global scope}} +}; + +static union { + int _barbeFleurie; // no-warning +}; + +using _Barbamama = __barbapapa; // expected-warning {{identifier '_Barbamama' is reserved because it starts with '_' followed by a capital letter}} + +int foobar() { + return foo__bar(); // no-warning +} + +namespace { +int _barbatruc; // no-warning +} + +long double operator"" _BarbeBleue(long double) // expected-warning {{identifier 'operator""_BarbeBleue' is reserved because it starts with '_' followed by a capital letter}} +{ + return 0.; +} + +struct _BarbeRouge { // expected-warning {{identifier '_BarbeRouge' is reserved because it starts with '_' followed by a capital letter}} +} p; +struct _BarbeNoire { // expected-warning {{identifier '_BarbeNoire' is reserved because it starts with '_' followed by a capital letter}} +} * q; + +struct Any { + friend void _barbegrise(); // expected-warning {{identifier '_barbegrise' is reserved because it starts with '_' at global scope}} +};