ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
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.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D60687
Files:
clangd/CodeComplete.cpp
clangd/Headers.cpp
clangd/Headers.h
clangd/IncludeFixer.cpp
unittests/clangd/HeadersTests.cpp
Index: unittests/clangd/HeadersTests.cpp
===================================================================
--- unittests/clangd/HeadersTests.cpp
+++ 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: clangd/IncludeFixer.cpp
===================================================================
--- clangd/IncludeFixer.cpp
+++ 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());
}
}
}
Index: clangd/Headers.h
===================================================================
--- clangd/Headers.h
+++ 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: clangd/Headers.cpp
===================================================================
--- clangd/Headers.cpp
+++ 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: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ 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();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
