LegalizeAdulthood updated this revision to Diff 402415. LegalizeAdulthood added a comment.
Testing on iterated dynamics revealed a number of shortcomings; update the code to address those: - Improve handling of include guards - Handle conditional compilation blocks better - Reject macros that expand to floating point literals - Start a new enum when macros have intervening text CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117522/new/ https://reviews.llvm.org/D117522 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
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,137 @@ +// RUN: %check_clang_tidy %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum + +#if 1 +#include "modernize-macro-to-enum.h" + +// These macros are skipped due to being inside a conditional compilation block. +#define GOO_RED 1 +#define GOO_GREEN 2 +#define GOO_BLUE 3 + +#endif + +#define RED 0xFF0000 +#define GREEN 0x00FF00 +#define BLUE 0x0000FF +// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'RED' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GREEN' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BLUE' defines an integral constant; prefer an enum instead +// 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]]:9: warning: Macro 'CoordModeOrigin' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: Macro 'CoordModePrevious' defines an integral constant; prefer an enum instead +// 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' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-12]]:9: warning: Macro 'BadAccess' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'BadAlloc' defines an integral constant; prefer an enum instead +// 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' defines an integral constant; prefer an enum instead +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: VALID1 = 1 +// CHECK-FIXES-NEXT: }; + +// Regular conditional compilation blocks should leave previous +// macro enums alone. +#if 0 +#include <non-existent.h> +#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); +} Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h @@ -0,0 +1,20 @@ +#pragma once + +#define GG3_RED 0xFF0000 +#define GG3_GREEN 0x00FF00 +#define GG3_BLUE 0x0000FF +// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG3_RED' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG3_GREEN' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG3_BLUE' defines an integral constant; prefer an enum instead +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: GG3_RED = 0xFF0000, +// CHECK-FIXES-NEXT: GG3_GREEN = 0x00FF00, +// CHECK-FIXES-NEXT: GG3_BLUE = 0x0000FF +// CHECK-FIXES-NEXT: }; + +#if 1 +#define RR3_RED 1 +#define RR3_GREEN 2 +#define RR3_BLUE 3 +#endif Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h @@ -0,0 +1,25 @@ +#ifndef MODERNIZE_MACRO_TO_ENUM2_H +#define MODERNIZE_MACRO_TO_ENUM2_H + +#include "modernize-macro-to-enum3.h" + +#define GG2_RED 0xFF0000 +#define GG2_GREEN 0x00FF00 +#define GG2_BLUE 0x0000FF +// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG2_RED' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG2_GREEN' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG2_BLUE' defines an integral constant; prefer an enum instead +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: GG2_RED = 0xFF0000, +// CHECK-FIXES-NEXT: GG2_GREEN = 0x00FF00, +// CHECK-FIXES-NEXT: GG2_BLUE = 0x0000FF +// CHECK-FIXES-NEXT: }; + +#if 1 +#define RR2_RED 1 +#define RR2_GREEN 2 +#define RR2_BLUE 3 +#endif + +#endif Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h @@ -0,0 +1,25 @@ +#if !defined(MODERNIZE_MACRO_TO_ENUM_H) +#define MODERNIZE_MACRO_TO_ENUM_H + +#include "modernize-macro-to-enum2.h" + +#define GG_RED 0xFF0000 +#define GG_GREEN 0x00FF00 +#define GG_BLUE 0x0000FF +// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Replace macro with enum +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG_RED' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG_GREEN' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-4]]:9: warning: Macro 'GG_BLUE' defines an integral constant; prefer an enum instead +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: GG_RED = 0xFF0000, +// CHECK-FIXES-NEXT: GG_GREEN = 0x00FF00, +// CHECK-FIXES-NEXT: GG_BLUE = 0x0000FF +// CHECK-FIXES-NEXT: }; + +#if 1 +#define RR_RED 1 +#define RR_GREEN 2 +#define RR_BLUE 3 +#endif + +#endif 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,56 @@ +.. title:: clang-tidy - modernize-macro-to-enum + +modernize-macro-to-enum +======================= + +Replaces groups of adjacent macros with an unscoped anonymous 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. + (Conditional include guards are exempt from this constraint.) +- 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, although clang-tidy can optionally format all fixes. + +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/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 <modernize-deprecated-headers.html>`_, "Yes" `modernize-deprecated-ios-base-aliases <modernize-deprecated-ios-base-aliases.html>`_, "Yes" `modernize-loop-convert <modernize-loop-convert.html>`_, "Yes" + `modernize-macro-to-enum <modernize-macro-to-enum.html>`_, "Yes" `modernize-make-shared <modernize-make-shared.html>`_, "Yes" `modernize-make-unique <modernize-make-unique.html>`_, "Yes" `modernize-pass-by-value <modernize-pass-by-value.html>`_, "Yes" Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -118,6 +118,11 @@ Reports identifier with unicode right-to-left characters. +- New :doc:`modernize-macro-to-enum + <clang-tidy/checks/modernize-macro-to-enum>` check. + + Replaces groups of adjacent macros with an unscoped anonymous enum. + - New :doc:`readability-container-data-pointer <clang-tidy/checks/readability-container-data-pointer>` check. 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,7 @@ CheckFactories.registerCheck<DeprecatedIosBaseAliasesCheck>( "modernize-deprecated-ios-base-aliases"); CheckFactories.registerCheck<LoopConvertCheck>("modernize-loop-convert"); + CheckFactories.registerCheck<MacroToEnumCheck>("modernize-macro-to-enum"); CheckFactories.registerCheck<MakeSharedCheck>("modernize-make-shared"); CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique"); CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value"); @@ -83,11 +85,11 @@ CheckFactories.registerCheck<UseDefaultMemberInitCheck>( "modernize-use-default-member-init"); CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace"); - CheckFactories.registerCheck<UseEqualsDefaultCheck>("modernize-use-equals-default"); + CheckFactories.registerCheck<UseEqualsDefaultCheck>( + "modernize-use-equals-default"); CheckFactories.registerCheck<UseEqualsDeleteCheck>( "modernize-use-equals-delete"); - CheckFactories.registerCheck<UseNodiscardCheck>( - "modernize-use-nodiscard"); + CheckFactories.registerCheck<UseNodiscardCheck>("modernize-use-nodiscard"); CheckFactories.registerCheck<UseNoexceptCheck>("modernize-use-noexcept"); CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr"); CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override"); Index: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h @@ -0,0 +1,34 @@ +//===--- 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" + +namespace clang { +namespace tidy { +namespace modernize { + +/// Replaces groups of related macros with an unscoped anonymous enum. +/// +/// 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,419 @@ +//===--- 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 "llvm/ADT/STLExtras.h" +#include <algorithm> +#include <cctype> +#include <string> + +#include <iostream> +#include <vector> + +namespace clang { +namespace tidy { +namespace modernize { + +static bool hasOnlyComments(SourceLocation Loc, const LangOptions &Options, + StringRef Text) { + // Use a lexer to look for tokens; if we find something other than a single + // hash, then there were intervening tokens between macro definitions. + std::string Buffer{Text}; + Lexer Lex(Loc, Options, Buffer.c_str(), Buffer.c_str(), + Buffer.c_str() + Buffer.size()); + Token Tok; + bool SeenHash = false; + while (!Lex.LexFromRawLexer(Tok)) { + if (Tok.getKind() == tok::hash && !SeenHash) { + SeenHash = true; + continue; + } + return false; + } + + // Everything in between was whitespace, so now just look for a blank line, + // consisting of two consecutive EOL sequences, either '\n', '\r' or '\r\n'. + enum class WhiteSpace { + Nothing, + CR, + LF, + CRLF, + CRLFCR, + }; + + WhiteSpace State = WhiteSpace::Nothing; + for (char C : Text) { + switch (C) { + case '\r': + if (State == WhiteSpace::CR) + return false; + + State = State == WhiteSpace::CRLF ? WhiteSpace::CRLFCR : WhiteSpace::CR; + break; + + case '\n': + if (State == WhiteSpace::LF || State == WhiteSpace::CRLFCR) + return false; + + State = State == WhiteSpace::CR ? WhiteSpace::CRLF : WhiteSpace::LF; + break; + + default: + State = WhiteSpace::Nothing; + break; + } + } + + return true; +} + +static bool isIntegralConstant(const Token &Token) { + const char *Begin = Token.getLiteralData(); + const char *End = Begin + Token.getLength(); + return std::all_of(Begin, End, [](char C) { return std::isdigit(C); }); +} + +namespace { + +struct EnumMacro { + EnumMacro(Token Name, const MacroDirective *Directive) + : Name(Name), Directive(Directive) {} + + Token Name; + const MacroDirective *Directive; +}; + +using MacroList = std::vector<EnumMacro>; + +enum class IncludeGuard { None, FileChanged, IfGuard, DefineGuard }; + +struct FileState { + FileState() + : ConditionScopes(0), LastLine(0), GuardScanner(IncludeGuard::None) {} + + int ConditionScopes; + unsigned int LastLine; + IncludeGuard GuardScanner; + SourceLocation LastMacroLocation; + std::string Name; +}; + +class MacroToEnumCallbacks : public PPCallbacks { +public: + MacroToEnumCallbacks(MacroToEnumCheck *Check, const LangOptions &LangOptions, + const SourceManager &SM, Preprocessor *PP) + : Check(Check), LangOptions(LangOptions), SM(SM), PP(PP) {} + + void FileChanged(SourceLocation Loc, FileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID) override; + + 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); + CurrentFile->Name = FileName.str(); + } + + // 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. + // Include guards are either + // #if !defined(GUARD) + // or + // #ifndef GUARD + void If(SourceLocation Loc, SourceRange ConditionRange, + ConditionValueKind ConditionValue) override; + void Ifndef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) override; + void Ifdef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) override; + void Endif(SourceLocation Loc, SourceLocation IfLoc) override; + void PragmaDirective(SourceLocation Loc, + PragmaIntroducerKind Introducer) override; + + // After we've seen everything, issue warnings and fix-its. + void EndOfMainFile() override; + +private: + void newEnum() { + if (Enums.empty() || !Enums.back().empty()) + Enums.emplace_back(); + } + bool insideConditional() const { + return (CurrentFile->GuardScanner == IncludeGuard::DefineGuard && + CurrentFile->ConditionScopes > 1) || + (CurrentFile->GuardScanner != IncludeGuard::DefineGuard && + CurrentFile->ConditionScopes > 0); + } + bool isConsecutiveMacro(const MacroDirective *MD) const; + void rememberLastMacroLocation(const MacroDirective *MD) { + CurrentFile->LastLine = SM.getSpellingLineNumber(MD->getLocation()); + CurrentFile->LastMacroLocation = Lexer::getLocForEndOfToken( + MD->getMacroInfo()->getDefinitionEndLoc(), 0, SM, LangOptions); + } + void clearLastMacroLocation() { + CurrentFile->LastLine = 0; + CurrentFile->LastMacroLocation = SourceLocation{}; + } + void clearCurrentEnum(SourceLocation Loc); + void conditionStart(const SourceLocation &Loc); + void warnMacroEnum(const EnumMacro &Macro) const; + void fixEnumMacro(const MacroList &MacroList) const; + + MacroToEnumCheck *Check; + const LangOptions &LangOptions; + const SourceManager &SM; + Preprocessor *PP; + std::vector<MacroList> Enums; + std::vector<FileState> Files; + FileState *CurrentFile = nullptr; +}; + +bool MacroToEnumCallbacks::isConsecutiveMacro(const MacroDirective *MD) const { + if (CurrentFile->LastMacroLocation.isInvalid()) + return false; + + SourceLocation Loc = MD->getLocation(); + if (CurrentFile->LastLine + 1 == SM.getSpellingLineNumber(Loc)) + return true; + + SourceLocation Define = + SM.translateLineCol(SM.getFileID(Loc), SM.getSpellingLineNumber(Loc), 1); + CharSourceRange BetweenMacros{ + SourceRange{CurrentFile->LastMacroLocation, Define}, true}; + CharSourceRange CharRange = + Lexer::makeFileCharRange(BetweenMacros, SM, LangOptions); + StringRef BetweenText = Lexer::getSourceText(CharRange, SM, LangOptions); + return hasOnlyComments(Define, LangOptions, BetweenText); +} + +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) == CurrentFile->LastLine + 1) + Enums.pop_back(); + + clearLastMacroLocation(); +} + +void MacroToEnumCallbacks::conditionStart(const SourceLocation &Loc) { + ++CurrentFile->ConditionScopes; + clearCurrentEnum(Loc); + if (CurrentFile->GuardScanner == IncludeGuard::FileChanged) { + CurrentFile->GuardScanner = IncludeGuard::IfGuard; + } +} + +void MacroToEnumCallbacks::FileChanged(SourceLocation Loc, + FileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID) { + newEnum(); + if (Reason == EnterFile) { + Files.emplace_back(); + if (!SM.isInMainFile(Loc)) + Files.back().GuardScanner = IncludeGuard::FileChanged; + } else if (Reason == ExitFile) { + assert(CurrentFile->ConditionScopes == 0); + Files.pop_back(); + } + CurrentFile = &Files.back(); +} + +void MacroToEnumCallbacks::MacroDefined(const Token &MacroNameTok, + const MacroDirective *MD) { + // Include guards are never candidates for becoming an enum. + if (CurrentFile->GuardScanner == IncludeGuard::IfGuard) { + CurrentFile->GuardScanner = IncludeGuard::DefineGuard; + return; + } + + if (insideConditional()) + return; + + if (SM.getFilename(MD->getLocation()).empty()) + return; + + const MacroInfo *Info = MD->getMacroInfo(); + if (Info->isFunctionLike() || Info->isBuiltinMacro() || + Info->tokens().empty() || Info->tokens().size() > 2) + return; + + // It can be +Lit, -Lit or just Lit. + Token Tok = Info->tokens().front(); + if (Info->tokens().size() == 2) { + if (!Tok.isOneOf(tok::TokenKind::minus, tok::TokenKind::plus)) + return; + Tok = Info->tokens().back(); + } + if (!Tok.isLiteral() || isStringLiteral(Tok.getKind()) || + !isIntegralConstant(Tok)) + return; + + 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(); + }; + + auto It = llvm::find_if(Enums, [MatchesToken](const MacroList &MacroList) { + return llvm::any_of(MacroList, MatchesToken); + }); + if (It != Enums.end()) + Enums.erase(It); + + clearLastMacroLocation(); + CurrentFile->GuardScanner = IncludeGuard::None; +} + +void MacroToEnumCallbacks::If(SourceLocation Loc, SourceRange ConditionRange, + ConditionValueKind ConditionValue) { + conditionStart(Loc); +} + +void MacroToEnumCallbacks::Ifndef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) { + conditionStart(Loc); +} + +void MacroToEnumCallbacks::Ifdef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) { + conditionStart(Loc); +} + +void MacroToEnumCallbacks::Endif(SourceLocation Loc, SourceLocation IfLoc) { + // The if directive for the include guard isn't counted in the + // ConditionScopes. + if (CurrentFile->ConditionScopes == 0 && + CurrentFile->GuardScanner == IncludeGuard::DefineGuard) + return; + + // We don't need to clear the current enum because the start of the + // conditional block already took care of that. + assert(CurrentFile->ConditionScopes > 0); + --CurrentFile->ConditionScopes; +} + +void MacroToEnumCallbacks::PragmaDirective(SourceLocation Loc, + PragmaIntroducerKind Introducer) { + if (CurrentFile->GuardScanner != IncludeGuard::FileChanged) + return; + + bool Invalid = false; + const char *Text = SM.getCharacterData( + Lexer::getLocForEndOfToken(Loc, 0, SM, LangOptions), &Invalid); + if (Invalid) + return; + + while (*Text && std::isspace(*Text)) + ++Text; + + if (std::strncmp("pragma", Text, 6) != 0) + return; + + Text += 6; + while (*Text && std::isspace(*Text)) + ++Text; + + if (std::strncmp("once", Text, 4) == 0) + CurrentFile->GuardScanner = IncludeGuard::IfGuard; +} + +void MacroToEnumCallbacks::EndOfMainFile() { + for (const MacroList &MacroList : Enums) { + if (MacroList.empty()) + continue; + + for (const EnumMacro &Macro : MacroList) + warnMacroEnum(Macro); + + fixEnumMacro(MacroList); + } +} + +void MacroToEnumCallbacks::warnMacroEnum(const EnumMacro &Macro) const { + Check->diag(Macro.Directive->getLocation(), + "Macro '%0' defines an integral constant; prefer an enum instead") + << 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<MacroToEnumCallbacks>(this, getLangOpts(), SM, PP)); +} + +} // namespace modernize +} // namespace tidy +} // namespace clang 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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits