[clang-tools-extra] r333188 - [clangd] Skip .inc headers when canonicalizing header #include.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Thu May 24 07:40:24 PDT 2018
Author: ioeric
Date: Thu May 24 07:40:24 2018
New Revision: 333188
URL: http://llvm.org/viewvc/llvm-project?rev=333188&view=rev
Log:
[clangd] Skip .inc headers when canonicalizing header #include.
Summary:
This assumes that .inc files are supposed to be included via headers
that include them.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47187
Modified:
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp?rev=333188&r1=333187&r2=333188&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp Thu May 24 07:40:24 2018
@@ -8,6 +8,8 @@
//===----------------------------------------------------------------------===//
#include "CanonicalIncludes.h"
+#include "../Headers.h"
+#include "clang/Driver/Types.h"
#include "llvm/Support/Regex.h"
namespace clang {
@@ -33,12 +35,33 @@ void CanonicalIncludes::addSymbolMapping
}
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);
+ // Find the first header such that the extension is not '.inc', and isn't a
+ // recognized non-header file
+ 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 declaring header.
+ StringRef Header = *I;
+ // If Header is not expected be included (e.g. .cc file), we fall back to
+ // the declaring header.
+ StringRef Ext = llvm::sys::path::extension(Header).trim('.');
+ // Include-able headers must have precompile type. Treat files with
+ // non-recognized extenstions (TY_INVALID) as headers.
+ auto ExtType = driver::types::lookupTypeForExtension(Ext);
+ if ((ExtType != driver::types::TY_INVALID) &&
+ !driver::types::onlyPrecompileType(ExtType))
+ return Headers[0];
for (auto &Entry : RegexHeaderMappingTable) {
#ifndef NDEBUG
std::string Dummy;
@@ -65,9 +88,8 @@ collectIWYUHeaderMaps(CanonicalIncludes
// FIXME(ioeric): resolve the header and store actual file path. For now,
// we simply assume the written header is suitable to be #included.
Includes->addMapping(PP.getSourceManager().getFilename(Range.getBegin()),
- (Text.startswith("<") || Text.startswith("\""))
- ? Text.str()
- : ("\"" + Text + "\"").str());
+ isLiteralInclude(Text) ? Text.str()
+ : ("\"" + Text + "\"").str());
return false;
}
Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h?rev=333188&r1=333187&r2=333188&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h Thu May 24 07:40:24 2018
@@ -48,9 +48,10 @@ public:
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 the include stack: Headers.front() is the file declaring the
+ /// symbol, and Headers.back() is the main file.
+ llvm::StringRef mapHeader(llvm::ArrayRef<std::string> Headers,
llvm::StringRef QualifiedName) const;
private:
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=333188&r1=333187&r2=333188&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Thu May 24 07:40:24 2018
@@ -200,23 +200,31 @@ bool shouldCollectIncludePath(index::Sym
/// 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 the #include stack.
+ while (true) {
+ if (!Loc.isValid())
+ break;
+ auto FilePath = SM.getFilename(Loc);
+ if (FilePath.empty())
+ break;
+ Headers.push_back(FilePath);
+ if (SM.isInMainFile(Loc))
+ break;
+ 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`.
Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=333188&r1=333187&r2=333188&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Thu May 24 07:40:24 2018
@@ -131,9 +131,9 @@ public:
auto Factory = llvm::make_unique<SymbolIndexActionFactory>(
CollectorOpts, PragmaHandler.get());
- std::vector<std::string> Args = {"symbol_collector", "-fsyntax-only",
- "-std=c++11", "-include",
- TestHeaderName, TestFileName};
+ std::vector<std::string> Args = {
+ "symbol_collector", "-fsyntax-only", "-xc++", "-std=c++11",
+ "-include", TestHeaderName, TestFileName};
Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
tooling::ToolInvocation Invocation(
Args,
@@ -666,6 +666,70 @@ TEST_F(SymbolCollectorTest, IWYUPragmaWi
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, MainFileIsHeaderWhenSkipIncFile) {
+ CollectorOpts.CollectIncludePath = true;
+ CanonicalIncludes Includes;
+ CollectorOpts.Includes = &Includes;
+ TestFileName = testPath("main.h");
+ TestFileURI = URI::createFile(TestFileName).toString();
+ auto IncFile = testPath("test.inc");
+ auto IncURI = URI::createFile(IncFile).toString();
+ InMemoryFileSystem->addFile(IncFile, 0,
+ llvm::MemoryBuffer::getMemBuffer("class X {};"));
+ runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader(TestFileURI))));
+}
+
+TEST_F(SymbolCollectorTest, MainFileIsHeaderWithoutExtensionWhenSkipIncFile) {
+ CollectorOpts.CollectIncludePath = true;
+ CanonicalIncludes Includes;
+ CollectorOpts.Includes = &Includes;
+ TestFileName = testPath("no_ext_main");
+ TestFileURI = URI::createFile(TestFileName).toString();
+ auto IncFile = testPath("test.inc");
+ auto IncURI = URI::createFile(IncFile).toString();
+ InMemoryFileSystem->addFile(IncFile, 0,
+ llvm::MemoryBuffer::getMemBuffer("class X {};"));
+ runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader(TestFileURI))));
+}
+
+TEST_F(SymbolCollectorTest, FallbackToIncFileWhenIncludingFileIsCC) {
+ CollectorOpts.CollectIncludePath = true;
+ CanonicalIncludes Includes;
+ CollectorOpts.Includes = &Includes;
+ auto IncFile = testPath("test.inc");
+ auto IncURI = URI::createFile(IncFile).toString();
+ InMemoryFileSystem->addFile(IncFile, 0,
+ llvm::MemoryBuffer::getMemBuffer("class X {};"));
+ runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader(IncURI))));
+}
+
TEST_F(SymbolCollectorTest, AvoidUsingFwdDeclsAsCanonicalDecls) {
CollectorOpts.CollectIncludePath = true;
Annotations Header(R"(
More information about the cfe-commits
mailing list