logan-5 updated this revision to Diff 236216. logan-5 added a comment. Change double-underscore fix-it finding algorithm to correctly collapse any number of >=2 underscores in a row, not just exactly 2 (or multiples of 2).
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72213/new/ https://reviews.llvm.org/D72213 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h clang-tools-extra/clang-tidy/utils/CMakeLists.txt clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/system/system-header.h clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/user-header.h clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp @@ -0,0 +1,183 @@ +// RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- -- \ +// RUN: -I%S/Inputs/bugprone-reserved-identifier \ +// RUN: -isystem %S/Inputs/bugprone-reserved-identifier/system + +// no warnings expected without -header-filter= +#include "user-header.h" +#include <system-header.h> + +#define _MACRO(m) int m = 0 +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier '_MACRO', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}#define MACRO(m) int m = 0{{$}} + +namespace _Ns { +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier '_Ns', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}namespace Ns {{{$}} + +class _Object { + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Object', which causes undefined behavior [bugprone-reserved-identifier] + // CHECK-FIXES: {{^}}class Object {{{$}} + int _Member; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Member', which causes undefined behavior [bugprone-reserved-identifier] + // CHECK-FIXES: {{^}} int Member;{{$}} +}; + +float _Global; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Global', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}float Global;{{$}} + +void _Function() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '_Function', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}void Function() {}{{$}} + +using _Alias = int; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Alias', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}using Alias = int;{{$}} + +} // namespace _Ns + +// + +#define __macro(m) int m = 0 +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier '__macro', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}} + +namespace __ns { +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier '__ns', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}namespace ns {{{$}} +class __object { + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__object', which causes undefined behavior [bugprone-reserved-identifier] + // CHECK-FIXES: {{^}}class object {{{$}} + int __member; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__member', which causes undefined behavior [bugprone-reserved-identifier] + // CHECK-FIXES: {{^}} int member;{{$}} +}; + +float __global; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__global', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}float global;{{$}} + +void __function() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '__function', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}void function() {}{{$}} + +using __alias = int; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__alias', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}using alias = int;{{$}} + +} // namespace __ns + +// + +#define macro___m(m) int m = 0 +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier 'macro___m', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}#define macro_m(m) int m = 0{{$}} + +namespace ns___n { +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier 'ns___n', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}namespace ns_n {{{$}} +class object___o { + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier 'object___o', which causes undefined behavior [bugprone-reserved-identifier] + // CHECK-FIXES: {{^}}class object_o {{{$}} + int member___m; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier 'member___m', which causes undefined behavior [bugprone-reserved-identifier] + // CHECK-FIXES: {{^}} int member_m;{{$}} +}; + +float global___g; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier 'global___g', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}float global_g;{{$}} + +void function___f() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier 'function___f', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}void function_f() {}{{$}} + +using alias___a = int; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier 'alias___a', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}using alias_a = int;{{$}} + +} // namespace ns___n + +// + +#define _macro(m) int m = 0 +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_macro', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}} + +namespace _ns { +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses identifier '_ns', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}namespace ns {{{$}} +int _i; +// no warning +} // namespace _ns +class _object { + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses identifier '_object', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier] + // CHECK-FIXES: {{^}}class object {{{$}} + int _member; + // no warning +}; +float _global; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses identifier '_global', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}float global;{{$}} +void _function() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses identifier '_function', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}void function() {}{{$}} +using _alias = int; +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses identifier '_alias', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}using alias = int;{{$}} + +void _float() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses identifier '_float', which is reserved in the global namespace; this causes undefined behavior; cannot be fixed because 'float' would conflict with a keyword [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}void _float() {}{{$}} + +#define SOME_MACRO +int SOME__MACRO; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses reserved identifier 'SOME__MACRO', which causes undefined behavior; cannot be fixed because 'SOME_MACRO' would conflict with a macro definition [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}int SOME__MACRO;{{$}} + +void _TWO__PROBLEMS() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '_TWO__PROBLEMS', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}void TWO_PROBLEMS() {}{{$}} +void _two__problems() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '_two__problems', which causes undefined behavior [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}void two_problems() {}{{$}} + +int __; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses reserved identifier '__', which causes undefined behavior; cannot be fixed automatically [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}int __;{{$}} + +int _________; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses reserved identifier '_________', which causes undefined behavior; cannot be fixed automatically [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}int _________;{{$}} + +int _; +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses identifier '_', which is reserved in the global namespace; this causes undefined behavior; cannot be fixed automatically [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}int _;{{$}} + +// these should pass +#define MACRO(m) int m = 0 + +namespace Ns { +class Object { + int Member; +}; +float Global; + +void Function() {} +using Alias = int; +} // namespace Ns +namespace ns_ { +class object_ { + int member_; +}; +float global_; +void function_() {} +using alias_ = int; +} // namespace ns_ + +class object_ { + int member_; +}; +float global_; +void function_() {} +using alias_ = int; Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp @@ -0,0 +1,70 @@ +// RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- \ +// RUN: -config='{CheckOptions: [ \ +// RUN: {key: bugprone-reserved-identifier.Invert, value: 1}, \ +// RUN: {key: bugprone-reserved-identifier.Whitelist, value: std;reference_wrapper;ref;cref;type;get}, \ +// RUN: ]}' -- \ +// RUN: -I%S/Inputs/bugprone-reserved-identifier \ +// RUN: -isystem %S/Inputs/bugprone-reserved-identifier/system + +namespace std { + +void __f() {} + +void f(); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses identifier 'f', which is not a reserved identifier [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}void __f();{{$}} +struct helper {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration uses identifier 'helper', which is not a reserved identifier [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}struct __helper {};{{$}} +struct Helper {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration uses identifier 'Helper', which is not a reserved identifier [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}struct _Helper {};{{$}} +struct _helper2 {}; +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration uses identifier '_helper2', which is not a reserved identifier [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}struct __helper2 {};{{$}} + +template <class _Tp> +class reference_wrapper { +public: + typedef _Tp type; + +private: + type *__f_; + +public: + reference_wrapper(type &__f) + : __f_(&__f) {} + // access + operator type &() const { return *__f_; } + type &get() const { return *__f_; } +}; + +template <class _Tp> +inline reference_wrapper<_Tp> +ref(_Tp &__t) noexcept { + return reference_wrapper<_Tp>(__t); +} + +template <class _Tp> +inline reference_wrapper<_Tp> +ref(reference_wrapper<_Tp> __t) noexcept { + return ref(__t.get()); +} + +template <class Up> +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: declaration uses identifier 'Up', which is not a reserved identifier [bugprone-reserved-identifier] +// CHECK-FIXES: {{^}}template <class _Up>{{$}} +inline reference_wrapper<const Up> +cref(const Up &u) noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: declaration uses identifier 'u', which is not a reserved identifier [bugprone-reserved-identifier] + // CHECK-FIXES: {{^}}cref(const Up &__u) noexcept {{{$}} + return reference_wrapper<const Up>(u); +} + +template <class _Tp> +inline reference_wrapper<_Tp> +cref(reference_wrapper<const _Tp> __t) noexcept { + return cref(__t.get()); +} + +} // namespace std Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/user-header.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/user-header.h @@ -0,0 +1,58 @@ +#define _HEADER_MACRO(m) int m = 0 + +namespace _Header_Ns { +class _Header_Object { + int _Header_Member; +}; + +float _Header_Global; + +void _Header_Function() {} + +using _Header_Alias = int; +} // namespace _Header_Ns + +// + +#define __header_macro(m) int m = 0 + +namespace __header_ns { +class __header_object { + int __header_member; +}; + +float __header_global; + +void __header_function() {} + +using __header_alias = int; +} // namespace __header_ns + +// + +#define header_macro__m(m) int m = 0 + +namespace header_ns__n { +class header_object__o { + int header_member__m; +}; + +float header_global__g; + +void header_function__f() {} + +using header_alias__a = int; +} // namespace header_ns__n + +// + +#define _header_macro(m) int m = 0 + +namespace _header_ns {} +class _header_object {}; + +float _header_global; + +void _header_function() {} + +using _header_alias = int; Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/system/system-header.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/system/system-header.h @@ -0,0 +1,33 @@ +namespace std { + +void __f() {} + +template <class _Tp> +class reference_wrapper { +public: + typedef _Tp type; + +private: + type *__f_; + +public: + reference_wrapper(type &__f) + : __f_(&__f) {} + // access + operator type &() const { return *__f_; } + type &get() const { return *__f_; } +}; + +template <class _Tp> +inline reference_wrapper<_Tp> +ref(_Tp &__t) noexcept { + return reference_wrapper<_Tp>(__t); +} + +template <class _Tp> +inline reference_wrapper<_Tp> +ref(reference_wrapper<_Tp> __t) noexcept { + return ref(__t.get()); +} + +} // namespace std Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst @@ -0,0 +1,33 @@ +.. title:: clang-tidy - bugprone-reserved-identifier + +bugprone-reserved-identifier +============================ + +Checks for usages of identifiers reserved for use by the C++ implementation. The C++ standard reserves the following names for such use: +* identifiers with a double underscore anywhere; +* identifiers that begin with an underscore followed by an uppercase letter; +* identifiers in the global namespace that begin with an underscore. + +Violating the naming rules above results in undefined behavior. + +.. code-block:: c++ + + namespace NS { + void __f(); // name is not allowed in user code + using _Int = int; // same with this + #define cool__macro // also this + } + int _g(); // disallowed in global namespace only + +The check can also be inverted, i.e. it can be configured to flag any identifier that is _not_ a reserved identifier. This mode is for use by e.g. standard library implementors, to ensure they don't infringe on the user namespace. + +Options +------- + +.. option:: Invert + + If non-zero, inverts the check, i.e. flags names that are not reserved. Default is `0`. + +.. option:: Whitelist + + Semicolon-separated list of names that the check ignores. Default is an empty list. Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -94,6 +94,16 @@ Without the null terminator it can result in undefined behaviour when the string is read. +- New :doc:`bugprone-reserved-identifier + <clang-tidy/checks/bugprone-reserved-identifier>` check. + + Checks for usages of identifiers reserved for use by the C++ implementation. The C++ standard reserves the following names for such use: + * identifiers with a double underscore anywhere; + * identifiers that begin with an underscore followed by an uppercase letter; + * identifiers in the global namespace that begin with an underscore. + + Violating the naming rules above results in undefined behavior. + - New :doc:`cert-mem57-cpp <clang-tidy/checks/cert-mem57-cpp>` check. Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h @@ -0,0 +1,145 @@ +//===--- RenamderClangTidyCheck.h - clang-tidy ------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H + +#include "../ClangTidyCheck.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/Optional.h" +#include <string> +#include <utility> + +namespace clang { + +class MacroInfo; + +namespace tidy { + +/// Base class for clang-tidy checks that want to flag declarations and/or +/// macros for renaming based on customizable criteria. +class RenamerClangTidyCheck : public ClangTidyCheck { +public: + RenamerClangTidyCheck(StringRef CheckName, ClangTidyContext *Context); + ~RenamerClangTidyCheck(); + + /// Derived classes should not implement any matching logic themselves; this + /// class will do the matching and call the derived class' + /// GetDeclFailureInfo() and GetMacroFailureInfo() for determining whether a + /// given identifier passes or fails the check. + void registerMatchers(ast_matchers::MatchFinder *Finder) override final; + void + check(const ast_matchers::MatchFinder::MatchResult &Result) override final; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override final; + void onEndOfTranslationUnit() override final; + + /// This enum will be used in %select of the diagnostic message. + /// Each value below IgnoreFailureThreshold should have an error message. + enum class ShouldFixStatus { + ShouldFix, + ConflictsWithKeyword, /// The fixup will conflict with a language keyword, + /// so we can't fix it automatically. + ConflictsWithMacroDefinition, /// The fixup will conflict with a macro + /// definition, so we can't fix it + /// automatically. + + /// Values pass this threshold will be ignored completely + /// i.e no message, no fixup. + IgnoreFailureThreshold, + + InsideMacro, /// If the identifier was used or declared within a macro we + /// won't offer a fixup for safety reasons. + }; + + /// Information describing a failed check + struct FailureInfo { + std::string KindName; // Tag or misc info to be used as derived classes need + std::string Fixup; // The name that will be proposed as a fix-it hint + }; + + /// Holds an identifier name check failure, tracking the kind of the + /// identifier, its possible fixup and the starting locations of all the + /// identifier usages. + struct NamingCheckFailure { + FailureInfo Info; + + /// Whether the failure should be fixed or not. + /// + /// e.g.: if the identifier was used or declared within a macro we won't + /// offer a fixup for safety reasons. + bool ShouldFix() const { + return FixStatus == ShouldFixStatus::ShouldFix && !Info.Fixup.empty(); + } + + bool ShouldNotify() const { + return FixStatus < ShouldFixStatus::IgnoreFailureThreshold; + } + + ShouldFixStatus FixStatus = ShouldFixStatus::ShouldFix; + + /// A set of all the identifier usages starting SourceLocation, in + /// their encoded form. + llvm::DenseSet<unsigned> RawUsageLocs; + + NamingCheckFailure() = default; + }; + + typedef std::pair<SourceLocation, std::string> NamingCheckId; + + typedef llvm::DenseMap<NamingCheckId, NamingCheckFailure> + NamingCheckFailureMap; + + /// Check Macros for style violations. + void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok, + const MacroInfo *MI); + + /// Add a usage of a macro if it already has a violation. + void expandMacro(const Token &MacroNameTok, const MacroInfo *MI); + +protected: + /// Overridden by derived classes, returns information about if and how a Decl + /// failed the check. A 'None' result means the Decl did not fail the check. + virtual llvm::Optional<FailureInfo> + GetDeclFailureInfo(const NamedDecl *Decl, const SourceManager &SM) const = 0; + + /// Overridden by derived classes, returns information about if and how a + /// macro failed the check. A 'None' result means the macro did not fail the + /// check. + virtual llvm::Optional<FailureInfo> + GetMacroFailureInfo(const Token &MacroNameTok, + const SourceManager &SM) const = 0; + + /// Represents customized diagnostic text and how arguments should be applied. + /// Example usage: + /// + /// return DiagInfo{"my %1 very %2 special %3 text", + /// [=](DiagnosticBuilder &diag) { + /// diag << arg1 << arg2 << arg3; + /// }}; + struct DiagInfo { + std::string Text; + llvm::unique_function<void(DiagnosticBuilder &)> ApplyArgs; + }; + + /// Overridden by derived classes, returns a description of the diagnostic + /// that should be emitted for the given failure. The base class will then + /// further customize the diagnostic by adding info about whether the fix-it + /// can be automatically applied or not. + virtual DiagInfo GetDiagInfo(const NamingCheckId &ID, + const NamingCheckFailure &Failure) const = 0; + +private: + NamingCheckFailureMap NamingCheckFailures; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_RENAMERCLANGTIDYCHECK_H Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp @@ -0,0 +1,411 @@ +//===--- RenamerClangTidyCheck.cpp - clang-tidy ---------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "RenamerClangTidyCheck.h" +#include "ASTUtils.h" +#include "clang/AST/CXXInheritance.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/PPCallbacks.h" +#include "clang/Lex/Preprocessor.h" +#include "llvm/ADT/DenseMapInfo.h" +#include "llvm/Support/Debug.h" +#include "llvm/Support/Format.h" + +#define DEBUG_TYPE "clang-tidy" + +using namespace clang::ast_matchers; + +namespace llvm { + +/// Specialisation of DenseMapInfo to allow NamingCheckId objects in DenseMaps +template <> +struct DenseMapInfo<clang::tidy::RenamerClangTidyCheck::NamingCheckId> { + using NamingCheckId = clang::tidy::RenamerClangTidyCheck::NamingCheckId; + + static inline NamingCheckId getEmptyKey() { + return NamingCheckId( + clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-1)), + "EMPTY"); + } + + static inline NamingCheckId getTombstoneKey() { + return NamingCheckId( + clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-2)), + "TOMBSTONE"); + } + + static unsigned getHashValue(NamingCheckId Val) { + assert(Val != getEmptyKey() && "Cannot hash the empty key!"); + assert(Val != getTombstoneKey() && "Cannot hash the tombstone key!"); + + std::hash<NamingCheckId::second_type> SecondHash; + return Val.first.getRawEncoding() + SecondHash(Val.second); + } + + static bool isEqual(const NamingCheckId &LHS, const NamingCheckId &RHS) { + if (RHS == getEmptyKey()) + return LHS == getEmptyKey(); + if (RHS == getTombstoneKey()) + return LHS == getTombstoneKey(); + return LHS == RHS; + } +}; + +} // namespace llvm + +namespace clang { +namespace tidy { + +namespace { + +/// Callback supplies macros to RenamerClangTidyCheck::checkMacro +class RenamerClangTidyCheckPPCallbacks : public PPCallbacks { +public: + RenamerClangTidyCheckPPCallbacks(Preprocessor *PP, + RenamerClangTidyCheck *Check) + : PP(PP), Check(Check) {} + + /// MacroDefined calls checkMacro for macros in the main file + void MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) override { + Check->checkMacro(PP->getSourceManager(), MacroNameTok, MD->getMacroInfo()); + } + + /// MacroExpands calls expandMacro for macros in the main file + void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, + SourceRange /*Range*/, + const MacroArgs * /*Args*/) override { + Check->expandMacro(MacroNameTok, MD.getMacroInfo()); + } + +private: + Preprocessor *PP; + RenamerClangTidyCheck *Check; +}; + +} // namespace + +RenamerClangTidyCheck::RenamerClangTidyCheck(StringRef CheckName, + ClangTidyContext *Context) + : ClangTidyCheck(CheckName, Context) {} +RenamerClangTidyCheck::~RenamerClangTidyCheck() = default; + +void RenamerClangTidyCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(namedDecl().bind("decl"), this); + Finder->addMatcher(usingDecl().bind("using"), this); + Finder->addMatcher(declRefExpr().bind("declRef"), this); + Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this); + Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this); + Finder->addMatcher(typeLoc().bind("typeLoc"), this); + Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this); +} + +void RenamerClangTidyCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + ModuleExpanderPP->addPPCallbacks( + std::make_unique<RenamerClangTidyCheckPPCallbacks>(ModuleExpanderPP, + this)); +} + +static void addUsage(RenamerClangTidyCheck::NamingCheckFailureMap &Failures, + const RenamerClangTidyCheck::NamingCheckId &Decl, + SourceRange Range, SourceManager *SourceMgr = nullptr) { + // Do nothing if the provided range is invalid. + if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid()) + return; + + // If we have a source manager, use it to convert to the spelling location for + // performing the fix. This is necessary because macros can map the same + // spelling location to different source locations, and we only want to fix + // the token once, before it is expanded by the macro. + SourceLocation FixLocation = Range.getBegin(); + if (SourceMgr) + FixLocation = SourceMgr->getSpellingLoc(FixLocation); + if (FixLocation.isInvalid()) + return; + + // Try to insert the identifier location in the Usages map, and bail out if it + // is already in there + RenamerClangTidyCheck::NamingCheckFailure &Failure = Failures[Decl]; + if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second) + return; + + if (!Failure.ShouldFix()) + return; + + if (!utils::rangeCanBeFixed(Range, SourceMgr)) + Failure.FixStatus = RenamerClangTidyCheck::ShouldFixStatus::InsideMacro; +} + +/// Convenience method when the usage to be added is a NamedDecl +static void addUsage(RenamerClangTidyCheck::NamingCheckFailureMap &Failures, + const NamedDecl *Decl, SourceRange Range, + SourceManager *SourceMgr = nullptr) { + return addUsage(Failures, + RenamerClangTidyCheck::NamingCheckId(Decl->getLocation(), + Decl->getNameAsString()), + Range, SourceMgr); +} + +void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *Decl = + Result.Nodes.getNodeAs<CXXConstructorDecl>("classRef")) { + if (Decl->isImplicit()) + return; + + addUsage(NamingCheckFailures, Decl->getParent(), + Decl->getNameInfo().getSourceRange()); + + for (const auto *Init : Decl->inits()) { + if (!Init->isWritten() || Init->isInClassMemberInitializer()) + continue; + if (const FieldDecl *FD = Init->getAnyMember()) + addUsage(NamingCheckFailures, FD, + SourceRange(Init->getMemberLocation())); + // Note: delegating constructors and base class initializers are handled + // via the "typeLoc" matcher. + } + return; + } + + if (const auto *Decl = + Result.Nodes.getNodeAs<CXXDestructorDecl>("classRef")) { + if (Decl->isImplicit()) + return; + + SourceRange Range = Decl->getNameInfo().getSourceRange(); + if (Range.getBegin().isInvalid()) + return; + // The first token that will be found is the ~ (or the equivalent trigraph), + // we want instead to replace the next token, that will be the identifier. + Range.setBegin(CharSourceRange::getTokenRange(Range).getEnd()); + + addUsage(NamingCheckFailures, Decl->getParent(), Range); + return; + } + + if (const auto *Loc = Result.Nodes.getNodeAs<TypeLoc>("typeLoc")) { + NamedDecl *Decl = nullptr; + if (const auto &Ref = Loc->getAs<TagTypeLoc>()) { + Decl = Ref.getDecl(); + } else if (const auto &Ref = Loc->getAs<InjectedClassNameTypeLoc>()) { + Decl = Ref.getDecl(); + } else if (const auto &Ref = Loc->getAs<UnresolvedUsingTypeLoc>()) { + Decl = Ref.getDecl(); + } else if (const auto &Ref = Loc->getAs<TemplateTypeParmTypeLoc>()) { + Decl = Ref.getDecl(); + } + + if (Decl) { + addUsage(NamingCheckFailures, Decl, Loc->getSourceRange()); + return; + } + + if (const auto &Ref = Loc->getAs<TemplateSpecializationTypeLoc>()) { + const TemplateDecl *Decl = + Ref.getTypePtr()->getTemplateName().getAsTemplateDecl(); + + SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc()); + if (const auto *ClassDecl = dyn_cast<TemplateDecl>(Decl)) { + if (const NamedDecl *TemplDecl = ClassDecl->getTemplatedDecl()) + addUsage(NamingCheckFailures, TemplDecl, Range); + return; + } + } + + if (const auto &Ref = + Loc->getAs<DependentTemplateSpecializationTypeLoc>()) { + if (const TagDecl *Decl = Ref.getTypePtr()->getAsTagDecl()) + addUsage(NamingCheckFailures, Decl, Loc->getSourceRange()); + return; + } + } + + if (const auto *Loc = + Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("nestedNameLoc")) { + if (const NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) { + if (const NamespaceDecl *Decl = Spec->getAsNamespace()) { + addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange()); + return; + } + } + } + + if (const auto *Decl = Result.Nodes.getNodeAs<UsingDecl>("using")) { + for (const auto *Shadow : Decl->shadows()) { + addUsage(NamingCheckFailures, Shadow->getTargetDecl(), + Decl->getNameInfo().getSourceRange()); + } + return; + } + + if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declRef")) { + SourceRange Range = DeclRef->getNameInfo().getSourceRange(); + addUsage(NamingCheckFailures, DeclRef->getDecl(), Range, + Result.SourceManager); + return; + } + + if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) { + if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit()) + return; + + // Fix type aliases in value declarations + if (const auto *Value = Result.Nodes.getNodeAs<ValueDecl>("decl")) { + if (const Type *TypePtr = Value->getType().getTypePtrOrNull()) { + if (const auto *Typedef = TypePtr->getAs<TypedefType>()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + } + } + + // Fix type aliases in function declarations + if (const auto *Value = Result.Nodes.getNodeAs<FunctionDecl>("decl")) { + if (const auto *Typedef = + Value->getReturnType().getTypePtr()->getAs<TypedefType>()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + for (unsigned i = 0; i < Value->getNumParams(); ++i) { + if (const TypedefType *Typedef = Value->parameters()[i] + ->getType() + .getTypePtr() + ->getAs<TypedefType>()) { + addUsage(NamingCheckFailures, Typedef->getDecl(), + Value->getSourceRange()); + } + } + } + + // Ignore ClassTemplateSpecializationDecl which are creating duplicate + // replacements with CXXRecordDecl + if (isa<ClassTemplateSpecializationDecl>(Decl)) + return; + + Optional<FailureInfo> MaybeFailure = + GetDeclFailureInfo(Decl, *Result.SourceManager); + if (!MaybeFailure) + return; + FailureInfo &Info = *MaybeFailure; + NamingCheckFailure &Failure = NamingCheckFailures[NamingCheckId( + Decl->getLocation(), Decl->getNameAsString())]; + SourceRange Range = + DeclarationNameInfo(Decl->getDeclName(), Decl->getLocation()) + .getSourceRange(); + + const IdentifierTable &Idents = Decl->getASTContext().Idents; + auto CheckNewIdentifier = Idents.find(Info.Fixup); + if (CheckNewIdentifier != Idents.end()) { + const IdentifierInfo *Ident = CheckNewIdentifier->second; + if (Ident->isKeyword(getLangOpts())) + Failure.FixStatus = ShouldFixStatus::ConflictsWithKeyword; + else if (Ident->hasMacroDefinition()) + Failure.FixStatus = ShouldFixStatus::ConflictsWithMacroDefinition; + } + + Failure.Info = std::move(Info); + addUsage(NamingCheckFailures, Decl, Range); + } +} + +void RenamerClangTidyCheck::checkMacro(SourceManager &SourceMgr, + const Token &MacroNameTok, + const MacroInfo *MI) { + Optional<FailureInfo> MaybeFailure = + GetMacroFailureInfo(MacroNameTok, SourceMgr); + if (!MaybeFailure) + return; + FailureInfo &Info = *MaybeFailure; + StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); + NamingCheckId ID(MI->getDefinitionLoc(), Name); + NamingCheckFailure &Failure = NamingCheckFailures[ID]; + SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc()); + + Failure.Info = std::move(Info); + addUsage(NamingCheckFailures, ID, Range); +} + +void RenamerClangTidyCheck::expandMacro(const Token &MacroNameTok, + const MacroInfo *MI) { + StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); + NamingCheckId ID(MI->getDefinitionLoc(), Name); + + auto Failure = NamingCheckFailures.find(ID); + if (Failure == NamingCheckFailures.end()) + return; + + SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc()); + addUsage(NamingCheckFailures, ID, Range); +} + +static std::string +getDiagnosticSuffix(const RenamerClangTidyCheck::ShouldFixStatus FixStatus, + const std::string &Fixup) { + if (Fixup.empty()) { + return "; cannot be fixed automatically"; + } + if (FixStatus == RenamerClangTidyCheck::ShouldFixStatus::ShouldFix) { + return {}; + } + if (FixStatus >= + RenamerClangTidyCheck::ShouldFixStatus::IgnoreFailureThreshold) { + return {}; + } + if (FixStatus == + RenamerClangTidyCheck::ShouldFixStatus::ConflictsWithKeyword) { + return "; cannot be fixed because '" + Fixup + + "' would conflict with a keyword"; + } + if (FixStatus == + RenamerClangTidyCheck::ShouldFixStatus::ConflictsWithMacroDefinition) { + return "; cannot be fixed because '" + Fixup + + "' would conflict with a macro definition"; + } + llvm_unreachable("invalid ShouldFixStatus"); +} + +void RenamerClangTidyCheck::onEndOfTranslationUnit() { + for (const auto &Pair : NamingCheckFailures) { + const NamingCheckId &Decl = Pair.first; + const NamingCheckFailure &Failure = Pair.second; + + if (Failure.Info.KindName.empty()) + continue; + + if (Failure.ShouldNotify()) { + auto DiagInfo = GetDiagInfo(Decl, Failure); + auto Diag = diag(Decl.first, + DiagInfo.Text + getDiagnosticSuffix(Failure.FixStatus, + Failure.Info.Fixup)); + DiagInfo.ApplyArgs(Diag); + + if (Failure.ShouldFix()) { + for (const auto &Loc : Failure.RawUsageLocs) { + // We assume that the identifier name is made of one token only. This + // is always the case as we ignore usages in macros that could build + // identifier names by combining multiple tokens. + // + // For destructors, we already take care of it by remembering the + // location of the start of the identifier and not the start of the + // tilde. + // + // Other multi-token identifiers, such as operators are not checked at + // all. + Diag << FixItHint::CreateReplacement( + SourceRange(SourceLocation::getFromRawEncoding(Loc)), + Failure.Info.Fixup); + } + } + } + } +} + +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/utils/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/utils/CMakeLists.txt +++ clang-tools-extra/clang-tidy/utils/CMakeLists.txt @@ -13,6 +13,7 @@ LexerUtils.cpp NamespaceAliaser.cpp OptionsUtils.cpp + RenamerClangTidyCheck.cpp TransformerClangTidyCheck.cpp TypeTraits.cpp UsingInserter.cpp Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h =================================================================== --- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h +++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h @@ -9,8 +9,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERNAMINGCHECK_H #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_IDENTIFIERNAMINGCHECK_H -#include "../ClangTidyCheck.h" - +#include "../utils/RenamerClangTidyCheck.h" namespace clang { class MacroInfo; @@ -31,17 +30,12 @@ /// different rules for different kind of identifier. In general, the /// rules are falling back to a more generic rule if the specific case is not /// configured. -class IdentifierNamingCheck : public ClangTidyCheck { +class IdentifierNamingCheck final : public RenamerClangTidyCheck { public: IdentifierNamingCheck(StringRef Name, ClangTidyContext *Context); ~IdentifierNamingCheck(); void storeOptions(ClangTidyOptions::OptionMap &Opts) override; - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, - Preprocessor *ModuleExpanderPP) override; - void onEndOfTranslationUnit() override; enum CaseType { CT_AnyCase = 0, @@ -65,66 +59,18 @@ std::string Suffix; }; - /// This enum will be used in %select of the diagnostic message. - /// Each value below IgnoreFailureThreshold should have an error message. - enum class ShouldFixStatus { - ShouldFix, - ConflictsWithKeyword, /// The fixup will conflict with a language keyword, - /// so we can't fix it automatically. - ConflictsWithMacroDefinition, /// The fixup will conflict with a macro - /// definition, so we can't fix it - /// automatically. - - /// Values pass this threshold will be ignored completely - /// i.e no message, no fixup. - IgnoreFailureThreshold, - - InsideMacro, /// If the identifier was used or declared within a macro we - /// won't offer a fixup for safety reasons. - }; - - /// Holds an identifier name check failure, tracking the kind of the - /// identifier, its possible fixup and the starting locations of all the - /// identifier usages. - struct NamingCheckFailure { - std::string KindName; - std::string Fixup; - - /// Whether the failure should be fixed or not. - /// - /// ie: if the identifier was used or declared within a macro we won't offer - /// a fixup for safety reasons. - bool ShouldFix() const { return FixStatus == ShouldFixStatus::ShouldFix; } - - bool ShouldNotify() const { - return FixStatus < ShouldFixStatus::IgnoreFailureThreshold; - } - - ShouldFixStatus FixStatus = ShouldFixStatus::ShouldFix; - - /// A set of all the identifier usages starting SourceLocation, in - /// their encoded form. - llvm::DenseSet<unsigned> RawUsageLocs; - - NamingCheckFailure() = default; - }; - - typedef std::pair<SourceLocation, std::string> NamingCheckId; - - typedef llvm::DenseMap<NamingCheckId, NamingCheckFailure> - NamingCheckFailureMap; - - /// Check Macros for style violations. - void checkMacro(SourceManager &sourceMgr, const Token &MacroNameTok, - const MacroInfo *MI); - - /// Add a usage of a macro if it already has a violation. - void expandMacro(const Token &MacroNameTok, const MacroInfo *MI); - private: + llvm::Optional<FailureInfo> + GetDeclFailureInfo(const NamedDecl *Decl, + const SourceManager &SM) const override; + llvm::Optional<FailureInfo> + GetMacroFailureInfo(const Token &MacroNameTok, + const SourceManager &SM) const override; + DiagInfo GetDiagInfo(const NamingCheckId &ID, + const NamingCheckFailure &Failure) const override; + std::vector<llvm::Optional<NamingStyle>> NamingStyles; bool IgnoreFailedSplit; - NamingCheckFailureMap NamingCheckFailures; }; } // namespace readability Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -22,44 +22,6 @@ using namespace clang::ast_matchers; -namespace llvm { -/// Specialisation of DenseMapInfo to allow NamingCheckId objects in DenseMaps -template <> -struct DenseMapInfo< - clang::tidy::readability::IdentifierNamingCheck::NamingCheckId> { - using NamingCheckId = - clang::tidy::readability::IdentifierNamingCheck::NamingCheckId; - - static inline NamingCheckId getEmptyKey() { - return NamingCheckId( - clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-1)), - "EMPTY"); - } - - static inline NamingCheckId getTombstoneKey() { - return NamingCheckId( - clang::SourceLocation::getFromRawEncoding(static_cast<unsigned>(-2)), - "TOMBSTONE"); - } - - static unsigned getHashValue(NamingCheckId Val) { - assert(Val != getEmptyKey() && "Cannot hash the empty key!"); - assert(Val != getTombstoneKey() && "Cannot hash the tombstone key!"); - - std::hash<NamingCheckId::second_type> SecondHash; - return Val.first.getRawEncoding() + SecondHash(Val.second); - } - - static bool isEqual(const NamingCheckId &LHS, const NamingCheckId &RHS) { - if (RHS == getEmptyKey()) - return LHS == getEmptyKey(); - if (RHS == getTombstoneKey()) - return LHS == getTombstoneKey(); - return LHS == RHS; - } -}; -} // namespace llvm - namespace clang { namespace tidy { namespace readability { @@ -164,7 +126,7 @@ IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) { + : RenamerClangTidyCheck(Name, Context) { auto const fromString = [](StringRef Str) { return llvm::StringSwitch<llvm::Optional<CaseType>>(Str) .Case("aNy_CasE", CT_AnyCase) @@ -233,23 +195,6 @@ Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit); } -void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher(namedDecl().bind("decl"), this); - Finder->addMatcher(usingDecl().bind("using"), this); - Finder->addMatcher(declRefExpr().bind("declRef"), this); - Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this); - Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this); - Finder->addMatcher(typeLoc().bind("typeLoc"), this); - Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this); -} - -void IdentifierNamingCheck::registerPPCallbacks( - const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { - ModuleExpanderPP->addPPCallbacks( - std::make_unique<IdentifierNamingCheckPPCallbacks>(ModuleExpanderPP, - this)); -} - static bool matchesStyle(StringRef Name, IdentifierNamingCheck::NamingStyle Style) { static llvm::Regex Matchers[] = { @@ -667,239 +612,47 @@ return SK_Invalid; } -static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures, - const IdentifierNamingCheck::NamingCheckId &Decl, - SourceRange Range, SourceManager *SourceMgr = nullptr) { - // Do nothing if the provided range is invalid. - if (Range.getBegin().isInvalid() || Range.getEnd().isInvalid()) - return; - - // If we have a source manager, use it to convert to the spelling location for - // performing the fix. This is necessary because macros can map the same - // spelling location to different source locations, and we only want to fix - // the token once, before it is expanded by the macro. - SourceLocation FixLocation = Range.getBegin(); - if (SourceMgr) - FixLocation = SourceMgr->getSpellingLoc(FixLocation); - if (FixLocation.isInvalid()) - return; - - // Try to insert the identifier location in the Usages map, and bail out if it - // is already in there - auto &Failure = Failures[Decl]; - if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second) - return; - - if (!Failure.ShouldFix()) - return; - - if (!utils::rangeCanBeFixed(Range, SourceMgr)) - Failure.FixStatus = IdentifierNamingCheck::ShouldFixStatus::InsideMacro; -} - -/// Convenience method when the usage to be added is a NamedDecl -static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures, - const NamedDecl *Decl, SourceRange Range, - SourceManager *SourceMgr = nullptr) { - return addUsage(Failures, - IdentifierNamingCheck::NamingCheckId(Decl->getLocation(), - Decl->getNameAsString()), - Range, SourceMgr); -} - -void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) { - if (const auto *Decl = - Result.Nodes.getNodeAs<CXXConstructorDecl>("classRef")) { - if (Decl->isImplicit()) - return; - - addUsage(NamingCheckFailures, Decl->getParent(), - Decl->getNameInfo().getSourceRange()); - - for (const auto *Init : Decl->inits()) { - if (!Init->isWritten() || Init->isInClassMemberInitializer()) - continue; - if (const auto *FD = Init->getAnyMember()) - addUsage(NamingCheckFailures, FD, - SourceRange(Init->getMemberLocation())); - // Note: delegating constructors and base class initializers are handled - // via the "typeLoc" matcher. - } - return; - } - - if (const auto *Decl = - Result.Nodes.getNodeAs<CXXDestructorDecl>("classRef")) { - if (Decl->isImplicit()) - return; - - SourceRange Range = Decl->getNameInfo().getSourceRange(); - if (Range.getBegin().isInvalid()) - return; - // The first token that will be found is the ~ (or the equivalent trigraph), - // we want instead to replace the next token, that will be the identifier. - Range.setBegin(CharSourceRange::getTokenRange(Range).getEnd()); - - addUsage(NamingCheckFailures, Decl->getParent(), Range); - return; - } - - if (const auto *Loc = Result.Nodes.getNodeAs<TypeLoc>("typeLoc")) { - NamedDecl *Decl = nullptr; - if (const auto &Ref = Loc->getAs<TagTypeLoc>()) { - Decl = Ref.getDecl(); - } else if (const auto &Ref = Loc->getAs<InjectedClassNameTypeLoc>()) { - Decl = Ref.getDecl(); - } else if (const auto &Ref = Loc->getAs<UnresolvedUsingTypeLoc>()) { - Decl = Ref.getDecl(); - } else if (const auto &Ref = Loc->getAs<TemplateTypeParmTypeLoc>()) { - Decl = Ref.getDecl(); - } - - if (Decl) { - addUsage(NamingCheckFailures, Decl, Loc->getSourceRange()); - return; - } - - if (const auto &Ref = Loc->getAs<TemplateSpecializationTypeLoc>()) { - const auto *Decl = - Ref.getTypePtr()->getTemplateName().getAsTemplateDecl(); - - SourceRange Range(Ref.getTemplateNameLoc(), Ref.getTemplateNameLoc()); - if (const auto *ClassDecl = dyn_cast<TemplateDecl>(Decl)) { - if (const auto *TemplDecl = ClassDecl->getTemplatedDecl()) - addUsage(NamingCheckFailures, TemplDecl, Range); - return; - } - } - - if (const auto &Ref = - Loc->getAs<DependentTemplateSpecializationTypeLoc>()) { - if (const auto *Decl = Ref.getTypePtr()->getAsTagDecl()) - addUsage(NamingCheckFailures, Decl, Loc->getSourceRange()); - return; - } - } - - if (const auto *Loc = - Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("nestedNameLoc")) { - if (NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) { - if (NamespaceDecl *Decl = Spec->getAsNamespace()) { - addUsage(NamingCheckFailures, Decl, Loc->getLocalSourceRange()); - return; - } - } - } +llvm::Optional<RenamerClangTidyCheck::FailureInfo> +IdentifierNamingCheck::GetDeclFailureInfo(const NamedDecl *Decl, + const SourceManager &SM) const { + StyleKind SK = findStyleKind(Decl, NamingStyles); + if (SK == SK_Invalid) + return None; - if (const auto *Decl = Result.Nodes.getNodeAs<UsingDecl>("using")) { - for (const auto *Shadow : Decl->shadows()) { - addUsage(NamingCheckFailures, Shadow->getTargetDecl(), - Decl->getNameInfo().getSourceRange()); - } - return; - } - - if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>("declRef")) { - SourceRange Range = DeclRef->getNameInfo().getSourceRange(); - addUsage(NamingCheckFailures, DeclRef->getDecl(), Range, - Result.SourceManager); - return; - } + if (!NamingStyles[SK]) + return None; - if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) { - if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit()) - return; - - // Fix type aliases in value declarations - if (const auto *Value = Result.Nodes.getNodeAs<ValueDecl>("decl")) { - if (const auto *TypePtr = Value->getType().getTypePtrOrNull()) { - if (const auto *Typedef = TypePtr->getAs<TypedefType>()) { - addUsage(NamingCheckFailures, Typedef->getDecl(), - Value->getSourceRange()); - } - } - } - - // Fix type aliases in function declarations - if (const auto *Value = Result.Nodes.getNodeAs<FunctionDecl>("decl")) { - if (const auto *Typedef = - Value->getReturnType().getTypePtr()->getAs<TypedefType>()) { - addUsage(NamingCheckFailures, Typedef->getDecl(), - Value->getSourceRange()); - } - for (unsigned i = 0; i < Value->getNumParams(); ++i) { - if (const auto *Typedef = Value->parameters()[i] - ->getType() - .getTypePtr() - ->getAs<TypedefType>()) { - addUsage(NamingCheckFailures, Typedef->getDecl(), - Value->getSourceRange()); - } - } - } + const NamingStyle &Style = *NamingStyles[SK]; + StringRef Name = Decl->getName(); + if (matchesStyle(Name, Style)) + return None; - // Ignore ClassTemplateSpecializationDecl which are creating duplicate - // replacements with CXXRecordDecl - if (isa<ClassTemplateSpecializationDecl>(Decl)) - return; - - StyleKind SK = findStyleKind(Decl, NamingStyles); - if (SK == SK_Invalid) - return; - - if (!NamingStyles[SK]) - return; - - const NamingStyle &Style = *NamingStyles[SK]; - StringRef Name = Decl->getName(); - if (matchesStyle(Name, Style)) - return; - - std::string KindName = fixupWithCase(StyleNames[SK], CT_LowerCase); - std::replace(KindName.begin(), KindName.end(), '_', ' '); - - std::string Fixup = fixupWithStyle(Name, Style); - if (StringRef(Fixup).equals(Name)) { - if (!IgnoreFailedSplit) { - LLVM_DEBUG(llvm::dbgs() - << Decl->getBeginLoc().printToString(*Result.SourceManager) - << llvm::format(": unable to split words for %s '%s'\n", - KindName.c_str(), Name.str().c_str())); - } - } else { - NamingCheckFailure &Failure = NamingCheckFailures[NamingCheckId( - Decl->getLocation(), Decl->getNameAsString())]; - SourceRange Range = - DeclarationNameInfo(Decl->getDeclName(), Decl->getLocation()) - .getSourceRange(); - - const IdentifierTable &Idents = Decl->getASTContext().Idents; - auto CheckNewIdentifier = Idents.find(Fixup); - if (CheckNewIdentifier != Idents.end()) { - const IdentifierInfo *Ident = CheckNewIdentifier->second; - if (Ident->isKeyword(getLangOpts())) - Failure.FixStatus = ShouldFixStatus::ConflictsWithKeyword; - else if (Ident->hasMacroDefinition()) - Failure.FixStatus = ShouldFixStatus::ConflictsWithMacroDefinition; - } + std::string KindName = fixupWithCase(StyleNames[SK], CT_LowerCase); + std::replace(KindName.begin(), KindName.end(), '_', ' '); - Failure.Fixup = std::move(Fixup); - Failure.KindName = std::move(KindName); - addUsage(NamingCheckFailures, Decl, Range); + std::string Fixup = fixupWithStyle(Name, Style); + if (StringRef(Fixup).equals(Name)) { + if (!IgnoreFailedSplit) { + LLVM_DEBUG(llvm::dbgs() + << Decl->getBeginLoc().printToString(SM) + << llvm::format(": unable to split words for %s '%s'\n", + KindName.c_str(), Name.str().c_str())); } + return None; } + return FailureInfo{std::move(KindName), std::move(Fixup)}; } -void IdentifierNamingCheck::checkMacro(SourceManager &SourceMgr, - const Token &MacroNameTok, - const MacroInfo *MI) { +llvm::Optional<RenamerClangTidyCheck::FailureInfo> +IdentifierNamingCheck::GetMacroFailureInfo(const Token &MacroNameTok, + const SourceManager &SM) const { if (!NamingStyles[SK_MacroDefinition]) - return; + return None; StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); const NamingStyle &Style = *NamingStyles[SK_MacroDefinition]; if (matchesStyle(Name, Style)) - return; + return None; std::string KindName = fixupWithCase(StyleNames[SK_MacroDefinition], CT_LowerCase); @@ -909,74 +662,22 @@ if (StringRef(Fixup).equals(Name)) { if (!IgnoreFailedSplit) { LLVM_DEBUG(llvm::dbgs() - << MacroNameTok.getLocation().printToString(SourceMgr) + << MacroNameTok.getLocation().printToString(SM) << llvm::format(": unable to split words for %s '%s'\n", KindName.c_str(), Name.str().c_str())); } - } else { - NamingCheckId ID(MI->getDefinitionLoc(), Name); - NamingCheckFailure &Failure = NamingCheckFailures[ID]; - SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc()); - - Failure.Fixup = std::move(Fixup); - Failure.KindName = std::move(KindName); - addUsage(NamingCheckFailures, ID, Range); + return None; } + return FailureInfo{std::move(KindName), std::move(Fixup)}; } -void IdentifierNamingCheck::expandMacro(const Token &MacroNameTok, - const MacroInfo *MI) { - StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); - NamingCheckId ID(MI->getDefinitionLoc(), Name); - - auto Failure = NamingCheckFailures.find(ID); - if (Failure == NamingCheckFailures.end()) - return; - - SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc()); - addUsage(NamingCheckFailures, ID, Range); -} - -void IdentifierNamingCheck::onEndOfTranslationUnit() { - for (const auto &Pair : NamingCheckFailures) { - const NamingCheckId &Decl = Pair.first; - const NamingCheckFailure &Failure = Pair.second; - - if (Failure.KindName.empty()) - continue; - - if (Failure.ShouldNotify()) { - auto Diag = - diag(Decl.first, - "invalid case style for %0 '%1'%select{|" // Case 0 is empty on - // purpose, because we - // intent to provide a - // fix - "; cannot be fixed because '%3' would conflict with a keyword|" - "; cannot be fixed because '%3' would conflict with a macro " - "definition}2") - << Failure.KindName << Decl.second - << static_cast<int>(Failure.FixStatus) << Failure.Fixup; - - if (Failure.ShouldFix()) { - for (const auto &Loc : Failure.RawUsageLocs) { - // We assume that the identifier name is made of one token only. This - // is always the case as we ignore usages in macros that could build - // identifier names by combining multiple tokens. - // - // For destructors, we already take care of it by remembering the - // location of the start of the identifier and not the start of the - // tilde. - // - // Other multi-token identifiers, such as operators are not checked at - // all. - Diag << FixItHint::CreateReplacement( - SourceRange(SourceLocation::getFromRawEncoding(Loc)), - Failure.Fixup); - } - } - } - } +RenamerClangTidyCheck::DiagInfo +IdentifierNamingCheck::GetDiagInfo(const NamingCheckId &ID, + const NamingCheckFailure &Failure) const { + return DiagInfo{"invalid case style for %0 '%1'", + [&](DiagnosticBuilder &diag) { + diag << Failure.Info.KindName << ID.second; + }}; } } // namespace readability Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h @@ -0,0 +1,55 @@ +//===--- ReservedIdentifierCheck.h - clang-tidy -----------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RESERVEDIDENTIFIERCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RESERVEDIDENTIFIERCHECK_H + +#include "../utils/RenamerClangTidyCheck.h" +#include "llvm/ADT/Optional.h" +#include <string> +#include <vector> + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Checks for usages of identifiers reserved for use by the C++ implementation. +/// The C++ standard reserves the following names for such use: +/// * identifiers with a double underscore anywhere; +/// * identifiers that begin with an underscore followed by an uppercase letter; +/// * identifiers in the global namespace that begin with an underscore. +/// +/// Violating the naming rules above results in undefined behavior. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-reserved-identifier.html +class ReservedIdentifierCheck final : public RenamerClangTidyCheck { + const bool Invert; + const std::vector<std::string> Whitelist; + +public: + ReservedIdentifierCheck(StringRef Name, ClangTidyContext *Context); + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +protected: + llvm::Optional<FailureInfo> + GetDeclFailureInfo(const NamedDecl *Decl, + const SourceManager &SM) const override; + llvm::Optional<FailureInfo> + GetMacroFailureInfo(const Token &MacroNameTok, + const SourceManager &SM) const override; + DiagInfo GetDiagInfo(const NamingCheckId &ID, + const NamingCheckFailure &Failure) const override; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RESERVEDIDENTIFIERCHECK_H Index: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp @@ -0,0 +1,173 @@ +//===--- ReservedIdentifierCheck.cpp - clang-tidy -------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "ReservedIdentifierCheck.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include <algorithm> + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +ReservedIdentifierCheck::ReservedIdentifierCheck(StringRef Name, + ClangTidyContext *Context) + : RenamerClangTidyCheck(Name, Context), + Invert(Options.get("Invert", false)), + Whitelist(utils::options::parseStringList(Options.get("Whitelist", ""))) { +} + +void ReservedIdentifierCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "Invert", Invert); + Options.store(Opts, "Whitelist", + utils::options::serializeStringList(Whitelist)); +} + +static std::string collapseConsecutive(const StringRef str, const char c) { + std::string result; + std::unique_copy( + str.begin(), str.end(), std::back_inserter(result), + [c](const char a, const char b) { return a == c && b == c; }); + return result; +} + +static bool hasDoubleUnderscore(const StringRef Name) { + return Name.find("__") != StringRef::npos; +} + +static Optional<std::string> getDoubleUnderscoreFixup(StringRef Name) { + if (hasDoubleUnderscore(Name)) { + Name.consume_front("__"); + return collapseConsecutive(Name, '_'); + } + return None; +} + +static bool startsWithUnderscoreCapital(const StringRef Name) { + return Name.size() >= 2 && Name[0] == '_' && std::isupper(Name[1]); +} + +static Optional<std::string> getUnderscoreCapitalFixup(const StringRef Name) { + if (startsWithUnderscoreCapital(Name)) { + return std::string(Name.drop_front(1)); + } + return None; +} + +static bool +startsWithUnderscoreInGlobalNamespace(const StringRef Name, + const bool IsInGlobalNamespace) { + return IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_'; +} + +static Optional<std::string> +getUnderscoreGlobalNamespaceFixup(const StringRef Name, + const bool IsInGlobalNamespace) { + if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace)) { + return std::string(Name.drop_front(1)); + } + return None; +} + +static std::string getNonReservedFixup(std::string Name) { + assert(!Name.empty()); + if (Name[0] == '_' || std::isupper(Name[0])) { + Name.insert(Name.begin(), '_'); + } else { + Name.insert(Name.begin(), 2, '_'); + } + return Name; +} + +static Optional<RenamerClangTidyCheck::FailureInfo> +getFailureInfoImpl(const StringRef Name, const bool IsInGlobalNamespace, + const bool Invert, const ArrayRef<std::string> Whitelist) { + assert(!Name.empty()); + if (std::find(Whitelist.begin(), Whitelist.end(), Name) != Whitelist.end()) + return None; + + using FailureInfo = RenamerClangTidyCheck::FailureInfo; + if (!Invert) { + Optional<FailureInfo> Info; + auto appendFailure = [&](StringRef Kind, std::string &&Fixup) { + if (!Info) { + Info = FailureInfo{Kind, std::move(Fixup)}; + } else { + Info->KindName += Kind; + Info->Fixup = std::move(Fixup); + } + }; + auto inProgressFixup = [&] { + return Info + .map([](const FailureInfo &Info) { return StringRef(Info.Fixup); }) + .getValueOr(Name); + }; + if (auto Fixup = getDoubleUnderscoreFixup(inProgressFixup())) { + appendFailure("du", *std::move(Fixup)); + } + if (auto Fixup = getUnderscoreCapitalFixup(inProgressFixup())) { + appendFailure("uc", *std::move(Fixup)); + } + if (auto Fixup = getUnderscoreGlobalNamespaceFixup(inProgressFixup(), + IsInGlobalNamespace)) { + appendFailure("global-under", *std::move(Fixup)); + } + if (Info) { + return Info; + } + } else { + if (!(hasDoubleUnderscore(Name) || startsWithUnderscoreCapital(Name) || + startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace))) { + return FailureInfo{"non-reserved", getNonReservedFixup(Name)}; + } + } + return None; +} + +Optional<RenamerClangTidyCheck::FailureInfo> +ReservedIdentifierCheck::GetDeclFailureInfo(const NamedDecl *Decl, + const SourceManager &) const { + assert(Decl && Decl->getIdentifier() && !Decl->getName().empty() && + !Decl->isImplicit() && + "Decl must be an explicit identifier with a name."); + return getFailureInfoImpl(Decl->getName(), + isa<TranslationUnitDecl>(Decl->getDeclContext()), + Invert, Whitelist); +} + +Optional<RenamerClangTidyCheck::FailureInfo> +ReservedIdentifierCheck::GetMacroFailureInfo(const Token &MacroNameTok, + const SourceManager &) const { + return getFailureInfoImpl(MacroNameTok.getIdentifierInfo()->getName(), true, + Invert, Whitelist); +} + +RenamerClangTidyCheck::DiagInfo +ReservedIdentifierCheck::GetDiagInfo(const NamingCheckId &ID, + const NamingCheckFailure &Failure) const { + return DiagInfo{ + Failure.Info.KindName == "non-reserved" + ? "declaration uses identifier '%0', which is not a reserved " + "identifier" + : Failure.Info.KindName == "global-under" + ? "declaration uses identifier '%0', which is reserved in " + "the global " + "namespace; this causes undefined behavior" + : "declaration uses reserved identifier '%0', which causes " + "undefined " + "behavior", + [&](DiagnosticBuilder &diag) { diag << ID.second; }}; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -28,6 +28,7 @@ NotNullTerminatedResultCheck.cpp ParentVirtualCallCheck.cpp PosixReturnCheck.cpp + ReservedIdentifierCheck.cpp SizeofContainerCheck.cpp SizeofExpressionCheck.cpp StringConstructorCheck.cpp Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -36,6 +36,7 @@ #include "NotNullTerminatedResultCheck.h" #include "ParentVirtualCallCheck.h" #include "PosixReturnCheck.h" +#include "ReservedIdentifierCheck.h" #include "SizeofContainerCheck.h" #include "SizeofExpressionCheck.h" #include "StringConstructorCheck.h" @@ -119,6 +120,8 @@ "bugprone-parent-virtual-call"); CheckFactories.registerCheck<PosixReturnCheck>( "bugprone-posix-return"); + CheckFactories.registerCheck<ReservedIdentifierCheck>( + "bugprone-reserved-identifier"); CheckFactories.registerCheck<SizeofContainerCheck>( "bugprone-sizeof-container"); CheckFactories.registerCheck<SizeofExpressionCheck>(
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits