Index: cfe/trunk/include/clang/Lex/HeaderSearch.h =================================================================== --- cfe/trunk/include/clang/Lex/HeaderSearch.h +++ cfe/trunk/include/clang/Lex/HeaderSearch.h @@ -709,21 +709,29 @@ /// Suggest a path by which the specified file could be found, for use in /// diagnostics to suggest a #include. Returned path will only contain forward - /// slashes as separators. + /// slashes as separators. MainFile is the absolute path of the file that we + /// are generating the diagnostics for. It will try to shorten the path using + /// MainFile location, if none of the include search directories were prefix + /// of File. /// /// \param IsSystem If non-null, filled in to indicate whether the suggested /// path is relative to a system header directory. std::string suggestPathToFileForDiagnostics(const FileEntry *File, + llvm::StringRef MainFile, bool *IsSystem = nullptr); /// Suggest a path by which the specified file could be found, for use in /// diagnostics to suggest a #include. Returned path will only contain forward - /// slashes as separators. + /// slashes as separators. MainFile is the absolute path of the file that we + /// are generating the diagnostics for. It will try to shorten the path using + /// MainFile location, if none of the include search directories were prefix + /// of File. /// /// \param WorkingDir If non-empty, this will be prepended to search directory /// paths that are relative. std::string suggestPathToFileForDiagnostics(llvm::StringRef File, llvm::StringRef WorkingDir, + llvm::StringRef MainFile, bool *IsSystem = nullptr); void PrintStats(); Index: cfe/trunk/lib/Lex/HeaderSearch.cpp =================================================================== --- cfe/trunk/lib/Lex/HeaderSearch.cpp +++ cfe/trunk/lib/Lex/HeaderSearch.cpp @@ -1665,28 +1665,25 @@ SearchDir.setSearchedAllModuleMaps(true); } -std::string HeaderSearch::suggestPathToFileForDiagnostics(const FileEntry *File, - bool *IsSystem) { +std::string HeaderSearch::suggestPathToFileForDiagnostics( + const FileEntry *File, llvm::StringRef MainFile, bool *IsSystem) { // FIXME: We assume that the path name currently cached in the FileEntry is // the most appropriate one for this analysis (and that it's spelled the // same way as the corresponding header search path). - return suggestPathToFileForDiagnostics(File->getName(), /*BuildDir=*/"", - IsSystem); + return suggestPathToFileForDiagnostics(File->getName(), /*WorkingDir=*/"", + MainFile, IsSystem); } std::string HeaderSearch::suggestPathToFileForDiagnostics( - llvm::StringRef File, llvm::StringRef WorkingDir, bool *IsSystem) { + llvm::StringRef File, llvm::StringRef WorkingDir, llvm::StringRef MainFile, + bool *IsSystem) { using namespace llvm::sys; unsigned BestPrefixLength = 0; - unsigned BestSearchDir; - - for (unsigned I = 0; I != SearchDirs.size(); ++I) { - // FIXME: Support this search within frameworks and header maps. - if (!SearchDirs[I].isNormalDir()) - continue; - - StringRef Dir = SearchDirs[I].getDir()->getName(); + // Checks whether Dir and File shares a common prefix, if they do and that's + // the longest prefix we've seen so for it returns true and updates the + // BestPrefixLength accordingly. + auto CheckDir = [&](llvm::StringRef Dir) -> bool { llvm::SmallString<32> DirPath(Dir.begin(), Dir.end()); if (!WorkingDir.empty() && !path::is_absolute(Dir)) fs::make_absolute(WorkingDir, DirPath); @@ -1710,7 +1707,7 @@ unsigned PrefixLength = NI - path::begin(File); if (PrefixLength > BestPrefixLength) { BestPrefixLength = PrefixLength; - BestSearchDir = I; + return true; } break; } @@ -1723,9 +1720,24 @@ if (*NI != *DI) break; } + return false; + }; + + for (unsigned I = 0; I != SearchDirs.size(); ++I) { + // FIXME: Support this search within frameworks and header maps. + if (!SearchDirs[I].isNormalDir()) + continue; + + StringRef Dir = SearchDirs[I].getDir()->getName(); + if (CheckDir(Dir) && IsSystem) + *IsSystem = BestPrefixLength ? I >= SystemDirIdx : false; } - if (IsSystem) - *IsSystem = BestPrefixLength ? BestSearchDir >= SystemDirIdx : false; + // Try to shorten include path using TUs directory, if we couldn't find any + // suitable prefix in include search paths. + if (!BestPrefixLength && CheckDir(path::parent_path(MainFile)) && IsSystem) + *IsSystem = false; + + return path::convert_to_slash(File.drop_front(BestPrefixLength)); } Index: cfe/trunk/lib/Sema/SemaLookup.cpp =================================================================== --- cfe/trunk/lib/Sema/SemaLookup.cpp +++ cfe/trunk/lib/Sema/SemaLookup.cpp @@ -21,6 +21,7 @@ #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/Basic/Builtins.h" +#include "clang/Basic/FileManager.h" #include "clang/Basic/LangOptions.h" #include "clang/Lex/HeaderSearch.h" #include "clang/Lex/ModuleLoader.h" @@ -5195,10 +5196,11 @@ /// Get a "quoted.h" or include path to use in a diagnostic /// suggesting the addition of a #include of the specified file. static std::string getIncludeStringForHeader(Preprocessor &PP, - const FileEntry *E) { - bool IsSystem; - auto Path = - PP.getHeaderSearchInfo().suggestPathToFileForDiagnostics(E, &IsSystem); + const FileEntry *E, + llvm::StringRef IncludingFile) { + bool IsSystem = false; + auto Path = PP.getHeaderSearchInfo().suggestPathToFileForDiagnostics( + E, IncludingFile, &IsSystem); return (IsSystem ? '<' : '"') + Path + (IsSystem ? '>' : '"'); } @@ -5240,6 +5242,11 @@ UniqueModules.push_back(M); } + llvm::StringRef IncludingFile; + if (const FileEntry *FE = + SourceMgr.getFileEntryForID(SourceMgr.getFileID(UseLoc))) + IncludingFile = FE->tryGetRealPathName(); + if (UniqueModules.empty()) { // All candidates were global module fragments. Try to suggest a #include. const FileEntry *E = @@ -5248,7 +5255,7 @@ // a FixItHint there. Diag(UseLoc, diag::err_module_unimported_use_global_module_fragment) << (int)MIK << Decl << !!E - << (E ? getIncludeStringForHeader(PP, E) : ""); + << (E ? getIncludeStringForHeader(PP, E, IncludingFile) : ""); // Produce a "previous" note if it will point to a header rather than some // random global module fragment. // FIXME: Suppress the note backtrace even under @@ -5284,8 +5291,8 @@ // FIXME: Find a smart place to suggest inserting a #include, and add // a FixItHint there. Diag(UseLoc, diag::err_module_unimported_use_header) - << (int)MIK << Decl << Modules[0]->getFullModuleName() - << getIncludeStringForHeader(PP, E); + << (int)MIK << Decl << Modules[0]->getFullModuleName() + << getIncludeStringForHeader(PP, E, IncludingFile); } else { // FIXME: Add a FixItHint that imports the corresponding module. Diag(UseLoc, diag::err_module_unimported_use) Index: cfe/trunk/unittests/Lex/HeaderSearchTest.cpp =================================================================== --- cfe/trunk/unittests/Lex/HeaderSearchTest.cpp +++ cfe/trunk/unittests/Lex/HeaderSearchTest.cpp @@ -59,35 +59,41 @@ TEST_F(HeaderSearchTest, NoSearchDir) { EXPECT_EQ(Search.search_dir_size(), 0u); - EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/y/z", /*WorkingDir=*/""), + EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/y/z", /*WorkingDir=*/"", + /*MainFile=*/""), "/x/y/z"); } TEST_F(HeaderSearchTest, SimpleShorten) { addSearchDir("/x"); addSearchDir("/x/y"); - EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/y/z", /*WorkingDir=*/""), + EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/y/z", /*WorkingDir=*/"", + /*MainFile=*/""), "z"); addSearchDir("/a/b/"); - EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/a/b/c", /*WorkingDir=*/""), + EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/a/b/c", /*WorkingDir=*/"", + /*MainFile=*/""), "c"); } TEST_F(HeaderSearchTest, ShortenWithWorkingDir) { addSearchDir("x/y"); EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/a/b/c/x/y/z", - /*WorkingDir=*/"/a/b/c"), + /*WorkingDir=*/"/a/b/c", + /*MainFile=*/""), "z"); } TEST_F(HeaderSearchTest, Dots) { addSearchDir("/x/./y/"); EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/y/./z", - /*WorkingDir=*/""), + /*WorkingDir=*/"", + /*MainFile=*/""), "z"); addSearchDir("a/.././c/"); EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/m/n/./c/z", - /*WorkingDir=*/"/m/n/"), + /*WorkingDir=*/"/m/n/", + /*MainFile=*/""), "z"); } @@ -95,14 +101,16 @@ TEST_F(HeaderSearchTest, BackSlash) { addSearchDir("C:\\x\\y\\"); EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t", - /*WorkingDir=*/""), + /*WorkingDir=*/"", + /*MainFile=*/""), "z/t"); } TEST_F(HeaderSearchTest, BackSlashWithDotDot) { addSearchDir("..\\y"); EXPECT_EQ(Search.suggestPathToFileForDiagnostics("C:\\x\\y\\z\\t", - /*WorkingDir=*/"C:/x/y/"), + /*WorkingDir=*/"C:/x/y/", + /*MainFile=*/""), "z/t"); } #endif @@ -110,9 +118,23 @@ TEST_F(HeaderSearchTest, DotDotsWithAbsPath) { addSearchDir("/x/../y/"); EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z", - /*WorkingDir=*/""), + /*WorkingDir=*/"", + /*MainFile=*/""), "z"); } +TEST_F(HeaderSearchTest, IncludeFromSameDirectory) { + EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z/t.h", + /*WorkingDir=*/"", + /*MainFile=*/"/y/a.cc"), + "z/t.h"); + + addSearchDir("/"); + EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z/t.h", + /*WorkingDir=*/"", + /*MainFile=*/"/y/a.cc"), + "y/z/t.h"); +} + } // namespace } // namespace clang Index: clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp =================================================================== --- clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp +++ clang-tools-extra/trunk/clang-include-fixer/IncludeFixer.cpp @@ -314,9 +314,9 @@ if (!Entry) return Include; - bool IsSystem; + bool IsSystem = false; std::string Suggestion = - HeaderSearch.suggestPathToFileForDiagnostics(Entry, &IsSystem); + HeaderSearch.suggestPathToFileForDiagnostics(Entry, "", &IsSystem); return IsSystem ? '<' + Suggestion + '>' : '"' + Suggestion + '"'; } Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp @@ -328,7 +328,7 @@ 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(); Index: clang-tools-extra/trunk/clangd/Headers.h =================================================================== --- clang-tools-extra/trunk/clangd/Headers.h +++ clang-tools-extra/trunk/clangd/Headers.h @@ -151,8 +151,12 @@ /// \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 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. Index: clang-tools-extra/trunk/clangd/Headers.cpp =================================================================== --- clang-tools-extra/trunk/clangd/Headers.cpp +++ clang-tools-extra/trunk/clangd/Headers.cpp @@ -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 @@ } 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 Index: clang-tools-extra/trunk/clangd/IncludeFixer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/IncludeFixer.cpp +++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp @@ -152,7 +152,7 @@ if (!ResolvedInserted) return ResolvedInserted.takeError(); return std::make_pair( - Inserter->calculateIncludePath(*ResolvedInserted), + Inserter->calculateIncludePath(*ResolvedInserted, File), Inserter->shouldInsertInclude(*ResolvedDeclaring, *ResolvedInserted)); }; @@ -397,7 +397,6 @@ return {}; } - llvm::Optional IncludeFixer::fuzzyFindCached(const FuzzyFindRequest &Req) const { auto ReqStr = llvm::formatv("{0}", toJSON(Req)).str(); Index: clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp =================================================================== --- clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp @@ -762,11 +762,7 @@ // 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()))); } Index: clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp =================================================================== --- clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp +++ clang-tools-extra/trunk/clangd/unittests/HeadersTests.cpp @@ -97,7 +97,7 @@ 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 @@ // 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 @@ 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 @@ 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 @@ auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false}; auto Verbatim = HeaderFile{"", /*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), ""); + EXPECT_EQ(Inserter.calculateIncludePath(Verbatim, MainFile), ""); EXPECT_EQ(Inserter.shouldInsertInclude(HeaderPath, Verbatim), true); }