diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 147c2741d59e..6bf20552e018 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -11,6 +11,7 @@ add_clang_library(clangTidyModernizeModule DeprecatedIosBaseAliasesCheck.cpp LoopConvertCheck.cpp LoopConvertUtils.cpp + MacroToEnumCheck.cpp MakeSharedCheck.cpp MakeSmartPtrCheck.cpp MakeUniqueCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp new file mode 100644 index 000000000000..e6115af77bd5 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp @@ -0,0 +1,489 @@ +//===--- 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 +#include +#include +#include + +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 two blank lines, + // 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; +} + +// Validate that this literal token is a valid integer literal. A literal token +// could be a floating-point token, which isn't acceptable as a value for an +// enumeration. A floating-point token must either have a decimal point or an +// exponent ('E' or 'P'). +static bool isIntegralConstant(const Token &Token) { + const char *Begin = Token.getLiteralData(); + const char *End = Begin + Token.getLength(); + + // not a hexadecimal floating-point literal + if (Token.getLength() > 2 && Begin[0] == '0' && std::toupper(Begin[1]) == 'X') + return std::none_of(Begin + 2, End, [](char C) { + return C == '.' || std::toupper(C) == 'P'; + }); + + // not a decimal floating-point literal + return std::none_of( + Begin, End, [](char C) { return C == '.' || std::toupper(C) == 'E'; }); +} + +namespace { + +struct EnumMacro { + EnumMacro(Token Name, const MacroDirective *Directive) + : Name(Name), Directive(Directive) {} + + Token Name; + const MacroDirective *Directive; +}; + +using MacroList = SmallVector; + +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; +}; + +class MacroToEnumCallbacks : public PPCallbacks { +public: + MacroToEnumCallbacks(MacroToEnumCheck *Check, const LangOptions &LangOptions, + const SourceManager &SM) + : Check(Check), LangOpts(LangOptions), SM(SM) {} + + 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); + } + + // 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. + // Macros used in conditional expressions clear any adjacent enum-like + // macros. + // Include guards are either + // #if !defined(GUARD) + // or + // #ifndef GUARD + void If(SourceLocation Loc, SourceRange ConditionRange, + ConditionValueKind ConditionValue) override { + conditionStart(Loc); + checkCondition(ConditionRange); + } + void Ifndef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) override { + conditionStart(Loc); + checkName(MacroNameTok); + } + void Ifdef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) override { + conditionStart(Loc); + checkName(MacroNameTok); + } + void Elif(SourceLocation Loc, SourceRange ConditionRange, + ConditionValueKind ConditionValue, SourceLocation IfLoc) override { + checkCondition(ConditionRange); + } + void Elifdef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) override { + checkName(MacroNameTok); + } + void Elifdef(SourceLocation Loc, SourceRange ConditionRange, + SourceLocation IfLoc) override { + PPCallbacks::Elifdef(Loc, ConditionRange, IfLoc); + } + void Elifndef(SourceLocation Loc, const Token &MacroNameTok, + const MacroDefinition &MD) override { + checkName(MacroNameTok); + } + void Elifndef(SourceLocation Loc, SourceRange ConditionRange, + SourceLocation IfLoc) override { + PPCallbacks::Elifndef(Loc, ConditionRange, IfLoc); + } + 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, LangOpts); + } + void clearLastMacroLocation() { + CurrentFile->LastLine = 0; + CurrentFile->LastMacroLocation = SourceLocation{}; + } + void clearCurrentEnum(SourceLocation Loc); + void conditionStart(const SourceLocation &Loc); + void checkCondition(SourceRange ConditionRange); + void checkName(const Token &MacroNameTok); + void warnMacroEnum(const EnumMacro &Macro) const; + void fixEnumMacro(const MacroList &MacroList) const; + + MacroToEnumCheck *Check; + const LangOptions &LangOpts; + const SourceManager &SM; + SmallVector Enums; + SmallVector 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, LangOpts); + StringRef BetweenText = Lexer::getSourceText(CharRange, SM, LangOpts); + return hasOnlyComments(Define, LangOpts, 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::checkCondition(SourceRange Range) { + CharSourceRange CharRange = Lexer::makeFileCharRange( + CharSourceRange::getTokenRange(Range), SM, LangOpts); + std::string Text = Lexer::getSourceText(CharRange, SM, LangOpts).str(); + Lexer Lex(CharRange.getBegin(), LangOpts, Text.data(), Text.data(), + Text.data() + Text.size()); + Token Tok; + bool End = false; + while (!End) { + End = Lex.LexFromRawLexer(Tok); + if (Tok.is(tok::raw_identifier) && + Tok.getRawIdentifier().str() != "defined") + checkName(Tok); + } +} + +void MacroToEnumCallbacks::checkName(const Token &MacroNameTok) { + std::string Id; + if (MacroNameTok.is(tok::raw_identifier)) + Id = MacroNameTok.getRawIdentifier().str(); + else if (MacroNameTok.is(tok::identifier)) + Id = MacroNameTok.getIdentifierInfo()->getName().str(); + else { + assert(false && "Expected either an identifier or raw identifier token"); + return; + } + + llvm::erase_if(Enums, [&Id](const MacroList &MacroList) { + return llvm::any_of(MacroList, [&Id](const EnumMacro &Macro) { + return Macro.Name.getIdentifierInfo()->getName().str() == Id; + }); + }); +} + +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, + tok::TokenKind::tilde)) + 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::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; +} + +namespace { + +template +bool textEquals(const char (&Needle)[N], const char *HayStack) { + return StringRef{HayStack, N - 1} == Needle; +} + +template size_t len(const char (&)[N]) { return N - 1; } + +} // namespace + +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, LangOpts), &Invalid); + if (Invalid) + return; + + while (*Text && std::isspace(*Text)) + ++Text; + + if (textEquals("pragma", Text)) + return; + + Text += len("pragma"); + while (*Text && std::isspace(*Text)) + ++Text; + + if (textEquals("once", Text)) + 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(); +} + +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, LangOpts); + Diagnostic << FixItHint::CreateInsertion(NameEnd, " ="); + + SourceLocation ValueEnd = Lexer::getLocForEndOfToken( + Macro.Directive->getMacroInfo()->getDefinitionEndLoc(), 0, SM, + LangOpts); + if (I < MacroList.size() - 1) + Diagnostic << FixItHint::CreateInsertion(ValueEnd, ","); + } + + SourceLocation End = Lexer::getLocForEndOfToken( + MacroList.back().Directive->getMacroInfo()->getDefinitionEndLoc(), 0, SM, + LangOpts); + 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 diff --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.h new file mode 100644 index 000000000000..667ff49d671a --- /dev/null +++ b/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 diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index d9ccd2cd0ad7..08076354dbe4 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/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 @@ public: 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"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 729bb3bbd736..e5140ed45cbb 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -106,9 +106,18 @@ New checks Finds initializations of C++ shared pointers to non-array type that are initialized with an array. +- New :doc:`modernize-macro-to-enum + ` check. + + Replaces groups of adjacent macros with an unscoped anonymous enum. + New check aliases ^^^^^^^^^^^^^^^^^ +- New alias :doc:`cppcoreguidelines-macro-to-enum + ` to :doc:`modernize-macro-to-enum + ` was added. + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-to-enum.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-to-enum.rst new file mode 100644 index 000000000000..c9601c652ad9 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-macro-to-enum.rst @@ -0,0 +1,9 @@ +.. title:: clang-tidy - cppcoreguidelines-macro-to-enum +.. meta:: + :http-equiv=refresh: 5;URL=modernize-macro-to-enum.html + +cppcoreguidelines-macro-to-enum +=============================== + +The cppcoreguidelines-macro-to-enum check is an alias, please see +:doc:`modernize-macro-to-enum ` for more information. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index ebdcd7ae1041..e9367216ac51 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -234,6 +234,7 @@ Clang-Tidy Checks `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" @@ -426,6 +427,7 @@ Clang-Tidy Checks `cppcoreguidelines-avoid-magic-numbers `_, `readability-magic-numbers `_, `cppcoreguidelines-c-copy-assignment-signature `_, `misc-unconventional-assign-operator `_, `cppcoreguidelines-explicit-virtual-functions `_, `modernize-use-override `_, "Yes" + `cppcoreguidelines-macro-to-enum `_, `modernize-macro-to-enum `_, "Yes" `cppcoreguidelines-non-private-member-variables-in-classes `_, `misc-non-private-member-variables-in-classes `_, `fuchsia-header-anon-namespaces `_, `google-build-namespaces `_, `google-readability-braces-around-statements `_, `readability-braces-around-statements `_, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst new file mode 100644 index 000000000000..6be406544ba8 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst @@ -0,0 +1,66 @@ +.. 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. + +This check can be used to enforce the C++ core guideline `Enum.1: +Prefer enumerations over macros +`_, +within the constraints outlined below. + +Potential macros for replacement must meet the following constraints: + +- Macros must expand only to integral literal tokens. The unary operators + plus, minus and tilde are recognized to allow for positive, negative + and bitwise negated integers. More complicated integral constant + expressions are not currently recognized by this check. +- 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. +- Macros must not be used in any conditional preprocessing directive. +- Macros must not be undefined. + +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. + }; diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum.h new file mode 100644 index 000000000000..f747be4761bb --- /dev/null +++ b/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 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum2.h new file mode 100644 index 000000000000..1616302a5b69 --- /dev/null +++ b/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 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-macro-to-enum/modernize-macro-to-enum3.h new file mode 100644 index 000000000000..a9f1fb7e0fa4 --- /dev/null +++ b/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 diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp new file mode 100644 index 000000000000..5a5514fc074c --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp @@ -0,0 +1,239 @@ +// RUN: %check_clang_tidy -std=c++14-or-later %s modernize-macro-to-enum %t -- -- -I%S/Inputs/modernize-macro-to-enum +// C++14 or later required for binary literals. + +#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 [modernize-macro-to-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: }; + +#define UNDEF1 1 +#define UNDEF2 2 +#define UNDEF3 3 + +// Undefining a macro later invalidates the set of possible adjacent macros +// from being considered as an enum. +#undef UNDEF2 + +// Integral constants can have an optional sign +#define SIGNED1 +1 +#define SIGNED2 -1 +// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: replace macro with enum +// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: macro 'SIGNED1' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: macro 'SIGNED2' defines an integral constant; prefer an enum instead +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: SIGNED1 = +1, +// CHECK-FIXES-NEXT: SIGNED2 = -1 +// CHECK-FIXES-NEXT: }; + +// Integral constants with bitwise negated values +#define UNOP1 ~0U +#define UNOP2 ~1U +// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: replace macro with enum +// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: macro 'UNOP1' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: macro 'UNOP2' defines an integral constant; prefer an enum instead +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: UNOP1 = ~0U, +// CHECK-FIXES-NEXT: UNOP2 = ~1U +// CHECK-FIXES-NEXT: }; + +// Integral constants in other bases and with suffixes are OK +#define BASE1 0777 // octal +#define BASE2 0xDEAD // hexadecimal +#define BASE3 0b0011 // binary +#define SUFFIX1 +1U +#define SUFFIX2 -1L +#define SUFFIX3 +1UL +#define SUFFIX4 -1LL +#define SUFFIX5 +1ULL +// CHECK-MESSAGES: :[[@LINE-8]]:1: warning: replace macro with enum +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: macro 'BASE1' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: macro 'BASE2' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: macro 'BASE3' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: macro 'SUFFIX1' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: macro 'SUFFIX2' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: macro 'SUFFIX3' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: macro 'SUFFIX4' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-9]]:9: warning: macro 'SUFFIX5' defines an integral constant; prefer an enum instead +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: BASE1 = 0777, // octal +// CHECK-FIXES-NEXT: BASE2 = 0xDEAD, // hexadecimal +// CHECK-FIXES-NEXT: BASE3 = 0b0011, // binary +// CHECK-FIXES-NEXT: SUFFIX1 = +1U, +// CHECK-FIXES-NEXT: SUFFIX2 = -1L, +// CHECK-FIXES-NEXT: SUFFIX3 = +1UL, +// CHECK-FIXES-NEXT: SUFFIX4 = -1LL, +// CHECK-FIXES-NEXT: SUFFIX5 = +1ULL +// CHECK-FIXES-NEXT: }; + +// Macros appearing in conditional expressions can't be replaced +// by enums. +#define USE_FOO 1 +#define USE_BAR 0 +#define USE_IF 1 +#define USE_ELIF 1 +#define USE_IFDEF 1 +#define USE_IFNDEF 1 + +#if defined(USE_FOO) && USE_FOO +extern void foo(); +#else +inline void foo() {} +#endif + +#if USE_BAR +extern void bar(); +#else +inline void bar() {} +#endif + +#if USE_IF +inline void used_if() {} +#endif + +#if 0 +#elif USE_ELIF +inline void used_elif() {} +#endif + +#ifdef USE_IFDEF +inline void used_ifdef() {} +#endif + +#ifndef USE_IFNDEF +#else +inline void used_ifndef() {} +#endif + +// 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 EPS1 1.0F +#define EPS2 1e5 +#define EPS3 1. + +#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); +}