This revision was automatically updated to reflect the committed changes.
Closed by commit rL358496: [clangd] Check file path of declaring header when
deciding whether to insert… (authored by ioeric, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60687/new/
https://reviews.llvm.org/D60687
Files:
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
Index: clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
@@ -93,11 +93,10 @@
&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 @@
/*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
Index: clang-tools-extra/trunk/clangd/Headers.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/Headers.cpp
+++ clang-tools-extra/trunk/clangd/Headers.cpp
@@ -173,22 +173,21 @@
/// 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;
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -349,15 +349,18 @@
// 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();
Index: clang-tools-extra/trunk/clangd/Headers.h
===================================================================
--- clang-tools-extra/trunk/clangd/Headers.h
+++ clang-tools-extra/trunk/clangd/Headers.h
@@ -137,25 +137,22 @@
/// 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.
Index: clang-tools-extra/trunk/clangd/IncludeFixer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp
@@ -142,15 +142,17 @@
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::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());
}
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
