llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Aaron Ballman (AaronBallman) <details> <summary>Changes</summary> This adds a new diagnostic group, -Widentifier-is-c++-keyword, which is off by default and grouped under -Wc++-compat. The diagnostic catches use of C++ keywords in C code. This change additionally fixes an issue with -Wreserved-identifier not diagnosing use of reserved identifiers in function parameter lists in a function declaration which is not a definition. Partially fixes https://github.com/llvm/llvm-project/issues/21898 --- Patch is 25.28 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/137234.diff 7 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+5) - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+2-1) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3) - (modified) clang/include/clang/Basic/IdentifierTable.h (+8-3) - (modified) clang/lib/Basic/IdentifierTable.cpp (+27) - (modified) clang/lib/Sema/SemaDecl.cpp (+31) - (added) clang/test/Sema/c++-keyword-in-c.c (+213) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d1f24fb23d44d..453bdc9870bdb 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -143,6 +143,9 @@ C Language Changes - Added ``-Wimplicit-void-ptr-cast``, grouped under ``-Wc++-compat``, which diagnoses implicit conversion from ``void *`` to another pointer type as being incompatible with C++. (#GH17792) +- Added ``-Widentifier-is-c++-keyword``, grouped under ``-Wc++-compat``, which + diagnoses when a C++ keyword is used as an identifier in C. Partially + addresses #GH21898. C2y Feature Support ^^^^^^^^^^^^^^^^^^^ @@ -416,6 +419,8 @@ Improvements to Clang's diagnostics - ``-Winitializer-overrides`` and ``-Wreorder-init-list`` are now grouped under the ``-Wc99-designator`` diagnostic group, as they also are about the behavior of the C99 feature as it was introduced into C++20. Fixes #GH47037 +- ``-Wreserved-identifier`` now fires on reserved parameter names in a function + declaration which is not a definition. Improvements to Clang's time-trace ---------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 6441b8049ed8d..84e590a857398 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -155,8 +155,9 @@ def C99Compat : DiagGroup<"c99-compat">; def C23Compat : DiagGroup<"c23-compat">; def : DiagGroup<"c2x-compat", [C23Compat]>; +def CppKeywordInC : DiagGroup<"identifier-is-c++-keyword">; def ImplicitVoidPtrCast : DiagGroup<"implicit-void-ptr-cast">; -def CXXCompat: DiagGroup<"c++-compat", [ImplicitVoidPtrCast]>; +def CXXCompat: DiagGroup<"c++-compat", [ImplicitVoidPtrCast, CppKeywordInC]>; def ExternCCompat : DiagGroup<"extern-c-compat">; def KeywordCompat : DiagGroup<"keyword-compat">; def GNUCaseRange : DiagGroup<"gnu-case-range">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 8ff170520aafe..50a960313349a 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -496,6 +496,9 @@ def warn_unused_lambda_capture: Warning<"lambda capture %0 is not " "%select{used|required to be captured for this use}1">, InGroup<UnusedLambdaCapture>, DefaultIgnore; +def warn_identifier_is_cpp_keyword : Warning< + "identifier %0 conflicts with a C++ keyword">, + InGroup<CppKeywordInC>, DefaultIgnore; def warn_reserved_extern_symbol: Warning< "identifier %0 is reserved because %select{" "<ERROR>|" // ReservedIdentifierStatus::NotReserved diff --git a/clang/include/clang/Basic/IdentifierTable.h b/clang/include/clang/Basic/IdentifierTable.h index 1275b056227b5..05c989c1b07d9 100644 --- a/clang/include/clang/Basic/IdentifierTable.h +++ b/clang/include/clang/Basic/IdentifierTable.h @@ -444,13 +444,18 @@ class alignas(IdentifierInfoAlignment) IdentifierInfo { } bool isCPlusPlusOperatorKeyword() const { return IsCPPOperatorKeyword; } - /// Return true if this token is a keyword in the specified language. + /// Return true if this identifier uses a keyword token and is a keyword in + /// the specified language. bool isKeyword(const LangOptions &LangOpts) const; - /// Return true if this token is a C++ keyword in the specified - /// language. + /// Return true if this identifier uses a keyword token and is a C++ keyword + /// in the specified language. bool isCPlusPlusKeyword(const LangOptions &LangOpts) const; + /// Returns true if the name of this identifier matches a keyword given the + /// specified language options. + bool isNameKeyword(const LangOptions &LangOpts) const; + /// Get and set FETokenInfo. The language front-end is allowed to associate /// arbitrary metadata with this token. void *getFETokenInfo() const { return FETokenInfo; } diff --git a/clang/lib/Basic/IdentifierTable.cpp b/clang/lib/Basic/IdentifierTable.cpp index 16151c94464f9..412f0af40861a 100644 --- a/clang/lib/Basic/IdentifierTable.cpp +++ b/clang/lib/Basic/IdentifierTable.cpp @@ -343,6 +343,14 @@ static KeywordStatus getTokenKwStatus(const LangOptions &LangOpts, } } +static KeywordStatus getNameKwStatus(const LangOptions &LangOpts, + StringRef Name) { + return llvm::StringSwitch<KeywordStatus>(Name) +#define KEYWORD(NAME, FLAGS) .Case(#NAME, getKeywordStatus(LangOpts, FLAGS)) +#include "clang/Basic/TokenKinds.def" + .Default(KS_Disabled); +} + /// Returns true if the identifier represents a keyword in the /// specified language. bool IdentifierInfo::isKeyword(const LangOptions &LangOpts) const { @@ -355,6 +363,25 @@ bool IdentifierInfo::isKeyword(const LangOptions &LangOpts) const { } } +bool IdentifierInfo::isNameKeyword(const LangOptions &LangOpts) const { + // This differs from IdentifierInfo::isCPlusPlusKeyword(). That function + // tests if the identifier is a keyword token in C++ mode and then isn't a + // keyword token in C modes. In our case, if it was a keyword, we wouldn't + // have gotten the identifier for it anyway, so that function will always + // return false for us. Instead, check against the token definitions directly. + // + // FIXME: It would be nice to handle things like char8_t and wchar_t here as + // well, but those require looking at the currently selected language options + // which would make this interface confusing with isCPlusPlusKeyword. + switch (getNameKwStatus(LangOpts, getName())) { + case KS_Enabled: + case KS_Extension: + return true; + default: + return false; + } +} + /// Returns true if the identifier represents a C++ keyword in the /// specified language. bool IdentifierInfo::isCPlusPlusKeyword(const LangOptions &LangOpts) const { diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index d28a2107d58a9..74432ea34bd57 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -61,6 +61,7 @@ #include "llvm/ADT/STLForwardCompat.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringSwitch.h" #include "llvm/Support/SaveAndRestore.h" #include "llvm/TargetParser/Triple.h" #include <algorithm> @@ -6107,6 +6108,23 @@ static bool isFromSystemHeader(SourceManager &SM, const Decl *D) { SM.isInSystemMacro(D->getLocation()); } +static bool isKeywordInCPlusPlus(const Sema &S, const IdentifierInfo* II) { + if (!II) + return false; + + // Clear all the C language options, set all the C++ language options, but + // otherwise leave all the defaults alone. This isn't ideal because some + // feature flags are language-specific, but this is a reasonable + // approximation. + LangOptions LO = S.getLangOpts(); + LO.CPlusPlus = LO.CPlusPlus11 = LO.CPlusPlus14 = LO.CPlusPlus17 = + LO.CPlusPlus20 = LO.CPlusPlus23 = LO.CPlusPlus26 = 1; + LO.C99 = LO.C11 = LO.C17 = LO.C23 = LO.C2y = 0; + LO.Char8 = 1; // Presume this is always a keyword in C++. + LO.WChar = 1; // Presume this is always a keyword in C++. + return II->isNameKeyword(LO); +} + void Sema::warnOnReservedIdentifier(const NamedDecl *D) { // Avoid warning twice on the same identifier, and don't warn on redeclaration // of system decl. @@ -6118,6 +6136,10 @@ void Sema::warnOnReservedIdentifier(const NamedDecl *D) { Diag(D->getLocation(), diag::warn_reserved_extern_symbol) << D << static_cast<int>(Status); } + // Diagnose use of C++ keywords in C as being incompatible with C++. + if (!getLangOpts().CPlusPlus && + isKeywordInCPlusPlus(*this, D->getIdentifier())) + Diag(D->getLocation(), diag::warn_identifier_is_cpp_keyword) << D; } Decl *Sema::ActOnDeclarator(Scope *S, Declarator &D) { @@ -10416,6 +10438,15 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, // Finally, we know we have the right number of parameters, install them. NewFD->setParams(Params); + // If this declarator is a declaration and not a definition, its parameters + // will not be pushed onto a scope chain. That means we will not issue any + // reserved identifier warnings for the declaration, but we will for the + // definition. Handle those here. + if (!Params.empty() && !D.isFunctionDefinition()) { + for (const ParmVarDecl *PVD : Params) + warnOnReservedIdentifier(PVD); + } + if (D.getDeclSpec().isNoreturnSpecified()) NewFD->addAttr( C11NoReturnAttr::Create(Context, D.getDeclSpec().getNoreturnSpecLoc())); diff --git a/clang/test/Sema/c++-keyword-in-c.c b/clang/test/Sema/c++-keyword-in-c.c new file mode 100644 index 0000000000000..364f115ddcb44 --- /dev/null +++ b/clang/test/Sema/c++-keyword-in-c.c @@ -0,0 +1,213 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Widentifier-is-c++-keyword %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wc++-compat %s +// RUN: %clang_cc1 -fsyntax-only -verify=good %s +// RUN: %clang_cc1 -fsyntax-only -verify=cxx -x c++ -std=c++2c %s +// good-no-diagnostics + +// 'try', 'this' and 'throw' are not tested as identifiers, but are instead +// tested as other constructs (otherwise there would be redefinition errors in +// C). +int catch; // expected-warning {{identifier 'catch' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int class; // expected-warning {{identifier 'class' conflicts with a C++ keyword}} \ + cxx-error {{declaration of anonymous class must be a definition}} \ + cxx-warning {{declaration does not declare anything}} +int const_cast; // expected-warning {{identifier 'const_cast' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int delete; // expected-warning {{identifier 'delete' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int dynamic_cast; // expected-warning {{identifier 'dynamic_cast' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int explicit; // expected-warning {{identifier 'explicit' conflicts with a C++ keyword}} \ + cxx-error {{'explicit' can only appear on non-static member functions}} \ + cxx-warning {{declaration does not declare anything}} +int export; // expected-warning {{identifier 'export' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int friend; // expected-warning {{identifier 'friend' conflicts with a C++ keyword}} \ + cxx-error {{'friend' used outside of class}} \ + cxx-warning {{declaration does not declare anything}} +int mutable; // expected-warning {{identifier 'mutable' conflicts with a C++ keyword}} \ + cxx-warning {{declaration does not declare anything}} +int namespace; // expected-warning {{identifier 'namespace' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int new; // expected-warning {{identifier 'new' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int operator; // expected-warning {{identifier 'operator' conflicts with a C++ keyword}} \ + cxx-error {{expected a type}} +int private; // expected-warning {{identifier 'private' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int protected; // expected-warning {{identifier 'protected' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int public; // expected-warning {{identifier 'public' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int reinterpret_cast; // expected-warning {{identifier 'reinterpret_cast' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int static_cast; // expected-warning {{identifier 'static_cast' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int template; // expected-warning {{identifier 'template' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int typename; // expected-warning {{identifier 'typename' conflicts with a C++ keyword}} \ + cxx-error {{expected a qualified name after 'typename'}} +int typeid; // expected-warning {{identifier 'typeid' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int using; // expected-warning {{identifier 'using' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int virtual; // expected-warning {{identifier 'virtual' conflicts with a C++ keyword}} \ + cxx-error {{'virtual' can only appear on non-static member functions}} \ + cxx-warning {{declaration does not declare anything}} +int wchar_t; // expected-warning {{identifier 'wchar_t' conflicts with a C++ keyword}} \ + cxx-error {{cannot combine with previous 'int' declaration specifier}} \ + cxx-warning {{declaration does not declare anything}} +int char8_t; // expected-warning {{identifier 'char8_t' conflicts with a C++ keyword}} \ + cxx-error {{cannot combine with previous 'int' declaration specifier}} \ + cxx-warning {{declaration does not declare anything}} +int char16_t; // expected-warning {{identifier 'char16_t' conflicts with a C++ keyword}} \ + cxx-error {{cannot combine with previous 'int' declaration specifier}} \ + cxx-warning {{declaration does not declare anything}} +int char32_t; // expected-warning {{identifier 'char32_t' conflicts with a C++ keyword}} \ + cxx-error {{cannot combine with previous 'int' declaration specifier}} \ + cxx-warning {{declaration does not declare anything}} +int noexcept; // expected-warning {{identifier 'noexcept' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int co_await; // expected-warning {{identifier 'co_await' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int co_return; // expected-warning {{identifier 'co_return' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int co_yield; // expected-warning {{identifier 'co_yield' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int consteval; // expected-warning {{identifier 'consteval' conflicts with a C++ keyword}} \ + cxx-error {{consteval can only be used in function declarations}} +int constinit; // expected-warning {{identifier 'constinit' conflicts with a C++ keyword}} \ + cxx-error {{constinit can only be used in variable declarations}} +int concept; // expected-warning {{identifier 'concept' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} +int requires; // expected-warning {{identifier 'requires' conflicts with a C++ keyword}} \ + cxx-error {{expected unqualified-id}} + +// Now try the same thing, but as struct members. +struct S { + int catch; // expected-warning {{identifier 'catch' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int class; // expected-warning {{identifier 'class' conflicts with a C++ keyword}} \ + cxx-error {{declaration of anonymous class must be a definition}} \ + cxx-warning {{declaration does not declare anything}} + int const_cast; // expected-warning {{identifier 'const_cast' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int delete; // expected-warning {{identifier 'delete' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int dynamic_cast; // expected-warning {{identifier 'dynamic_cast' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int explicit; // expected-warning {{identifier 'explicit' conflicts with a C++ keyword}} \ + cxx-error {{'explicit' can only appear on non-static member functions}} \ + cxx-warning {{declaration does not declare anything}} + int export; // expected-warning {{identifier 'export' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int friend; // expected-warning {{identifier 'friend' conflicts with a C++ keyword}} \ + cxx-error {{'friend' must appear first in a non-function declaration}} + int mutable; // expected-warning {{identifier 'mutable' conflicts with a C++ keyword}} \ + cxx-warning {{declaration does not declare anything}} + int namespace; // expected-warning {{identifier 'namespace' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int new; // expected-warning {{identifier 'new' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int operator; // expected-warning {{identifier 'operator' conflicts with a C++ keyword}} \ + cxx-error {{expected a type}} + int private; // expected-warning {{identifier 'private' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int protected; // expected-warning {{identifier 'protected' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int public; // expected-warning {{identifier 'public' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int reinterpret_cast; // expected-warning {{identifier 'reinterpret_cast' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int static_cast; // expected-warning {{identifier 'static_cast' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int template; // expected-warning {{identifier 'template' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int this; // expected-warning {{identifier 'this' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int throw; // expected-warning {{identifier 'throw' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int try; // expected-warning {{identifier 'try' conflicts with a C++ keyword}} \ + cxx-error {{expected member name or ';' after declaration specifiers}} + int typename; // expected-warning {{identifier 'typename' conflicts with a C++ keyword}} \ + cxx-error {{exp... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/137234 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
