serge-sans-paille updated this revision to Diff 333810.
serge-sans-paille added a comment.
Adress final(?) comments from @rsmith and @aaron.ballman :
Don't warn on argument (or template argument) of top-level decl.
Extra test cases
Return an enumeration as part of the test
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93095/new/
https://reviews.llvm.org/D93095
Files:
clang/include/clang/AST/Decl.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/Decl.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaStmt.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/Sema/reserved-identifier.c
clang/test/Sema/reserved-identifier.cpp
Index: clang/test/Sema/reserved-identifier.cpp
===================================================================
--- /dev/null
+++ clang/test/Sema/reserved-identifier.cpp
@@ -0,0 +1,87 @@
+// 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 <class T> constexpr bool __toucan = true; // expected-warning {{identifier '__toucan' is reserved because it starts with '__'}}
+
+template <class T>
+concept _Barbotine = __toucan<T>; // expected-warning {{identifier '_Barbotine' is reserved because it starts with '_' followed by a capital letter}}
+
+template <class __> // expected-warning {{'__' is reserved because it starts with '__'}}
+struct BarbeNoire {};
+
+template <class _not_reserved> // no-warning
+struct BarbeJaune {};
+
+template <class __> // 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;
Index: clang/test/Sema/reserved-identifier.c
===================================================================
--- /dev/null
+++ clang/test/Sema/reserved-identifier.c
@@ -0,0 +1,60 @@
+// 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;
+__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
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ 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()),
@@ -8634,6 +8637,7 @@
}
ActOnDocumentableDecl(NewDecl);
+ warnOnReservedIdentifier(NewDecl);
PushOnScopeChains(NewDecl, S);
return NewDecl;
}
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -541,6 +541,12 @@
return SubStmt;
}
+ if (!Context.getSourceManager().isInSystemHeader(IdentLoc)) {
+ ReservedIdentifierStatus Status = TheDecl->isReserved(getLangOpts());
+ if (Status != ReservedIdentifierStatus::NotReserved)
+ Diag(IdentLoc, diag::warn_reserved_extern_symbol) << TheDecl << Status;
+ }
+
// Otherwise, things are good. Fill in the declaration and return it.
LabelStmt *LS = new (Context) LabelStmt(IdentLoc, TheDecl, SubStmt);
TheDecl->setStmt(LS);
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -10995,6 +10995,9 @@
// for the namespace has the declarations that showed up in that particular
// namespace definition.
PushDeclContext(NamespcScope, Namespc);
+
+ warnOnReservedIdentifier(Namespc);
+
return Namespc;
}
@@ -12669,6 +12672,9 @@
PushOnScopeChains(NewND, S);
ActOnDocumentableDecl(NewND);
+
+ warnOnReservedIdentifier(NewND);
+
return NewND;
}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5554,6 +5554,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())
+ return;
+ if (!Context.getSourceManager().isInSystemHeader(D->getLocation())) {
+ ReservedIdentifierStatus Status = D->isReserved(getLangOpts());
+ if (Status != ReservedIdentifierStatus::NotReserved)
+ Diag(D->getLocation(), diag::warn_reserved_extern_symbol) << D << Status;
+ }
+}
+
Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D) {
D.setFunctionDefinitionKind(FunctionDefinitionKind::Declaration);
Decl *Dcl = HandleDeclarator(S, D, MultiTemplateParamsArg());
@@ -5899,6 +5911,7 @@
if (isInOpenMPDeclareTargetContext())
checkDeclIsAllowedInOpenMPTarget(nullptr, New);
+ warnOnReservedIdentifier(New);
return New;
}
@@ -13692,6 +13705,8 @@
if (getLangOpts().OpenCL)
deduceOpenCLAddressSpace(New);
+ warnOnReservedIdentifier(New);
+
return New;
}
@@ -16346,6 +16361,7 @@
} else if (SkipBody && SkipBody->ShouldSkip) {
return SkipBody->Previous;
} else {
+ warnOnReservedIdentifier(New);
return New;
}
}
@@ -17158,6 +17174,8 @@
i != end; ++i) {
FieldDecl *FD = cast<FieldDecl>(*i);
+ warnOnReservedIdentifier(FD);
+
// Get the type for the field.
const Type *FDTy = FD->getType().getTypePtr();
@@ -17883,6 +17901,8 @@
ActOnDocumentableDecl(New);
+ warnOnReservedIdentifier(New);
+
return New;
}
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -1078,6 +1078,57 @@
return L == getCachedLinkage();
}
+ReservedIdentifierStatus
+NamedDecl::isReserved(const LangOptions &LangOpts) const {
+ const IdentifierInfo *II = nullptr;
+ if (const auto *FD = dyn_cast<FunctionDecl>(this))
+ II = FD->getLiteralIdentifier();
+
+ if (!II)
+ II = getIdentifier();
+
+ if (!II)
+ return ReservedIdentifierStatus::NotReserved;
+
+ StringRef Name = II->getName();
+
+ // '_' is a reserved identifier, but it's 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;
+ }
+
+ // Walk up the lexical parents to determine if we're at TU level or not.
+ if (!isa<ParmVarDecl>(this) && !isTemplateParameter()) {
+ const DeclContext *DC = getLexicalDeclContext();
+ while (DC->isTransparentContext())
+ DC = DC->getLexicalParent();
+ if (isa<TranslationUnitDecl>(DC)) {
+ return ReservedIdentifierStatus::StartsWithUnderscoreAtGlobalScope;
+ }
+ }
+ }
+
+ // Each name that contains a double underscore (__) is reserved.
+ if (LangOpts.CPlusPlus && Name.contains("__")) {
+ return ReservedIdentifierStatus::ContainsDoubleUnderscore;
+ }
+
+ return ReservedIdentifierStatus::NotReserved;
+}
+
ObjCStringFormatFamily NamedDecl::getObjCFStringFormattingFamily() const {
StringRef name = getName();
if (name.empty()) return SFF_None;
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2584,6 +2584,8 @@
SourceLocation Less,
SourceLocation Greater);
+ void warnOnReservedIdentifier(const NamedDecl *D);
+
Decl *ActOnDeclarator(Scope *S, Declarator &D);
NamedDecl *HandleDeclarator(Scope *S, Declarator &D,
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -378,6 +378,15 @@
"%select{used|required to be captured for this use}1">,
InGroup<UnusedLambdaCapture>, DefaultIgnore;
+def warn_reserved_extern_symbol: Warning<
+ "identifier %0 is reserved because %select{"
+ "|"
+ "it starts with '_' at global scope|"
+ "it starts with '__'|"
+ "it starts with '_' followed by a capital letter|"
+ "it contains '__'}1">,
+ InGroup<ReservedIdAsSymbol>, DefaultIgnore;
+
def warn_parameter_size: Warning<
"%0 is a large (%1 bytes) pass-by-value argument; "
"pass it by reference instead ?">, InGroup<LargeByValueCopy>;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -794,6 +794,11 @@
def DuplicateArgDecl : DiagGroup<"duplicate-method-arg">;
def SignedEnumBitfield : DiagGroup<"signed-enum-bitfield">;
+
+def ReservedIdAsSymbol : DiagGroup<"reserved-extern-identifier">;
+def ReservedIdentifier : DiagGroup<"reserved-identifier",
+ [ReservedIdAsMacro, ReservedIdAsSymbol]>;
+
// Unreachable code warning groups.
//
// The goal is make -Wunreachable-code on by default, in -Wall, or at
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -78,6 +78,19 @@
class UnresolvedSetImpl;
class VarTemplateDecl;
+enum class ReservedIdentifierStatus {
+ NotReserved = 0,
+ StartsWithUnderscoreAtGlobalScope,
+ StartsWithDoubleUnderscore,
+ StartsWithUnderscoreFollowedByCapitalLetter,
+ ContainsDoubleUnderscore,
+};
+
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+ ReservedIdentifierStatus Status) {
+ return DB << static_cast<int>(Status);
+}
+
/// The top declaration context.
class TranslationUnitDecl : public Decl, public DeclContext {
ASTContext &Ctx;
@@ -356,6 +369,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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
