ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov, klimek.
This assumes that .inc files are supposed to be included via headers
that include them.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47187
Files:
clangd/index/CanonicalIncludes.cpp
clangd/index/CanonicalIncludes.h
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -666,6 +666,24 @@
IncludeHeader("\"the/good/header.h\""))));
}
+TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) {
+ CollectorOpts.CollectIncludePath = true;
+ CanonicalIncludes Includes;
+ Includes.addMapping(TestHeaderName, "<canonical>");
+ CollectorOpts.Includes = &Includes;
+ auto IncFile = testPath("test.inc");
+ auto IncURI = URI::createFile(IncFile).toString();
+ InMemoryFileSystem->addFile(IncFile, 0,
+ llvm::MemoryBuffer::getMemBuffer("class X {};"));
+ runSymbolCollector("#include \"test.inc\"\nclass Y {};", /*Main=*/"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader("<canonical>")),
+ AllOf(QName("Y"), DeclURI(TestHeaderURI),
+ IncludeHeader("<canonical>"))));
+}
+
TEST_F(SymbolCollectorTest, AvoidUsingFwdDeclsAsCanonicalDecls) {
CollectorOpts.CollectIncludePath = true;
Annotations Header(R"(
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -200,23 +200,29 @@
/// Gets a canonical include (URI of the header or <header> or "header") for
/// header of \p Loc.
/// Returns None if fails to get include header for \p Loc.
-/// FIXME: we should handle .inc files whose symbols are expected be exported by
-/// their containing headers.
llvm::Optional<std::string>
getIncludeHeader(llvm::StringRef QName, const SourceManager &SM,
SourceLocation Loc, const SymbolCollector::Options &Opts) {
- llvm::StringRef FilePath = SM.getFilename(Loc);
- if (FilePath.empty())
+ std::vector<std::string> Headers;
+ // Collect all headers in the #include stack.
+ while (true) {
+ if (!Loc.isValid() || SM.isInMainFile(Loc))
+ break;
+ auto FilePath = SM.getFilename(Loc);
+ if (FilePath.empty())
+ break;
+ Headers.push_back(FilePath);
+ Loc = SM.getIncludeLoc(SM.getFileID(Loc));
+ }
+ if (Headers.empty())
return llvm::None;
+ llvm::StringRef Header = Headers[0];
if (Opts.Includes) {
- llvm::StringRef Mapped = Opts.Includes->mapHeader(FilePath, QName);
- if (Mapped != FilePath)
- return (Mapped.startswith("<") || Mapped.startswith("\""))
- ? Mapped.str()
- : ("\"" + Mapped + "\"").str();
+ Header = Opts.Includes->mapHeader(Headers, QName);
+ if (Header.startswith("<") || Header.startswith("\""))
+ return Header.str();
}
-
- return toURI(SM, SM.getFilename(Loc), Opts);
+ return toURI(SM, Header, Opts);
}
// Return the symbol location of the given declaration `D`.
Index: clangd/index/CanonicalIncludes.h
===================================================================
--- clangd/index/CanonicalIncludes.h
+++ clangd/index/CanonicalIncludes.h
@@ -48,9 +48,12 @@
void addSymbolMapping(llvm::StringRef QualifiedName,
llvm::StringRef CanonicalPath);
- /// Returns the canonical include for symbol with \p QualifiedName, which is
- /// declared in \p Header
- llvm::StringRef mapHeader(llvm::StringRef Header,
+ /// Returns the canonical include for symbol with \p QualifiedName. \p Headers
+ /// is a stack of #includes that starts from the last included header (i.e.
+ /// header that defines the symbol) to the header file that is directly
+ /// included by the main file. When mapping headers, this skips files that are
+ /// not supposed to be #included directly e.g. .inc file.
+ llvm::StringRef mapHeader(llvm::ArrayRef<std::string> Headers,
llvm::StringRef QualifiedName) const;
private:
Index: clangd/index/CanonicalIncludes.cpp
===================================================================
--- clangd/index/CanonicalIncludes.cpp
+++ clangd/index/CanonicalIncludes.cpp
@@ -33,12 +33,22 @@
}
llvm::StringRef
-CanonicalIncludes::mapHeader(llvm::StringRef Header,
+CanonicalIncludes::mapHeader(llvm::ArrayRef<std::string> Headers,
llvm::StringRef QualifiedName) const {
+ assert(!Headers.empty());
auto SE = SymbolMapping.find(QualifiedName);
if (SE != SymbolMapping.end())
return SE->second;
std::lock_guard<std::mutex> Lock(RegexMutex);
+ auto I = std::find_if(Headers.begin(), Headers.end(),
+ [](llvm::StringRef Include) {
+ // Skip .inc file whose including header file should
+ // be #included instead.
+ return !Include.endswith(".inc");
+ });
+ if (I == Headers.end())
+ return Headers[0]; // Fallback to the defining header.
+ llvm::StringRef Header = *I;
for (auto &Entry : RegexHeaderMappingTable) {
#ifndef NDEBUG
std::string Dummy;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
