Index: clang-tidy/cert/AvoidReservedNamesCheck.h =================================================================== --- /dev/null +++ clang-tidy/cert/AvoidReservedNamesCheck.h @@ -0,0 +1,40 @@ +//===--- AvoidReservedNamesCheck.h - clang-tidy------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_AVOID_RESERVED_NAMES_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_AVOID_RESERVED_NAMES_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace cert { + +/// This check will catch names declared or defined as a reserved identifier. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cert-dcl51-cpp.html +class AvoidReservedNamesCheck : public ClangTidyCheck { +public: + AvoidReservedNamesCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void declNameIsReservedCheck(StringRef Name, SourceLocation Location, bool IsGlobal); + void macroNameIsKeywordCheck(const Token &MacroNameTok); + void registerPPCallbacks(CompilerInstance &Compiler) override; +private: + bool isInGlobalNamespace(const Decl *D); +}; + +} // namespace cert +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_AVOID_RESERVED_NAMES_H Index: clang-tidy/cert/AvoidReservedNamesCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/cert/AvoidReservedNamesCheck.cpp @@ -0,0 +1,152 @@ +//===--- AvoidReservedNamesCheck.cpp - clang-tidy--------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "AvoidReservedNamesCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include +#include + +using namespace clang::ast_matchers; + +namespace { +const std::vector Keywords = { +#define KEYWORD(X, Y) #X, +#define CXX_KEYWORD_OPERATOR(X, Y) #X, +#define CXX11_KEYWORD(X, Y) #X, +#define CONCEPTS_KEYWORD(X) #X, +#include "clang/Basic/TokenKinds.def" + "noreturn", + "carries_dependency", + "deprecated", + "fallthrough", + "nodiscard", + "maybe_unused", + "optimize_for_synchronized"}; +} // namespace + +namespace clang { +namespace tidy { +namespace cert { + +namespace { +class AvoidReservedNamesPPCallbacks : public PPCallbacks { +public: + AvoidReservedNamesPPCallbacks(Preprocessor *PP, + AvoidReservedNamesCheck *Check) + : PP(PP), Check(Check) {} + + void MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) override { + macroNameCheck(MacroNameTok, MD); + } + void MacroUndefined(const Token &MacroNameTok, const MacroDefinition &, + const MacroDirective *MD) override { + macroNameCheck(MacroNameTok, MD); + } + +private: + Preprocessor *PP; + AvoidReservedNamesCheck *Check; + bool identical(const StringRef Name, const StringRef Alias) { + if (Name == Alias || + (Alias.endswith(Name) && Alias.size() == (Name.size() + 2))) + return true; + return false; + } + bool semanticallyEquivalent(const Token &MNT, const MacroDirective *MD) { + if (MD->getMacroInfo()->tokens_begin() == MD->getMacroInfo()->tokens_end()) + return false; + for (auto TI : MD->getMacroInfo()->tokens()) { + if (!identical(MNT.getIdentifierInfo()->getName(), + TI.getIdentifierInfo()->getName())) + return false; + } + return true; + } + void macroNameCheck(const Token &MacroNameTok, const MacroDirective *MD) { + if (MacroNameTok.getLocation().isInvalid() || + PP->getSourceManager().isInSystemHeader(MacroNameTok.getLocation()) || + semanticallyEquivalent(MacroNameTok, MD)) + return; + Check->macroNameIsKeywordCheck(MacroNameTok); + Check->declNameIsReservedCheck(MacroNameTok.getIdentifierInfo()->getName(), + MacroNameTok.getLocation(), true); + } +}; +} // namespace + +void AvoidReservedNamesCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(namedDecl().bind("var"), this); + Finder->addMatcher(labelStmt().bind("label"), this); +} + +void AvoidReservedNamesCheck::check(const MatchFinder::MatchResult &Result) { + const auto *ND = Result.Nodes.getNodeAs("var"); + + if (const auto *L = Result.Nodes.getNodeAs("label")) + ND = L->getDecl(); + + if (ND->isImplicit()) + return; + + if (ND->getLocation().isInvalid() || + Result.SourceManager->isInSystemHeader(ND->getLocation()) || + !ND->getIdentifier()) + return; + + declNameIsReservedCheck(ND->getName(), ND->getLocation(), + isInGlobalNamespace(ND)); +} + +void AvoidReservedNamesCheck::declNameIsReservedCheck(StringRef Name, + SourceLocation Location, + bool IsGlobal) { + + if (Name.find("__") != StringRef::npos) + diag(Location, "identifiers containing double underscores are reserved to " + "the implementation"); + if (IsGlobal && Name.startswith("_")) + diag(Location, "do not use global names that start with an underscore"); + if (Name.startswith("_") && Name.size() > 1 && std::isupper(Name[1])) + diag(Location, "identifiers that begin with an underscore followed by an " + "uppercase letter are reserved to the implementation"); +} + +bool AvoidReservedNamesCheck::isInGlobalNamespace(const Decl *D) { + if (D->getDeclContext()->isTranslationUnit()) + return true; + if (const auto *NS = dyn_cast(D->getDeclContext())) { + if (!NS->isAnonymousNamespace()) + return false; + return isInGlobalNamespace(NS); + } + return false; +} + +void AvoidReservedNamesCheck::macroNameIsKeywordCheck( + const Token &MacroNameTok) { + if (Keywords.end() != std::find(Keywords.begin(), Keywords.end(), + MacroNameTok.getIdentifierInfo()->getName())) + diag(MacroNameTok.getLocation(), "do not use a macro name that is " + "identical to a keyword or an attribute"); +} + +void AvoidReservedNamesCheck::registerPPCallbacks(CompilerInstance &Compiler) { + Compiler.getPreprocessor().addPPCallbacks( + llvm::make_unique( + &Compiler.getPreprocessor(), this)); +} + +} // namespace cert +} // namespace tidy +} // namespace clang Index: clang-tidy/cert/CERTTidyModule.cpp =================================================================== --- clang-tidy/cert/CERTTidyModule.cpp +++ clang-tidy/cert/CERTTidyModule.cpp @@ -16,6 +16,7 @@ #include "../misc/NonCopyableObjects.h" #include "../misc/StaticAssertCheck.h" #include "../misc/ThrowByValueCatchByReferenceCheck.h" +#include "AvoidReservedNamesCheck.h" #include "CommandProcessorCheck.h" #include "DontModifyStdNamespaceCheck.h" #include "FloatLoopCounter.h" @@ -39,6 +40,8 @@ CheckFactories.registerCheck( "cert-dcl21-cpp"); CheckFactories.registerCheck("cert-dcl50-cpp"); + CheckFactories.registerCheck( + "cert-dcl51-cpp"); CheckFactories.registerCheck( "cert-dcl54-cpp"); CheckFactories.registerCheck( Index: clang-tidy/cert/CMakeLists.txt =================================================================== --- clang-tidy/cert/CMakeLists.txt +++ clang-tidy/cert/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyCERTModule + AvoidReservedNamesCheck.cpp CERTTidyModule.cpp CommandProcessorCheck.cpp DontModifyStdNamespaceCheck.cpp Index: clang-tidy/tool/run-clang-tidy.py =================================================================== --- clang-tidy/tool/run-clang-tidy.py +++ clang-tidy/tool/run-clang-tidy.py @@ -36,7 +36,6 @@ from __future__ import print_function import argparse -import glob import json import multiprocessing import os @@ -48,7 +47,6 @@ import tempfile import threading import traceback -import yaml def find_compilation_database(path): @@ -91,31 +89,6 @@ return start -def merge_replacement_files(tmpdir, mergefile): - """Merge all replacement files in a directory into a single file""" - # The fixes suggested by clang-tidy >= 4.0.0 are given under - # the top level key 'Diagnostics' in the output yaml files - mergekey="Diagnostics" - merged=[] - for replacefile in glob.iglob(os.path.join(tmpdir, '*.yaml')): - content = yaml.safe_load(open(replacefile, 'r')) - if not content: - continue # Skip empty files. - merged.extend(content.get(mergekey, [])) - - if merged: - # MainSourceFile: The key is required by the definition inside - # include/clang/Tooling/ReplacementsYaml.h, but the value - # is actually never used inside clang-apply-replacements, - # so we set it to '' here. - output = { 'MainSourceFile': '', mergekey: merged } - with open(mergefile, 'w') as out: - yaml.safe_dump(output, out) - else: - # Empty the file: - open(mergefile, 'w').close() - - def check_clang_apply_replacements_binary(args): """Checks if invoking supplied clang-apply-replacements binary works.""" try: @@ -128,7 +101,7 @@ def apply_fixes(args, tmpdir): - """Calls clang-apply-fixes on a given directory.""" + """Calls clang-apply-fixes on a given directory. Deletes the dir when done.""" invocation = [args.clang_apply_replacements_binary] if args.format: invocation.append('-format') @@ -170,9 +143,6 @@ 'headers to output diagnostics from. Diagnostics from ' 'the main file of each translation unit are always ' 'displayed.') - parser.add_argument('-export-fixes', metavar='filename', dest='export_fixes', - help='Create a yaml file to store suggested fixes in, ' - 'which can be applied with clang-apply-replacements.') parser.add_argument('-j', type=int, default=0, help='number of tidy instances to be run in parallel.') parser.add_argument('files', nargs='*', default=['.*'], @@ -224,7 +194,7 @@ max_task = multiprocessing.cpu_count() tmpdir = None - if args.fix or args.export_fixes: + if args.fix: check_clang_apply_replacements_binary(args) tmpdir = tempfile.mkdtemp() @@ -252,32 +222,24 @@ # This is a sad hack. Unfortunately subprocess goes # bonkers with ctrl-c and we start forking merrily. print('\nCtrl-C detected, goodbye.') - if tmpdir: + if args.fix: shutil.rmtree(tmpdir) os.kill(0, 9) - return_code = 0 - if args.export_fixes: - print('Writing fixes to ' + args.export_fixes + ' ...') - try: - merge_replacement_files(tmpdir, args.export_fixes) - except: - print('Error exporting fixes.\n', file=sys.stderr) - traceback.print_exc() - return_code=1 - if args.fix: print('Applying fixes ...') + successfully_applied = False + try: apply_fixes(args, tmpdir) + successfully_applied = True except: print('Error applying fixes.\n', file=sys.stderr) traceback.print_exc() - return_code=1 - if tmpdir: shutil.rmtree(tmpdir) - sys.exit(return_code) + if not successfully_applied: + sys.exit(1) if __name__ == '__main__': main() Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -58,7 +58,10 @@ -------------------------- The improvements are... +- New `cert-dcl51-cpp + `_ check + Checks if a name is declared or defined with a reserved identifier. Improvements to include-fixer ----------------------------- Index: docs/clang-tidy/checks/cert-dcl51-cpp.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/cert-dcl51-cpp.rst @@ -0,0 +1,38 @@ +.. title:: clang-tidy - cert-dcl51-cpp + +cert-dcl51-cpp +============== + +This check will catch the following errors due to the names being reserved to the implementation: + - Names in the global namespace should not start with an underscore. + - Names should not start with an underscore followed by an uppercase letter. + - Names should not contain a double underscore. + - Names defined in macros should not be identical to keywords. + +Here's an example using variables: + +.. code-block:: c++ + + #define override + // warning: macro name should not be identical to keyword + + int _global_variable; + // warning: global name starts with an underscore + + void function() { + int loval__variable; + // warning: name contains a double underscore + + int _Uppercase_variable; + // warning: name starts with an underscore followed by an uppercase letter + } + +This check does not include warnings for reserved macro names and +user defined literals, since Clang checks have already been written to those: + - Wreserved-id-macro + - Wuser-defined-literals + +This check corresponds to the CERT C++ Coding Standard rule +`DCL51-CPP. Do not declare or define a reserved identifier +`_. + Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -14,6 +14,7 @@ cert-dcl03-c (redirects to misc-static-assert) cert-dcl21-cpp cert-dcl50-cpp + cert-dcl51-cpps cert-dcl54-cpp (redirects to misc-new-delete-overloads) cert-dcl58-cpp cert-dcl59-cpp (redirects to google-build-namespaces) Index: test/clang-tidy/Inputs/Headers/dcl51.h =================================================================== --- /dev/null +++ test/clang-tidy/Inputs/Headers/dcl51.h @@ -0,0 +1,6 @@ +#pragma clang system_header + +namespace std { + #define override __override__ + int __variable__; +} Index: test/clang-tidy/cert-avoid-reserved-names.cpp =================================================================== --- /dev/null +++ test/clang-tidy/cert-avoid-reserved-names.cpp @@ -0,0 +1,151 @@ +// RUN: %check_clang_tidy %s cert-dcl51-cpp %t -- -- -I %S/Inputs/Headers + +#include "dcl51.h" + +#define _MACRO_NAME_ +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] +#define switch +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not use a macro name that is identical to a keyword or an attribute [cert-dcl51-cpp] +#define else__if +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: identifiers containing double underscores are reserved to the implementation [cert-dcl51-cpp] + +int _global_variable; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +int _Upper; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] +int __variable; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: identifiers containing double underscores are reserved to the implementation [cert-dcl51-cpp] +int some__variable; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: identifiers containing double underscores are reserved to the implementation [cert-dcl51-cpp] +void fun__ction() { + int local__variable; +} +// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: identifiers containing double underscores are reserved to the implementation [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-3]]:7: warning: identifiers containing double underscores are reserved to the implementation [cert-dcl51-cpp] +void _Function() { + int _Case; +} +// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-4]]:7: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] + +typedef int _Integer; +// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] + +using _Char = char; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-2]]:7: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] + +template void _fun__ction(); +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-3]]:28: warning: identifiers containing double underscores are reserved to the implementation [cert-dcl51-cpp] + +enum _global_names { + _Variable = 1, + second__variable = 2 +}; +// CHECK-MESSAGES: :[[@LINE-4]]:6: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: identifiers containing double underscores are reserved to the implementation [cert-dcl51-cpp] + +void __Function() { + _label__: + if (true) { + goto _label__; + } +} +// CHECK-MESSAGES: :[[@LINE-6]]:6: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-7]]:6: warning: identifiers containing double underscores are reserved to the implementation [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-7]]:3: warning: identifiers containing double underscores are reserved to the implementation [cert-dcl51-cpp] + +namespace _reserved_namespace_ { + int _Variable; + struct _Struct_name { + void Fun__ction(); + }; +} +// CHECK-MESSAGES: :[[@LINE-6]]:11: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-6]]:7: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-6]]:10: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-6]]:10: warning: identifiers containing double underscores are reserved to the implementation [cert-dcl51-cpp] + +namespace { + int _variable; + class _Classname { + void _Function(); + }; + namespace { + int _still_global; + } +} +// CHECK-MESSAGES: :[[@LINE-8]]:7: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-8]]:9: warning: do not use global names that start with an underscore [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-9]]:10: warning: identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation [cert-dcl51-cpp] +// CHECK-MESSAGES: :[[@LINE-7]]:9: warning: do not use global names that start with an underscore [cert-dcl51-cpp] + + +// Something that doesn't trigger the check: +struct X { + int *begin(); + int *end(); +}; + +void function_for() { + X x; + for(int i : x) { + // Intentionally left empty. + } +} + +#define MACRO_NAME +#define override +#define else_if + +#define const const +#define inline __inline + +int some_variable; + +void fun_ction() { + int _local_lower_variable; +} + +typedef int Integer; +using Char = char; + +template void function(); + +enum my_enum { + variable = 1, + other_variable = 2 +}; + +void Function() { + _label_: + if (true) { + goto _label_; + } +} + +namespace my_space { + int _variable; + struct _struct_name_ { + void _function(); + }; + namespace { + int _my_variable; + } +} + +namespace { + class Classname { + void _my_function(); + }; + bool operator==(Classname a, double b); +} Index: unittests/clangd/ClangdTests.cpp =================================================================== --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -422,9 +422,9 @@ MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer; MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true); - CDB.ExtraClangFlags.insert(CDB.ExtraClangFlags.end(), - {"-xc++", "-target", "x86_64-linux-unknown", - "-m64", "--gcc-toolchain=/randomusr"}); + CDB.ExtraClangFlags.insert( + CDB.ExtraClangFlags.end(), + {"-xc++", "-target", "x86_64-linux-unknown", "-m64"}); ClangdServer Server(CDB, DiagConsumer, FS, /*RunSynchronously=*/true); @@ -432,7 +432,7 @@ SmallString<8> Version("4.9.3"); // A lib dir for gcc installation - SmallString<64> LibDir("/randomusr/lib/gcc/x86_64-linux-gnu"); + SmallString<64> LibDir("/usr/lib/gcc/x86_64-linux-gnu"); llvm::sys::path::append(LibDir, Version); // Put crtbegin.o into LibDir/64 to trick clang into thinking there's a gcc @@ -441,7 +441,7 @@ llvm::sys::path::append(DummyLibFile, LibDir, "64", "crtbegin.o"); FS.Files[DummyLibFile] = ""; - SmallString<64> IncludeDir("/randomusr/include/c++"); + SmallString<64> IncludeDir("/usr/include/c++"); llvm::sys::path::append(IncludeDir, Version); SmallString<64> StringPath;