[clang-tools-extra] r365005 - [clang][HeaderSearch] Shorten paths for includes in mainfile's directory

Kadir Cetinkaya via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 3 00:47:19 PDT 2019


Author: kadircet
Date: Wed Jul  3 00:47:19 2019
New Revision: 365005

URL: http://llvm.org/viewvc/llvm-project?rev=365005&view=rev
Log:
[clang][HeaderSearch] Shorten paths for includes in mainfile's directory

Summary:
Currently HeaderSearch only looks at SearchDir's passed into it, but in
addition to those paths headers can be relative to including file's directory.

This patch makes sure that is taken into account.

Reviewers: gribozavr

Subscribers: jkorous, arphaman, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D63295

Modified:
    clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp
    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/clangd/unittests/CodeCompleteTests.cpp
    clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp

Modified: clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp?rev=365005&r1=365004&r2=365005&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp Wed Jul  3 00:47:19 2019
@@ -314,9 +314,9 @@ std::string IncludeFixerSemaSource::mini
   if (!Entry)
     return Include;
 
-  bool IsSystem;
+  bool IsSystem = false;
   std::string Suggestion =
-      HeaderSearch.suggestPathToFileForDiagnostics(Entry, &IsSystem);
+      HeaderSearch.suggestPathToFileForDiagnostics(Entry, "", &IsSystem);
 
   return IsSystem ? '<' + Suggestion + '>' : '"' + Suggestion + '"';
 }

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=365005&r1=365004&r2=365005&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Jul  3 00:47:19 2019
@@ -328,7 +328,7 @@ struct CodeCompletionBuilder {
       if (!ResolvedInserted)
         return ResolvedInserted.takeError();
       return std::make_pair(
-          Includes.calculateIncludePath(*ResolvedInserted),
+          Includes.calculateIncludePath(*ResolvedInserted, FileName),
           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=365005&r1=365004&r2=365005&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Headers.cpp (original)
+++ clang-tools-extra/trunk/clangd/Headers.cpp Wed Jul  3 00:47:19 2019
@@ -14,6 +14,7 @@
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/HeaderSearch.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
@@ -186,15 +187,18 @@ bool IncludeInserter::shouldInsertInclud
 }
 
 std::string
-IncludeInserter::calculateIncludePath(const HeaderFile &InsertedHeader) const {
+IncludeInserter::calculateIncludePath(const HeaderFile &InsertedHeader,
+                                      llvm::StringRef IncludingFile) const {
   assert(InsertedHeader.valid());
   if (InsertedHeader.Verbatim)
     return InsertedHeader.File;
   bool IsSystem = false;
+  // FIXME(kadircet): Handle same directory includes even if there is no
+  // HeaderSearchInfo.
   if (!HeaderSearchInfo)
     return "\"" + InsertedHeader.File + "\"";
   std::string Suggested = HeaderSearchInfo->suggestPathToFileForDiagnostics(
-      InsertedHeader.File, BuildDir, &IsSystem);
+      InsertedHeader.File, BuildDir, IncludingFile, &IsSystem);
   if (IsSystem)
     Suggested = "<" + Suggested + ">";
   else

Modified: clang-tools-extra/trunk/clangd/Headers.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.h?rev=365005&r1=365004&r2=365005&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Headers.h (original)
+++ clang-tools-extra/trunk/clangd/Headers.h Wed Jul  3 00:47:19 2019
@@ -151,8 +151,12 @@ public:
   /// \param InsertedHeader The preferred header to be inserted. This could be
   /// the same as DeclaringHeader but must be provided.
   ///
+  /// \param IncludingFile is the absolute path of the file that InsertedHeader
+  /// will be inserted.
+  ///
   /// \return A quoted "path" or <path> to be included.
-  std::string calculateIncludePath(const HeaderFile &InsertedHeader) const;
+  std::string calculateIncludePath(const HeaderFile &InsertedHeader,
+                                   llvm::StringRef IncludingFile) 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=365005&r1=365004&r2=365005&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/IncludeFixer.cpp (original)
+++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp Wed Jul  3 00:47:19 2019
@@ -152,7 +152,7 @@ std::vector<Fix> IncludeFixer::fixesForS
     if (!ResolvedInserted)
       return ResolvedInserted.takeError();
     return std::make_pair(
-        Inserter->calculateIncludePath(*ResolvedInserted),
+        Inserter->calculateIncludePath(*ResolvedInserted, File),
         Inserter->shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted));
   };
 
@@ -397,7 +397,6 @@ std::vector<Fix> IncludeFixer::fixUnreso
   return {};
 }
 
