Index: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -11,6 +11,7 @@ DeprecatedIosBaseAliasesCheck.cpp LoopConvertCheck.cpp LoopConvertUtils.cpp + MacroToEnumCheck.cpp MakeSharedCheck.cpp MakeSmartPtrCheck.cpp MakeUniqueCheck.cpp Index: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h @@ -0,0 +1,35 @@ +//===--- MacroToEnumCheck.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_MODERNIZE_MACROTOENUMCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MACROTOENUMCHECK_H + +#include "../ClangTidyCheck.h" +#include + +namespace clang { +namespace tidy { +namespace modernize { + +/// FIXME: Write a short description. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-macro-to-enum.html +class MacroToEnumCheck : public ClangTidyCheck { +public: + MacroToEnumCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; +}; + +} // namespace modernize +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MACROTOENUMCHECK_H Index: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp @@ -0,0 +1,289 @@ +//===--- MacroToEnumCheck.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 "MacroToEnumCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" + +#include +#include +#include +#include +#include +#include + +namespace clang { +namespace tidy { +namespace modernize { + +namespace { + +struct EnumMacro { + EnumMacro(Token Name, const MacroDirective *Directive) + : Name(Name), Directive(Directive) {} + + Token Name; + const MacroDirective *Directive; +}; + +using MacroList = std::vector; + +class MacroToEnumCallbacks : public PPCallbacks { +public: + MacroToEnumCallbacks(MacroToEnumCheck *Check, const LangOptions &LangOptions, + const SourceManager &SM) + : Check(Check), LangOptions(LangOptions), SM(SM) {} + + void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, + StringRef FileName, bool IsAngled, + CharSourceRange FilenameRange, const FileEntry *File, + StringRef SearchPath, StringRef RelativePath, + const Module *Imported, + SrcMgr::CharacteristicKind FileType) override { + clearCurrentEnum(HashLoc); + } + + // Keep track of macro definitions that look like enums. + void MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) override; + + // Undefining an enum-like macro results in the enum set being dropped. + void MacroUndefined(const Token &MacroNameTok, const MacroDefinition &MD, + const MacroDirective *Undef) override; + + // Conditional compilation clears any adjacent enum-like macros. + void If(SourceLocation Loc, SourceRange ConditionRange, + ConditionValueKind ConditionValue) override { + conditionStart(Loc); + } + void Ifdef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) override { + conditionStart(Loc); + } + void Ifndef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) override { + conditionStart(Loc); + } + void Endif(SourceLocation Loc, SourceLocation IfLoc) override { + conditionEnd(); + } + + // After we've seen everything, issue warnings and fix-its. + void EndOfMainFile() override; + +private: + void newEnum(); + bool isConsecutiveMacro(const MacroDirective *MD) const; + void rememberLastMacroLocation(const MacroDirective *MD); + void clearLastMacroLocation(); + void clearCurrentEnum(SourceLocation Loc); + void conditionStart(const SourceLocation &Loc) { + ++ConditionScope; + clearCurrentEnum(Loc); + } + // We don't need to clear the current enum because the start of the + // conditional block already took care of that. + void conditionEnd() { --ConditionScope; } + void warnMacroEnum(const EnumMacro &Macro) const; + void fixEnumMacro(const MacroList &MacroList) const; + + MacroToEnumCheck *Check; + const LangOptions &LangOptions; + const SourceManager &SM; + std::vector Enums; + StringRef LastFile; + unsigned int LastLine = 0; + SourceLocation LastMacroLocation; + int ConditionScope = 0; +}; + +bool hasBlankLines(const std::string &Text) { + enum class WhiteSpaceState { + Nothing, + CR, + LF, + CRLF, + CRLFCR, + }; + WhiteSpaceState State = WhiteSpaceState::Nothing; + for (char c : Text) { + if (c == '\r') { + if (State == WhiteSpaceState::CR) + return true; + + State = State == WhiteSpaceState::CRLF ? WhiteSpaceState::CRLFCR + : WhiteSpaceState::CR; + } else if (c == '\n') { + if (State == WhiteSpaceState::LF || State == WhiteSpaceState::CRLFCR) + return true; + + State = State == WhiteSpaceState::CR ? WhiteSpaceState::CRLF + : WhiteSpaceState::LF; + } else + State = WhiteSpaceState::Nothing; + } + return false; +} + +bool MacroToEnumCallbacks::isConsecutiveMacro(const MacroDirective *MD) const { + if (LastMacroLocation.isInvalid()) + return false; + + SourceLocation Loc = MD->getLocation(); + if (LastLine + 1 == SM.getSpellingLineNumber(Loc)) + return true; + + SourceLocation Define = + SM.translateLineCol(SM.getFileID(Loc), SM.getSpellingLineNumber(Loc), 1); + CharSourceRange BetweenMacros{SourceRange{LastMacroLocation, Define}, true}; + CharSourceRange CharRange = + Lexer::makeFileCharRange(BetweenMacros, SM, LangOptions); + std::string BetweenText = + Lexer::getSourceText(CharRange, SM, LangOptions).str(); + return !hasBlankLines(BetweenText); +} + +void MacroToEnumCallbacks::rememberLastMacroLocation(const MacroDirective *MD) { + LastLine = SM.getSpellingLineNumber(MD->getLocation()); + LastMacroLocation = Lexer::getLocForEndOfToken( + MD->getMacroInfo()->getDefinitionEndLoc(), 0, SM, LangOptions); +} + +void MacroToEnumCallbacks::clearLastMacroLocation() { + LastLine = 0; + LastMacroLocation = SourceLocation{}; +} + +void MacroToEnumCallbacks::clearCurrentEnum(SourceLocation Loc) { + // Only drop the most recent Enum set if the directive immediately follows. + if (!Enums.empty() && !Enums.back().empty() && + SM.getSpellingLineNumber(Loc) == LastLine + 1) + Enums.pop_back(); + + clearLastMacroLocation(); +} + +void MacroToEnumCallbacks::newEnum() { + if (Enums.empty() || !Enums.back().empty()) + Enums.emplace_back(); +} + +void MacroToEnumCallbacks::MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) { + StringRef CurrentFile = SM.getFilename(MD->getLocation()); + if (CurrentFile.empty() || MD->getMacroInfo()->isFunctionLike() || + MD->getMacroInfo()->isUsedForHeaderGuard() || + MD->getMacroInfo()->isBuiltinMacro() || ConditionScope > 0) + return; + + if (std::any_of(MD->getMacroInfo()->tokens_begin(), + MD->getMacroInfo()->tokens_end(), [](Token Token) { + return !Token.isLiteral() || + isStringLiteral(Token.getKind()); + })) + return; + + if (LastFile != CurrentFile) { + LastFile = CurrentFile; + newEnum(); + } else if (!isConsecutiveMacro(MD)) { + newEnum(); + } + Enums.back().emplace_back(MacroNameTok, MD); + rememberLastMacroLocation(MD); +} + +// Any macro that is undefined removes all adjacent macros from consideration as +// an enum and starts a new enum scan. +void MacroToEnumCallbacks::MacroUndefined(const Token &MacroNameTok, + const MacroDefinition &MD, + const MacroDirective *Undef) { + auto MatchesToken = [&MacroNameTok](const EnumMacro &Macro) { + return Macro.Name.getIdentifierInfo()->getName() == + MacroNameTok.getIdentifierInfo()->getName(); + }; + + Enums.erase(std::remove_if(Enums.begin(), Enums.end(), + [MatchesToken](const MacroList &MacroList) { + return std::find_if( + MacroList.begin(), MacroList.end(), + MatchesToken) != MacroList.end(); + }), + Enums.end()); + + clearLastMacroLocation(); +} + +void MacroToEnumCallbacks::EndOfMainFile() { + for (const MacroList &MacroList : Enums) { + for (const EnumMacro &Macro : MacroList) + warnMacroEnum(Macro); + + fixEnumMacro(MacroList); + } +} + +void MacroToEnumCallbacks::warnMacroEnum(const EnumMacro &Macro) const { + Check->diag(Macro.Directive->getLocation(), + "Macro '%0' is disguised as an enum") + << Macro.Name.getIdentifierInfo()->getName(); +} + +void MacroToEnumCallbacks::fixEnumMacro(const MacroList &MacroList) const { + SourceLocation Begin = + MacroList.front().Directive->getMacroInfo()->getDefinitionLoc(); + Begin = SM.translateLineCol(SM.getFileID(Begin), + SM.getSpellingLineNumber(Begin), 1); + DiagnosticBuilder Diagnostic = + Check->diag(Begin, "Replace macro with enum") + << FixItHint::CreateInsertion(Begin, "enum {\n"); + + for (size_t I = 0u; I < MacroList.size(); ++I) { + const EnumMacro &Macro = MacroList[I]; + SourceLocation DefineEnd = + Macro.Directive->getMacroInfo()->getDefinitionLoc(); + SourceLocation DefineBegin = SM.translateLineCol( + SM.getFileID(DefineEnd), SM.getSpellingLineNumber(DefineEnd), 1); + CharSourceRange DefineRange; + DefineRange.setBegin(DefineBegin); + DefineRange.setEnd(DefineEnd); + Diagnostic << FixItHint::CreateRemoval(DefineRange); + + SourceLocation NameEnd = Lexer::getLocForEndOfToken( + Macro.Directive->getMacroInfo()->getDefinitionLoc(), 0, SM, + LangOptions); + Diagnostic << FixItHint::CreateInsertion(NameEnd, " ="); + + SourceLocation ValueEnd = Lexer::getLocForEndOfToken( + Macro.Directive->getMacroInfo()->getDefinitionEndLoc(), 0, SM, + LangOptions); + if (I < MacroList.size() - 1) + Diagnostic << FixItHint::CreateInsertion(ValueEnd, ","); + } + + SourceLocation End = Lexer::getLocForEndOfToken( + MacroList.back().Directive->getMacroInfo()->getDefinitionEndLoc(), 0, SM, + LangOptions); + End = SM.translateLineCol(SM.getFileID(End), + SM.getSpellingLineNumber(End) + 1, 1); + Diagnostic << FixItHint::CreateInsertion(End, "};\n"); +} + +} // namespace + +void MacroToEnumCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + PP->addPPCallbacks( + std::make_unique(this, getLangOpts(), SM)); +} + +} // namespace modernize +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -15,6 +15,7 @@ #include "DeprecatedHeadersCheck.h" #include "DeprecatedIosBaseAliasesCheck.h" #include "LoopConvertCheck.h" +#include "MacroToEnumCheck.h" #include "MakeSharedCheck.h" #include "MakeUniqueCheck.h" #include "PassByValueCheck.h" @@ -59,6 +60,8 @@ CheckFactories.registerCheck( "modernize-deprecated-ios-base-aliases"); CheckFactories.registerCheck("modernize-loop-convert"); + CheckFactories.registerCheck( + "modernize-macro-to-enum"); CheckFactories.registerCheck("modernize-make-shared"); CheckFactories.registerCheck("modernize-make-unique"); CheckFactories.registerCheck("modernize-pass-by-value"); Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -115,6 +115,11 @@ Reports identifier with unicode right-to-left characters. +- New :doc:`modernize-macro-to-enum + ` check. + + Replaces macros with anonymous enums. + - New :doc:`readability-container-data-pointer ` check. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -233,6 +233,7 @@ `modernize-deprecated-headers `_, "Yes" `modernize-deprecated-ios-base-aliases `_, "Yes" `modernize-loop-convert `_, "Yes" + `modernize-macro-to-enum `_, "Yes" `modernize-make-shared `_, "Yes" `modernize-make-unique `_, "Yes" `modernize-pass-by-value `_, "Yes" Index: clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst @@ -0,0 +1,55 @@ +.. title:: clang-tidy - modernize-macro-to-enum + +modernize-macro-to-enum +======================= + +This check performs basic analysis of macros and replaces them +with an anonymous unscoped enum. Using an unscoped anonymous enum +ensures that everywhere the macro token was used previously, the +enumerator name may be safely used. + +Potential macros for replacement must meet the following constraints: +- Macros must expand only to integral literal tokens. +- Macros must be defined on sequential source file lines, or with + only comment lines in between macro definitions. +- Macros must all be defined in the same source file. +- Macros must not be defined within a conditional compilation block. +- Macros must not be defined adjacent to other preprocessor directives. + +Each cluster of macros meeting the above constraints is presumed to +be a set of values suitable for replacement by an anonymous enum. +From there, a developer can give the anonymous enum a name and +continue refactoring to a scoped enum if desired. Comments on the +same line as a macro definition or between subsequent macro definitions +are preserved in the output. No formatting is assumed in the provided +replacements. + +Examples: + +.. code-block:: c++ + + #define RED 0xFF0000 + #define GREEN 0x00FF00 + #define BLUE 0x0000FF + + #define TM_ONE 1 // Use tailored method one. + #define TM_TWO 2 // Use tailored method two. Method two + // is preferable to method one. + #define TM_THREE 3 // Use tailored method three. + +becomes + +.. code-block:: c++ + + enum { + RED = 0xFF0000, + GREEN = 0x00FF00, + BLUE = 0x0000FF + }; + + enum { + TM_ONE = 1, // Use tailored method one. + TM_TWO = 2, // Use tailored method two. Method two + // is preferable to method one. + TM_THREE = 3 // Use tailored method three. + }; Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp @@ -0,0 +1,142 @@ +// RUN: %check_clang_tidy %s modernize-macro-to-enum %t + +#define RED 0xFF0000 +#define GREEN 0x00FF00 +#define BLUE 0x0000FF +// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum +// CHECK-MESSAGES: :[[@LINE-4]]:1: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-5]]:12: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-6]]:21: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-6]]:1: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-7]]:14: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-8]]:23: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-8]]:1: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-9]]:13: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-9]]:1: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-13]]:9: warning: Macro 'RED' is disguised as an enum +// CHECK-MESSAGES: :[[@LINE-13]]:9: warning: Macro 'GREEN' is disguised as an enum +// CHECK-MESSAGES: :[[@LINE-13]]:9: warning: Macro 'BLUE' is disguised as an enum +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: RED = 0xFF0000, +// CHECK-FIXES-NEXT: GREEN = 0x00FF00, +// CHECK-FIXES-NEXT: BLUE = 0x0000FF +// CHECK-FIXES-NEXT: }; + +// Verify that comments are preserved. +#define CoordModeOrigin 0 /* relative to the origin */ +#define CoordModePrevious 1 /* relative to previous point */ +// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Replace macro with enum +// CHECK-MESSAGES: :[[@LINE-3]]:1: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-4]]:24: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-5]]:34: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-5]]:1: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-6]]:26: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-6]]:1: note: FIX-IT applied suggested code changes +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: Macro 'CoordModeOrigin' is disguised as an enum +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: Macro 'CoordModePrevious' is disguised as an enum +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: CoordModeOrigin = 0, /* relative to the origin */ +// CHECK-FIXES-NEXT: CoordModePrevious = 1 /* relative to previous point */ +// CHECK-FIXES-NEXT: }; + +// Verify that multiline comments are preserved. +#define BadDrawable 9 /* parameter not a Pixmap or Window */ +#define BadAccess 10 /* depending on context: + - key/button already grabbed + - attempt to free an illegal + cmap entry + - attempt to store into a read-only + color map entry. */ + // - attempt to modify the access control + // list from other than the local host. + // +#define BadAlloc 11 /* insufficient resources */ +// CHECK-MESSAGES: :[[@LINE-11]]:1: warning: Replace macro with enum +// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadDrawable' is disguised as an enum +// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' is disguised as an enum +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' is disguised as an enum +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: BadDrawable = 9, /* parameter not a Pixmap or Window */ +// CHECK-FIXES-NEXT: BadAccess = 10, /* depending on context: +// CHECK-FIXES-NEXT: - key/button already grabbed +// CHECK-FIXES-NEXT: - attempt to free an illegal +// CHECK-FIXES-NEXT: cmap entry +// CHECK-FIXES-NEXT: - attempt to store into a read-only +// CHECK-FIXES-NEXT: color map entry. */ +// CHECK-FIXES-NEXT: // - attempt to modify the access control +// CHECK-FIXES-NEXT: // list from other than the local host. +// CHECK-FIXES-NEXT: // +// CHECK-FIXES-NEXT: BadAlloc = 11 /* insufficient resources */ +// CHECK-FIXES-NEXT: }; + +// Undefining a macro invalidates adjacent macros +// from being considered as an enum. +#define REMOVED1 1 +#define REMOVED2 2 +#define REMOVED3 3 +#undef REMOVED2 +#define VALID1 1 +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Replace macro with enum +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: Macro 'VALID1' is disguised as an enum +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: VALID1 = 1 +// CHECK-FIXES-NEXT: }; + +// Regular conditional compilation blocks should leave previous +// macro enums alone. +#if 0 +#include +#endif + +// Conditional compilation blocks invalidate adjacent macros +// from being considered as an enum. Conditionally compiled +// blocks could contain macros that should rightly be included +// in the enum, but we can't explore multiple branches of a +// conditionally compiled section in clang-tidy, only the active +// branch based on compilation options. +#define CONDITION1 1 +#define CONDITION2 2 +#if 0 +#define CONDITION3 3 +#else +#define CONDITION3 -3 +#endif + +#define IFDEF1 1 +#define IFDEF2 2 +#ifdef FROB +#define IFDEF3 3 +#endif + +#define IFNDEF1 1 +#define IFNDEF2 2 +#ifndef GOINK +#define IFNDEF3 3 +#endif + +// These macros do not expand to integral constants. +#define HELLO "Hello, " +#define WORLD "World" + +#define DO_RED draw(RED) +#define DO_GREEN draw(GREEN) +#define DO_BLUE draw(BLUE) + +#define FN_RED(x) draw(RED | x) +#define FN_GREEN(x) draw(GREEN | x) +#define FN_BLUE(x) draw(BLUE | x) + +extern void draw(unsigned int Color); + +void f() +{ + draw(RED); + draw(GREEN); + draw(BLUE); + DO_RED; + DO_GREEN; + DO_BLUE; + FN_RED(0); + FN_GREEN(0); + FN_BLUE(0); +}