kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147139
Files:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -384,6 +384,35 @@
EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
}
+TEST(IncludeCleaner, MacroExpandedThroughIncludes) {
+ Annotations MainFile(R"cpp(
+ #include "all.h"
+ #define FOO(X) const Foo *X
+ void foo() {
+ #include [["expander.inc"]]
+ }
+)cpp");
+
+ TestTU TU;
+ TU.AdditionalFiles["expander.inc"] = guard("FOO(f1);FOO(f2);");
+ TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+ TU.AdditionalFiles["all.h"] = guard("#include \"foo.h\"");
+
+ TU.Code = MainFile.code();
+ ParsedAST AST = TU.build();
+
+ auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
+ // FIXME: Deduplicate references resulting from expansion of the same macro
in
+ // multiple places.
+ EXPECT_THAT(Findings, testing::SizeIs(2));
+ auto RefRange = Findings.front().SymRefRange;
+ auto &SM = AST.getSourceManager();
+ EXPECT_EQ(RefRange.file(), SM.getMainFileID());
+ // FIXME: Point at the spelling location, rather than the include.
+ EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
+ EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -51,9 +51,11 @@
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
+#include <cassert>
#include <iterator>
#include <optional>
#include <string>
+#include <utility>
#include <vector>
namespace clang {
@@ -266,7 +268,6 @@
}
} // namespace
-
std::vector<include_cleaner::SymbolReference>
collectMacroReferences(ParsedAST &AST) {
const auto &SM = AST.getSourceManager();
@@ -398,6 +399,10 @@
// FIXME: Use presumed locations to map such usages back to patched
// locations safely.
auto Loc = SM.getFileLoc(Ref.RefLocation);
+ // File locations can be outside of the main file if macro is expanded
+ // through an #include.
+ while (SM.getFileID(Loc) != SM.getMainFileID())
+ Loc = SM.getIncludeLoc(SM.getFileID(Loc));
const auto *Token = AST.getTokens().spelledTokenAt(Loc);
MissingIncludeDiagInfo DiagInfo{Ref.Target, Token->range(SM),
Providers};
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -384,6 +384,35 @@
EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
}
+TEST(IncludeCleaner, MacroExpandedThroughIncludes) {
+ Annotations MainFile(R"cpp(
+ #include "all.h"
+ #define FOO(X) const Foo *X
+ void foo() {
+ #include [["expander.inc"]]
+ }
+)cpp");
+
+ TestTU TU;
+ TU.AdditionalFiles["expander.inc"] = guard("FOO(f1);FOO(f2);");
+ TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+ TU.AdditionalFiles["all.h"] = guard("#include \"foo.h\"");
+
+ TU.Code = MainFile.code();
+ ParsedAST AST = TU.build();
+
+ auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
+ // FIXME: Deduplicate references resulting from expansion of the same macro in
+ // multiple places.
+ EXPECT_THAT(Findings, testing::SizeIs(2));
+ auto RefRange = Findings.front().SymRefRange;
+ auto &SM = AST.getSourceManager();
+ EXPECT_EQ(RefRange.file(), SM.getMainFileID());
+ // FIXME: Point at the spelling location, rather than the include.
+ EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
+ EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -51,9 +51,11 @@
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
+#include <cassert>
#include <iterator>
#include <optional>
#include <string>
+#include <utility>
#include <vector>
namespace clang {
@@ -266,7 +268,6 @@
}
} // namespace
-
std::vector<include_cleaner::SymbolReference>
collectMacroReferences(ParsedAST &AST) {
const auto &SM = AST.getSourceManager();
@@ -398,6 +399,10 @@
// FIXME: Use presumed locations to map such usages back to patched
// locations safely.
auto Loc = SM.getFileLoc(Ref.RefLocation);
+ // File locations can be outside of the main file if macro is expanded
+ // through an #include.
+ while (SM.getFileID(Loc) != SM.getMainFileID())
+ Loc = SM.getIncludeLoc(SM.getFileID(Loc));
const auto *Token = AST.getTokens().spelledTokenAt(Loc);
MissingIncludeDiagInfo DiagInfo{Ref.Target, Token->range(SM),
Providers};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
