[clangd] Replay macro definitions from preamble for clang-tidy checks (#202495)
Clang-tidy checkers observe preprocessor events via PPCallbacks. When using a preamble, macro definitions in the preamble region of the main file are not replayed during the main-file build, causing checkers like bugprone-reserved-identifier to miss them. This patch extends ReplayPreamble::replay() to also replay MacroDefined events for macros defined directly in the preamble region of the open file, similar to how InclusionDirective events are already replayed. Fixes: https://github.com/clangd/clangd/issues/2501
This commit is contained in:
parent
2964323a93
commit
2cd778c206
2 changed files with 83 additions and 9 deletions
|
|
@ -142,7 +142,8 @@ public:
|
|||
// Attach preprocessor hooks such that preamble events will be injected at
|
||||
// the appropriate time.
|
||||
// Events will be delivered to the *currently registered* PP callbacks.
|
||||
static void attach(std::vector<Inclusion> Includes, CompilerInstance &Clang,
|
||||
static void attach(std::vector<Inclusion> Includes,
|
||||
const MainFileMacros Macros, CompilerInstance &Clang,
|
||||
const PreambleBounds &PB) {
|
||||
auto &PP = Clang.getPreprocessor();
|
||||
auto *ExistingCallbacks = PP.getPPCallbacks();
|
||||
|
|
@ -150,8 +151,8 @@ public:
|
|||
if (!ExistingCallbacks)
|
||||
return;
|
||||
PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(new ReplayPreamble(
|
||||
std::move(Includes), ExistingCallbacks, Clang.getSourceManager(), PP,
|
||||
Clang.getLangOpts(), PB)));
|
||||
std::move(Includes), std::move(Macros), ExistingCallbacks,
|
||||
Clang.getSourceManager(), PP, Clang.getLangOpts(), PB)));
|
||||
// We're relying on the fact that addPPCallbacks keeps the old PPCallbacks
|
||||
// around, creating a chaining wrapper. Guard against other implementations.
|
||||
assert(PP.getPPCallbacks() != ExistingCallbacks &&
|
||||
|
|
@ -159,10 +160,12 @@ public:
|
|||
}
|
||||
|
||||
private:
|
||||
ReplayPreamble(std::vector<Inclusion> Includes, PPCallbacks *Delegate,
|
||||
const SourceManager &SM, Preprocessor &PP,
|
||||
const LangOptions &LangOpts, const PreambleBounds &PB)
|
||||
: Includes(std::move(Includes)), Delegate(Delegate), SM(SM), PP(PP) {
|
||||
ReplayPreamble(std::vector<Inclusion> Includes, MainFileMacros Macros,
|
||||
PPCallbacks *Delegate, const SourceManager &SM,
|
||||
Preprocessor &PP, const LangOptions &LangOpts,
|
||||
const PreambleBounds &PB)
|
||||
: Includes(std::move(Includes)), Macros(std::move(Macros)),
|
||||
Delegate(Delegate), SM(SM), PP(PP) {
|
||||
// Only tokenize the preamble section of the main file, as we are not
|
||||
// interested in the rest of the tokens.
|
||||
MainFileTokens = syntax::tokenize(
|
||||
|
|
@ -193,6 +196,24 @@ private:
|
|||
}
|
||||
|
||||
void replay() {
|
||||
// Replay macro definitions from the preamble region of the main file,
|
||||
// so that clang-tidy checks can observe them.
|
||||
for (const auto &[SID, Refs] : Macros.MacroRefs) {
|
||||
for (const auto &Ref : Refs) {
|
||||
if (!Ref.IsDefinition)
|
||||
continue;
|
||||
auto Loc = SM.getComposedLoc(SM.getMainFileID(), Ref.StartOffset);
|
||||
Token Tok;
|
||||
if (Lexer::getRawToken(Loc, Tok, SM, PP.getLangOpts(), false))
|
||||
continue;
|
||||
if (auto *II = PP.getIdentifierInfo(Tok.getRawIdentifier())) {
|
||||
Tok.setIdentifierInfo(II);
|
||||
Tok.setKind(tok::identifier);
|
||||
if (auto *MD = PP.getLocalMacroDirective(II))
|
||||
Delegate->MacroDefined(Tok, MD);
|
||||
}
|
||||
}
|
||||
}
|
||||
for (const auto &Inc : Includes) {
|
||||
OptionalFileEntryRef File;
|
||||
if (Inc.Resolved != "")
|
||||
|
|
@ -250,6 +271,7 @@ private:
|
|||
}
|
||||
|
||||
const std::vector<Inclusion> Includes;
|
||||
const MainFileMacros Macros;
|
||||
PPCallbacks *Delegate;
|
||||
const SourceManager &SM;
|
||||
Preprocessor &PP;
|
||||
|
|
@ -667,8 +689,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
|
|||
Includes = Preamble->Includes;
|
||||
Includes.MainFileIncludes = Patch->preambleIncludes();
|
||||
// Replay the preamble includes so that clang-tidy checks can see them.
|
||||
ReplayPreamble::attach(Patch->preambleIncludes(), *Clang,
|
||||
Patch->modifiedBounds());
|
||||
ReplayPreamble::attach(Patch->preambleIncludes(), Patch->mainFileMacros(),
|
||||
*Clang, Patch->modifiedBounds());
|
||||
PI = *Preamble->Pragmas;
|
||||
}
|
||||
// Important: collectIncludeStructure is registered *after* ReplayPreamble!
|
||||
|
|
|
|||
|
|
@ -67,6 +67,7 @@ struct Inclusion {
|
|||
};
|
||||
static std::vector<Inclusion> Includes;
|
||||
static std::vector<syntax::Token> SkippedFiles;
|
||||
static std::vector<std::string> DefinedMacros;
|
||||
struct ReplayPreamblePPCallback : public PPCallbacks {
|
||||
const SourceManager &SM;
|
||||
explicit ReplayPreamblePPCallback(const SourceManager &SM) : SM(SM) {}
|
||||
|
|
@ -84,6 +85,11 @@ struct ReplayPreamblePPCallback : public PPCallbacks {
|
|||
SrcMgr::CharacteristicKind) override {
|
||||
SkippedFiles.emplace_back(FilenameTok);
|
||||
}
|
||||
|
||||
void MacroDefined(const Token &MacroNameTok,
|
||||
const MacroDirective *MD) override {
|
||||
DefinedMacros.push_back(MacroNameTok.getIdentifierInfo()->getName().str());
|
||||
}
|
||||
};
|
||||
struct ReplayPreambleCheck : public tidy::ClangTidyCheck {
|
||||
ReplayPreambleCheck(StringRef Name, tidy::ClangTidyContext *Context)
|
||||
|
|
@ -107,6 +113,52 @@ MATCHER_P(rangeIs, R, "") {
|
|||
return arg.beginOffset() == R.Begin && arg.endOffset() == R.End;
|
||||
}
|
||||
|
||||
TEST(ReplayPreambleTest, MacroDefinitions) {
|
||||
DefinedMacros.clear();
|
||||
|
||||
TestTU TU;
|
||||
TU.ClangTidyProvider = addTidyChecks(CheckName);
|
||||
TU.Code = R"cpp(
|
||||
#ifndef _TEST_H
|
||||
#define _TEST_H
|
||||
#define _TEST_MACRO
|
||||
#endif
|
||||
)cpp";
|
||||
|
||||
Config Cfg;
|
||||
Cfg.Diagnostics.ClangTidy.FastCheckFilter = Config::FastCheckPolicy::Loose;
|
||||
WithContextValue WithCfg(Config::Key, std::move(Cfg));
|
||||
|
||||
TU.build();
|
||||
|
||||
EXPECT_THAT(DefinedMacros, testing::Contains(std::string("_TEST_H")));
|
||||
EXPECT_THAT(DefinedMacros, testing::Contains(std::string("_TEST_MACRO")));
|
||||
}
|
||||
|
||||
TEST(ReplayPreambleTest, MacroDefinitionsPartialPreamble) {
|
||||
DefinedMacros.clear();
|
||||
|
||||
TestTU TU;
|
||||
TU.ClangTidyProvider = addTidyChecks(CheckName);
|
||||
TU.Code = R"cpp(
|
||||
#ifndef _TEST_H
|
||||
#define _TEST_H
|
||||
void unused(void);
|
||||
#define _TEST_MACRO
|
||||
#endif
|
||||
)cpp";
|
||||
|
||||
Config Cfg;
|
||||
Cfg.Diagnostics.ClangTidy.FastCheckFilter = Config::FastCheckPolicy::Loose;
|
||||
WithContextValue WithCfg(Config::Key, std::move(Cfg));
|
||||
|
||||
TU.build();
|
||||
|
||||
// Both macros should be seen by clang-tidy
|
||||
EXPECT_THAT(DefinedMacros, testing::Contains(std::string("_TEST_H")));
|
||||
EXPECT_THAT(DefinedMacros, testing::Contains(std::string("_TEST_MACRO")));
|
||||
}
|
||||
|
||||
TEST(ReplayPreambleTest, IncludesAndSkippedFiles) {
|
||||
TestTU TU;
|
||||
// This check records inclusion directives replayed by clangd.
|
||||
|
|
|
|||
Loading…
Reference in a new issue