[clang-tools-extra] r358496 - [clangd] Check file path of declaring header when deciding whether to insert include.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 16 07:35:49 PDT 2019
Author: ioeric
Date: Tue Apr 16 07:35:49 2019
New Revision: 358496
URL: http://llvm.org/viewvc/llvm-project?rev=358496&view=rev
Log:
[clangd] Check file path of declaring header when deciding whether to insert include.
Summary:
Previously, we would use include spelling of the declaring header to check
whether the inserted header is the same as the main file. This doesn't help because
we only have file path of the main file.
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60687
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Headers.cpp
clang-tools-extra/trunk/clangd/Headers.h
clang-tools-extra/trunk/clangd/IncludeFixer.cpp
clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358496&r1=358495&r2=358496&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Apr 16 07:35:49 2019
@@ -349,15 +349,18 @@ struct CodeCompletionBuilder {
// Turn absolute path into a literal string that can be #included.
auto Inserted = [&](llvm::StringRef Header)
-> llvm::Expected<std::pair<std::string, bool>> {
- auto ResolvedDeclaring =
- toHeaderFile(C.IndexResult->CanonicalDeclaration.FileURI, FileName);
+ auto DeclaringURI =
+ URI::parse(C.IndexResult->CanonicalDeclaration.FileURI);
+ if (!DeclaringURI)
+ return DeclaringURI.takeError();
+ auto ResolvedDeclaring = URI::resolve(*DeclaringURI, FileName);
if (!ResolvedDeclaring)
return ResolvedDeclaring.takeError();
auto ResolvedInserted = toHeaderFile(Header, FileName);
if (!ResolvedInserted)
return ResolvedInserted.takeError();
return std::make_pair(
- Includes.calculateIncludePath(*ResolvedDeclaring, *ResolvedInserted),
+ Includes.calculateIncludePath(*ResolvedInserted),
Includes.shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted));
};
bool ShouldInsert = C.headerToInsertIfAllowed(Opts).hasValue();
Modified: clang-tools-extra/trunk/clangd/Headers.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.cpp?rev=358496&r1=358495&r2=358496&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Headers.cpp (original)
+++ clang-tools-extra/trunk/clangd/Headers.cpp Tue Apr 16 07:35:49 2019
@@ -173,22 +173,21 @@ void IncludeInserter::addExisting(const
/// FIXME(ioeric): we might not want to insert an absolute include path if the
/// path is not shortened.
bool IncludeInserter::shouldInsertInclude(
- const HeaderFile &DeclaringHeader, const HeaderFile &InsertedHeader) const {
- assert(DeclaringHeader.valid() && InsertedHeader.valid());
+ PathRef DeclaringHeader, const HeaderFile &InsertedHeader) const {
+ assert(InsertedHeader.valid());
if (!HeaderSearchInfo && !InsertedHeader.Verbatim)
return false;
- if (FileName == DeclaringHeader.File || FileName == InsertedHeader.File)
+ if (FileName == DeclaringHeader || FileName == InsertedHeader.File)
return false;
auto Included = [&](llvm::StringRef Header) {
return IncludedHeaders.find(Header) != IncludedHeaders.end();
};
- return !Included(DeclaringHeader.File) && !Included(InsertedHeader.File);
+ return !Included(DeclaringHeader) && !Included(InsertedHeader.File);
}
std::string
-IncludeInserter::calculateIncludePath(const HeaderFile &DeclaringHeader,
- const HeaderFile &InsertedHeader) const {
- assert(DeclaringHeader.valid() && InsertedHeader.valid());
+IncludeInserter::calculateIncludePath(const HeaderFile &InsertedHeader) const {
+ assert(InsertedHeader.valid());
if (InsertedHeader.Verbatim)
return InsertedHeader.File;
bool IsSystem = false;
Modified: clang-tools-extra/trunk/clangd/Headers.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.h?rev=358496&r1=358495&r2=358496&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Headers.h (original)
+++ clang-tools-extra/trunk/clangd/Headers.h Tue Apr 16 07:35:49 2019
@@ -137,25 +137,22 @@ public:
/// in \p Inclusions (including those included via different paths).
/// - \p DeclaringHeader or \p InsertedHeader is the same as \p File.
///
- /// \param DeclaringHeader is the original header corresponding to \p
+ /// \param DeclaringHeader is path of the original header corresponding to \p
/// InsertedHeader e.g. the header that declares a symbol.
/// \param InsertedHeader The preferred header to be inserted. This could be
/// the same as DeclaringHeader but must be provided.
- bool shouldInsertInclude(const HeaderFile &DeclaringHeader,
+ bool shouldInsertInclude(PathRef DeclaringHeader,
const HeaderFile &InsertedHeader) const;
/// Determines the preferred way to #include a file, taking into account the
/// search path. Usually this will prefer a shorter representation like
/// 'Foo/Bar.h' over a longer one like 'Baz/include/Foo/Bar.h'.
///
- /// \param DeclaringHeader is the original header corresponding to \p
- /// InsertedHeader e.g. the header that declares a symbol.
/// \param InsertedHeader The preferred header to be inserted. This could be
/// the same as DeclaringHeader but must be provided.
///
/// \return A quoted "path" or <path> to be included.
- std::string calculateIncludePath(const HeaderFile &DeclaringHeader,
- const HeaderFile &InsertedHeader) const;
+ std::string calculateIncludePath(const HeaderFile &InsertedHeader) const;
/// Calculates an edit that inserts \p VerbatimHeader into code. If the header
/// is already included, this returns None.
Modified: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/IncludeFixer.cpp?rev=358496&r1=358495&r2=358496&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp Tue Apr 16 07:35:49 2019
@@ -142,15 +142,17 @@ std::vector<Fix> IncludeFixer::fixIncomp
std::vector<Fix> IncludeFixer::fixesForSymbols(const SymbolSlab &Syms) const {
auto Inserted = [&](const Symbol &Sym, llvm::StringRef Header)
-> llvm::Expected<std::pair<std::string, bool>> {
- auto ResolvedDeclaring =
- toHeaderFile(Sym.CanonicalDeclaration.FileURI, File);
+ auto DeclaringURI = URI::parse(Sym.CanonicalDeclaration.FileURI);
+ if (!DeclaringURI)
+ return DeclaringURI.takeError();
+ auto ResolvedDeclaring = URI::resolve(*DeclaringURI, File);
if (!ResolvedDeclaring)
return ResolvedDeclaring.takeError();
auto ResolvedInserted = toHeaderFile(Header, File);
if (!ResolvedInserted)
return ResolvedInserted.takeError();
return std::make_pair(
- Inserter->calculateIncludePath(*ResolvedDeclaring, *ResolvedInserted),
+ Inserter->calculateIncludePath(*ResolvedInserted),
Inserter->shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted));
};
@@ -173,8 +175,8 @@ std::vector<Fix> IncludeFixer::fixesForS
{std::move(*Edit)}});
}
} else {
- vlog("Failed to calculate include insertion for {0} into {1}: {2}",
- File, Inc, ToInclude.takeError());
+ vlog("Failed to calculate include insertion for {0} into {1}: {2}", Inc,
+ File, ToInclude.takeError());
}
}
}
Modified: clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp?rev=358496&r1=358495&r2=358496&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp Tue Apr 16 07:35:49 2019
@@ -93,11 +93,10 @@ protected:
&Clang->getPreprocessor().getHeaderSearchInfo());
for (const auto &Inc : Inclusions)
Inserter.addExisting(Inc);
- auto Declaring = ToHeaderFile(Original);
auto Inserted = ToHeaderFile(Preferred);
- if (!Inserter.shouldInsertInclude(Declaring, Inserted))
+ if (!Inserter.shouldInsertInclude(Original, Inserted))
return "";
- std::string Path = Inserter.calculateIncludePath(Declaring, Inserted);
+ std::string Path = Inserter.calculateIncludePath(Inserted);
Action.EndSourceFile();
return Path;
}
@@ -258,16 +257,15 @@ TEST(Headers, NoHeaderSearchInfo) {
/*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
auto HeaderPath = testPath("sub/bar.h");
- auto Declaring = HeaderFile{HeaderPath, /*Verbatim=*/false};
auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};
auto Verbatim = HeaderFile{"<x>", /*Verbatim=*/true};
- EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Inserting),
+ EXPECT_EQ(Inserter.calculateIncludePath(Inserting),
"\"" + HeaderPath + "\"");
- EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Inserting), false);
+ EXPECT_EQ(Inserter.shouldInsertInclude(HeaderPath, Inserting), false);
- EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Verbatim), "<x>");
- EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Verbatim), true);
+ EXPECT_EQ(Inserter.calculateIncludePath(Verbatim), "<x>");
+ EXPECT_EQ(Inserter.shouldInsertInclude(HeaderPath, Verbatim), true);
}
} // namespace
More information about the cfe-commits
mailing list