-
 llvm::Optional<const SymbolSlab *>
 IncludeFixer::fuzzyFindCached(const FuzzyFindRequest &Req) const {
   auto ReqStr = llvm::formatv("{0}", toJSON(Req)).str();

Modified: clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp?rev=365005&r1=365004&r2=365005&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp Wed Jul  3 00:47:19 2019
@@ -762,11 +762,7 @@ TEST(CompletionTest, DynamicIndexInclude
   // Wait for the dynamic index being built.
   ASSERT_TRUE(Server.blockUntilIdleForTest());
   EXPECT_THAT(completions(Server, "Foo^ foo;").Completions,
-              ElementsAre(AllOf(Named("Foo"),
-                                HasInclude('"' +
-                                           llvm::sys::path::convert_to_slash(
-                                               testPath("foo_header.h")) +
-                                           '"'),
+              ElementsAre(AllOf(Named("Foo"), HasInclude("\"foo_header.h\""),
                                 InsertInclude())));
 }
 

Modified: clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp?rev=365005&r1=365004&r2=365005&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp Wed Jul  3 00:47:19 2019
@@ -97,7 +97,7 @@ protected:
     auto Inserted = ToHeaderFile(Preferred);
     if (!Inserter.shouldInsertInclude(Original, Inserted))
       return "";
-    std::string Path = Inserter.calculateIncludePath(Inserted);
+    std::string Path = Inserter.calculateIncludePath(Inserted, MainFile);
     Action.EndSourceFile();
     return Path;
   }
@@ -180,13 +180,14 @@ TEST_F(HeadersTest, PreambleIncludesPres
   // includes. (We'd test more directly, but it's pretty well encapsulated!)
   auto TU = TestTU::withCode(R"cpp(
     #include "a.h"
+
     #include "a.h"
     void foo();
     #include "a.h"
   )cpp");
   TU.HeaderFilename = "a.h"; // suppress "not found".
   EXPECT_THAT(TU.build().getIncludeStructure().MainFileIncludes,
-              ElementsAre(IncludeLine(1), IncludeLine(2), IncludeLine(4)));
+              ElementsAre(IncludeLine(1), IncludeLine(3), IncludeLine(5)));
 }
 
 TEST_F(HeadersTest, UnResolvedInclusion) {
@@ -211,7 +212,7 @@ TEST_F(HeadersTest, DoNotInsertIfInSameF
   EXPECT_EQ(calculate(MainFile), "");
 }
 
-TEST_F(HeadersTest, ShortenedInclude) {
+TEST_F(HeadersTest, ShortenIncludesInSearchPath) {
   std::string BarHeader = testPath("sub/bar.h");
   EXPECT_EQ(calculate(BarHeader), "\"bar.h\"");
 
@@ -221,10 +222,10 @@ TEST_F(HeadersTest, ShortenedInclude) {
   EXPECT_EQ(calculate(BarHeader), "\"sub/bar.h\"");
 }
 
-TEST_F(HeadersTest, NotShortenedInclude) {
+TEST_F(HeadersTest, ShortenedIncludeNotInSearchPath) {
   std::string BarHeader =
       llvm::sys::path::convert_to_slash(testPath("sub-2/bar.h"));
-  EXPECT_EQ(calculate(BarHeader, ""), "\"" + BarHeader + "\"");
+  EXPECT_EQ(calculate(BarHeader, ""), "\"sub-2/bar.h\"");
 }
 
 TEST_F(HeadersTest, PreferredHeader) {
@@ -267,10 +268,12 @@ TEST(Headers, NoHeaderSearchInfo) {
   auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};
   auto Verbatim = HeaderFile{"<x>", /*Verbatim=*/true};
 
-  EXPECT_EQ(Inserter.calculateIncludePath(Inserting), "\"" + HeaderPath + "\"");
+  // FIXME(kadircet): This should result in "sub/bar.h" instead of full path.
+  EXPECT_EQ(Inserter.calculateIncludePath(Inserting, MainFile),
+            '"' + HeaderPath + '"');
   EXPECT_EQ(Inserter.shouldInsertInclude(HeaderPath, Inserting), false);
 
-  EXPECT_EQ(Inserter.calculateIncludePath(Verbatim), "<x>");
+  EXPECT_EQ(Inserter.calculateIncludePath(Verbatim, MainFile), "<x>");
   EXPECT_EQ(Inserter.shouldInsertInclude(HeaderPath, Verbatim), true);
 }
 




More information about the cfe-commits mailing list