Skip to content

Commit

Permalink
[include-cleaner] Record macro references in #ifdef clause.
Browse files Browse the repository at this point in the history
Records macro references in #ifdef clauses as ambiguous.

Reviewed By: hokein

Differential Revision: https://reviews.llvm.org/D138559
  • Loading branch information
VitaNuova authored and hokein committed Nov 24, 2022
1 parent 0923628 commit 6ebd0aa
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 6 deletions.
50 changes: 45 additions & 5 deletions clang-tools-extra/include-cleaner/lib/Record.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,54 @@ class PPRecorder : public PPCallbacks {
recordMacroRef(MacroName, *MI);
}

void Ifdef(SourceLocation Loc, const Token &MacroNameTok,
const MacroDefinition &MD) override {
if (!Active)
return;
if (const auto *MI = MD.getMacroInfo())
recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
}

void Ifndef(SourceLocation Loc, const Token &MacroNameTok,
const MacroDefinition &MD) override {
if (!Active)
return;
if (const auto *MI = MD.getMacroInfo())
recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
}

void Elifdef(SourceLocation Loc, const Token &MacroNameTok,
const MacroDefinition &MD) override {
if (!Active)
return;
if (const auto *MI = MD.getMacroInfo())
recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
}

void Elifndef(SourceLocation Loc, const Token &MacroNameTok,
const MacroDefinition &MD) override {
if (!Active)
return;
if (const auto *MI = MD.getMacroInfo())
recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
}

void Defined(const Token &MacroNameTok, const MacroDefinition &MD,
SourceRange Range) override {
if (!Active)
return;
if (const auto *MI = MD.getMacroInfo())
recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous);
}

private:
void recordMacroRef(const Token &Tok, const MacroInfo &MI) {
void recordMacroRef(const Token &Tok, const MacroInfo &MI,
RefType RT = RefType::Explicit) {
if (MI.isBuiltinMacro())
return; // __FILE__ is not a reference.
Recorded.MacroReferences.push_back(
SymbolReference{Tok.getLocation(),
Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()},
RefType::Explicit});
Recorded.MacroReferences.push_back(SymbolReference{
Tok.getLocation(),
Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()}, RT});
}

bool Active = false;
Expand Down
41 changes: 40 additions & 1 deletion clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
//===----------------------------------------------------------------------===//

#include "clang-include-cleaner/Record.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Frontend/FrontendAction.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Testing/TestAST.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Testing/Support/Annotations.h"
#include "gmock/gmock.h"
Expand Down Expand Up @@ -217,6 +218,44 @@ TEST_F(RecordPPTest, CapturesMacroRefs) {
EXPECT_THAT(ExpOffsets, ElementsAreArray(MainFile.points("exp")));
}

TEST_F(RecordPPTest, CapturesConditionalMacroRefs) {
llvm::Annotations MainFile(R"cpp(
#define X 1
#ifdef ^X
#endif
#if defined(^X)
#endif
#ifndef ^X
#endif
#ifdef Y
#elifdef ^X
#endif
#ifndef ^X
#elifndef ^X
#endif
)cpp");

Inputs.Code = MainFile.code();
Inputs.ExtraArgs.push_back("-std=c++2b");
auto AST = build();

std::vector<unsigned> RefOffsets;
SourceManager &SM = AST.sourceManager();
for (const auto &Ref : Recorded.MacroReferences) {
auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation);
ASSERT_EQ(FID, SM.getMainFileID());
EXPECT_EQ(Ref.RT, RefType::Ambiguous);
EXPECT_EQ("X", Ref.Target.macro().Name->getName());
RefOffsets.push_back(Off);
}
EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points()));
}

// Matches an Include* on the specified line;
MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; }

Expand Down

3 comments on commit 6ebd0aa

@kazutakahirata
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this patch, I am getting:

In file included from clang-tools-extra/include-cleaner/lib/Record.cpp:17:
clang/include/clang/Lex/PPCallbacks.h:410:16: warning: ‘virtual void clang::PPCallbacks::Elifndef(clang::SourceLocation, clang::SourceRange, clang::SourceLocation)’ was hidden [-Woverloaded-virtual]
  410 |   virtual void Elifndef(SourceLocation Loc, SourceRange ConditionRange,
      |                ^~~~~~~~
clang-tools-extra/include-cleaner/lib/Record.cpp:110:8: note:   by ‘virtual void clang::include_cleaner::{anonymous}::PPRecorder::Elifndef(clang::SourceLocation, const clang::Token&, const clang::MacroDefinition&)’
  110 |   void Elifndef(SourceLocation Loc, const Token &MacroNameTok,
      |        ^~~~~~~~
In file included from clang-tools-extra/include-cleaner/lib/Record.cpp:17:
clang/include/clang/Lex/PPCallbacks.h:386:16: warning: ‘virtual void clang::PPCallbacks::Elifdef(clang::SourceLocation, clang::SourceRange, clang::SourceLocation)’ was hidden [-Woverloaded-virtual]
  386 |   virtual void Elifdef(SourceLocation Loc, SourceRange ConditionRange,
      |                ^~~~~~~
clang-tools-extra/include-cleaner/lib/Record.cpp:102:8: note:   by ‘virtual void clang::include_cleaner::{anonymous}::PPRecorder::Elifdef(clang::SourceLocation, const clang::Token&, const clang::MacroDefinition&)’
  102 |   void Elifdef(SourceLocation Loc, const Token &MacroNameTok,
      |        ^~~~~~~

I'm using gcc-12.2.0. Would you mind looking into this? Thanks in advance!

@hokein
Copy link
Collaborator

@hokein hokein commented on 6ebd0aa Nov 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, should be fixed in 454a34d.

@kazutakahirata
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @hokein!

Please sign in to comment.