LegalizeAdulthood created this revision.
LegalizeAdulthood added a reviewer: aaron.ballman.
LegalizeAdulthood added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
LegalizeAdulthood requested review of this revision.
When a "keyword" token like __restrict was present in a macro condition,
modernize-macro-to-enum would assert in non-release builds. However,
even for a "keyword" token, calling getIdentifierInfo()->getName() would
retrieve the text of the token, which is what we want. Our intention is
to scan names that appear in conditional expressions in potential enum
clusters and invalidate those clusters if they contain the name.
Also, guard against "raw identifiers" appearing as potential enums.
This shouldn't happen, but it doesn't hurt to generalize the code.
Fixes #54775
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D123349
Files:
clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
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
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -208,6 +208,10 @@
#define IFNDEF3 3
#endif
+// Sometimes an argument to ifdef can be classified as a keyword token.
+#ifdef __restrict
+#endif
+
// These macros do not expand to integral constants.
#define HELLO "Hello, "
#define WORLD "World"
Index: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
@@ -278,20 +278,17 @@
}
}
+inline StringRef getTokenName(const Token &Tok) {
+ return Tok.is(tok::raw_identifier) ? Tok.getRawIdentifier()
+ : Tok.getIdentifierInfo()->getName();
+}
+
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;
- }
+ std::string Id = getTokenName(MacroNameTok).str();
llvm::erase_if(Enums, [&Id](const MacroList &MacroList) {
return llvm::any_of(MacroList, [&Id](const EnumMacro &Macro) {
- return Macro.Name.getIdentifierInfo()->getName().str() == Id;
+ return getTokenName(Macro.Name).str() == Id;
});
});
}
@@ -355,8 +352,7 @@
const MacroDefinition &MD,
const MacroDirective *Undef) {
auto MatchesToken = [&MacroNameTok](const EnumMacro &Macro) {
- return Macro.Name.getIdentifierInfo()->getName() ==
- MacroNameTok.getIdentifierInfo()->getName();
+ return getTokenName(Macro.Name) == getTokenName(MacroNameTok);
};
auto It = llvm::find_if(Enums, [MatchesToken](const MacroList &MacroList) {
@@ -432,8 +428,8 @@
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();
+ "macro '%0' defines an integral constant; prefer an enum
instead")
+ << getTokenName(Macro.Name).str();
}
void MacroToEnumCallbacks::fixEnumMacro(const MacroList &MacroList) const {
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
@@ -208,6 +208,10 @@
#define IFNDEF3 3
#endif
+// Sometimes an argument to ifdef can be classified as a keyword token.
+#ifdef __restrict
+#endif
+
// These macros do not expand to integral constants.
#define HELLO "Hello, "
#define WORLD "World"
Index: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
@@ -278,20 +278,17 @@
}
}
+inline StringRef getTokenName(const Token &Tok) {
+ return Tok.is(tok::raw_identifier) ? Tok.getRawIdentifier()
+ : Tok.getIdentifierInfo()->getName();
+}
+
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;
- }
+ std::string Id = getTokenName(MacroNameTok).str();
llvm::erase_if(Enums, [&Id](const MacroList &MacroList) {
return llvm::any_of(MacroList, [&Id](const EnumMacro &Macro) {
- return Macro.Name.getIdentifierInfo()->getName().str() == Id;
+ return getTokenName(Macro.Name).str() == Id;
});
});
}
@@ -355,8 +352,7 @@
const MacroDefinition &MD,
const MacroDirective *Undef) {
auto MatchesToken = [&MacroNameTok](const EnumMacro &Macro) {
- return Macro.Name.getIdentifierInfo()->getName() ==
- MacroNameTok.getIdentifierInfo()->getName();
+ return getTokenName(Macro.Name) == getTokenName(MacroNameTok);
};
auto It = llvm::find_if(Enums, [MatchesToken](const MacroList &MacroList) {
@@ -432,8 +428,8 @@
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();
+ "macro '%0' defines an integral constant; prefer an enum instead")
+ << getTokenName(Macro.Name).str();
}
void MacroToEnumCallbacks::fixEnumMacro(const MacroList &MacroList) const {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
