[clang-tidy] cppcoreguidelines-macro-usage: print macro names
Summary:
The macro may not have location (or more generally, the location may not exist),
e.g. if it originates from compiler's command-line.
The check complains on all the macros, even those without the location info.
Which means, it only says it does not like it. What is 'it'? I have no idea.
If we don't print the name, then there is no way to deal with that situation.
And in general, not printing name here forces the user to try to understand,
given, the macro definition location, what is the macro name?
This isn't fun.
Also, ignores-by-default the macros originating from command-line,
with an option to not ignore those.
I suspect some more issues may crop up later.
Reviewers: JonasToth, aaron.ballman, hokein, xazax.hun, alexfh
Reviewed By: JonasToth, aaron.ballman
Subscribers: nemanjai, kbarton, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53817
llvm-svn: 345610
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
index 47f563b..8b893c7 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
@@ -31,32 +31,41 @@
class MacroUsageCallbacks : public PPCallbacks {
public:
- MacroUsageCallbacks(MacroUsageCheck *Check, StringRef RegExp, bool CapsOnly)
- : Check(Check), RegExp(RegExp), CheckCapsOnly(CapsOnly) {}
+ MacroUsageCallbacks(MacroUsageCheck *Check, const SourceManager &SM,
+ StringRef RegExp, bool CapsOnly, bool IgnoreCommandLine)
+ : Check(Check), SM(SM), RegExp(RegExp), CheckCapsOnly(CapsOnly),
+ IgnoreCommandLineMacros(IgnoreCommandLine) {}
void MacroDefined(const Token &MacroNameTok,
const MacroDirective *MD) override {
if (MD->getMacroInfo()->isUsedForHeaderGuard() ||
MD->getMacroInfo()->getNumTokens() == 0)
return;
+ if (IgnoreCommandLineMacros &&
+ SM.isWrittenInCommandLineFile(MD->getLocation()))
+ return;
+
StringRef MacroName = MacroNameTok.getIdentifierInfo()->getName();
if (!CheckCapsOnly && !llvm::Regex(RegExp).match(MacroName))
- Check->warnMacro(MD);
+ Check->warnMacro(MD, MacroName);
if (CheckCapsOnly && !isCapsOnly(MacroName))
- Check->warnNaming(MD);
+ Check->warnNaming(MD, MacroName);
}
private:
MacroUsageCheck *Check;
+ const SourceManager &SM;
StringRef RegExp;
bool CheckCapsOnly;
+ bool IgnoreCommandLineMacros;
};
} // namespace
void MacroUsageCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "AllowedRegexp", AllowedRegexp);
Options.store(Opts, "CheckCapsOnly", CheckCapsOnly);
+ Options.store(Opts, "IgnoreCommandLineMacros", IgnoreCommandLineMacros);
}
void MacroUsageCheck::registerPPCallbacks(CompilerInstance &Compiler) {
@@ -64,31 +73,34 @@
return;
Compiler.getPreprocessor().addPPCallbacks(
- llvm::make_unique<MacroUsageCallbacks>(this, AllowedRegexp,
- CheckCapsOnly));
+ llvm::make_unique<MacroUsageCallbacks>(this, Compiler.getSourceManager(),
+ AllowedRegexp, CheckCapsOnly,
+ IgnoreCommandLineMacros));
}
-void MacroUsageCheck::warnMacro(const MacroDirective *MD) {
+void MacroUsageCheck::warnMacro(const MacroDirective *MD, StringRef MacroName) {
StringRef Message =
- "macro used to declare a constant; consider using a 'constexpr' "
+ "macro '%0' used to declare a constant; consider using a 'constexpr' "
"constant";
/// A variadic macro is function-like at the same time. Therefore variadic
/// macros are checked first and will be excluded for the function-like
/// diagnostic.
if (MD->getMacroInfo()->isVariadic())
- Message = "variadic macro used; consider using a 'constexpr' "
+ Message = "variadic macro '%0' used; consider using a 'constexpr' "
"variadic template function";
else if (MD->getMacroInfo()->isFunctionLike())
- Message = "function-like macro used; consider a 'constexpr' template "
+ Message = "function-like macro '%0' used; consider a 'constexpr' template "
"function";
- diag(MD->getLocation(), Message);
+ diag(MD->getLocation(), Message) << MacroName;
}
-void MacroUsageCheck::warnNaming(const MacroDirective *MD) {
+void MacroUsageCheck::warnNaming(const MacroDirective *MD,
+ StringRef MacroName) {
diag(MD->getLocation(), "macro definition does not define the macro name "
- "using all uppercase characters");
+ "'%0' using all uppercase characters")
+ << MacroName;
}
} // namespace cppcoreguidelines
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.h
index ff33310..a39d55a 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.h
@@ -28,17 +28,20 @@
MacroUsageCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
AllowedRegexp(Options.get("AllowedRegexp", "^DEBUG_*")),
- CheckCapsOnly(Options.get("CheckCapsOnly", 0)) {}
+ CheckCapsOnly(Options.get("CheckCapsOnly", 0)),
+ IgnoreCommandLineMacros(Options.get("IgnoreCommandLineMacros", 1)) {}
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerPPCallbacks(CompilerInstance &Compiler) override;
- void warnMacro(const MacroDirective *MD);
- void warnNaming(const MacroDirective *MD);
+ void warnMacro(const MacroDirective *MD, StringRef MacroName);
+ void warnNaming(const MacroDirective *MD, StringRef MacroName);
private:
/// A regular expression that defines how allowed macros must look like.
std::string AllowedRegexp;
/// Control if only the check shall only test on CAPS_ONLY macros.
bool CheckCapsOnly;
+ /// Should the macros without a valid location be diagnosed?
+ bool IgnoreCommandLineMacros;
};
} // namespace cppcoreguidelines