Skip to content

[include-cleaner] Pass WorkingDir to suggestPathToFileForDiagnostics #95114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

kadircet
Copy link
Member

Addresses #81215.

@kadircet kadircet requested a review from VitaNuo June 11, 2024 13:51
@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra labels Jun 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 11, 2024

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang

Author: kadir çetinkaya (kadircet)

Changes

Addresses #81215.


Full diff: https://github.com/llvm/llvm-project/pull/95114.diff

4 Files Affected:

  • (modified) clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp (+9-2)
  • (modified) clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp (+18)
  • (modified) clang/include/clang/Testing/TestAST.h (+4)
  • (modified) clang/lib/Testing/TestAST.cpp (+3)
diff --git a/clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp b/clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp
index 2073f0a1d3d87..8332eb685d652 100644
--- a/clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp
+++ b/clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp
@@ -9,6 +9,7 @@
 #include "clang-include-cleaner/IncludeSpeller.h"
 #include "clang-include-cleaner/Types.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Registry.h"
 #include <memory>
@@ -30,8 +31,14 @@ class DefaultIncludeSpeller : public IncludeSpeller {
       return Input.H.verbatim().str();
     case Header::Physical:
       bool IsAngled = false;
+      std::string WorkingDir;
+      if (auto WD = Input.HS.getFileMgr()
+                        .getVirtualFileSystem()
+                        .getCurrentWorkingDirectory())
+        WorkingDir = *WD;
       std::string FinalSpelling = Input.HS.suggestPathToFileForDiagnostics(
-          Input.H.physical(), Input.Main->tryGetRealPathName(), &IsAngled);
+          Input.H.resolvedPath(), WorkingDir, Input.Main->tryGetRealPathName(),
+          &IsAngled);
       return IsAngled ? "<" + FinalSpelling + ">" : "\"" + FinalSpelling + "\"";
     }
     llvm_unreachable("Unknown clang::include_cleaner::Header::Kind enum");
@@ -60,4 +67,4 @@ std::string spellHeader(const IncludeSpeller::Input &Input) {
   return Spelling;
 }
 
-} // namespace clang::include_cleaner
\ No newline at end of file
+} // namespace clang::include_cleaner
diff --git a/clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp b/clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp
index a548868071a12..8f6ad09c46cc4 100644
--- a/clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/IncludeSpellerTest.cpp
@@ -89,6 +89,24 @@ TEST(IncludeSpeller, CanOverrideSystemHeaders) {
                          HS, MainFile}));
 }
 
+TEST(IncludeSpeller, RelativeIncludeSearchPath) {
+  TestInputs Inputs;
+
+  Inputs.WorkingDir = "/root/inner";
+  Inputs.ExtraArgs.push_back("-I..");
+  Inputs.ExtraFiles["/root/foo.h"] = "";
+  TestAST AST{Inputs};
+
+  auto &FM = AST.fileManager();
+  auto &HS = AST.preprocessor().getHeaderSearchInfo();
+  const auto *MainFile = AST.sourceManager().getFileEntryForID(
+      AST.sourceManager().getMainFileID());
+
+  EXPECT_EQ("\"foo.h\"",
+            spellHeader(
+                {Header{*FM.getOptionalFileRef("/root/foo.h")}, HS, MainFile}));
+}
+
 IncludeSpellingStrategy::Add<DummyIncludeSpeller>
     Speller("dummy", "Dummy Include Speller");
 
diff --git a/clang/include/clang/Testing/TestAST.h b/clang/include/clang/Testing/TestAST.h
index 845e31f65438b..8878bfbe16984 100644
--- a/clang/include/clang/Testing/TestAST.h
+++ b/clang/include/clang/Testing/TestAST.h
@@ -49,6 +49,10 @@ struct TestInputs {
   /// Keys are plain filenames ("foo.h"), values are file content.
   llvm::StringMap<std::string> ExtraFiles = {};
 
+  /// Root of execution, all relative paths in Args/Files are resolved against
+  /// this.
+  std::string WorkingDir;
+
   /// Filename to use for translation unit. A default will be used when empty.
   std::string FileName;
 
diff --git a/clang/lib/Testing/TestAST.cpp b/clang/lib/Testing/TestAST.cpp
index 3a50c2d9b5d05..fe8b93851613d 100644
--- a/clang/lib/Testing/TestAST.cpp
+++ b/clang/lib/Testing/TestAST.cpp
@@ -13,6 +13,7 @@
 #include "clang/Frontend/TextDiagnostic.h"
 #include "clang/Testing/CommandLineArgs.h"
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 #include "gtest/gtest.h"
@@ -106,6 +107,8 @@ TestAST::TestAST(const TestInputs &In) {
 
   // Set up a VFS with only the virtual file visible.
   auto VFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+  if (auto Err = VFS->setCurrentWorkingDirectory(In.WorkingDir))
+    ADD_FAILURE() << "Failed to setWD: " << Err.message();
   VFS->addFile(Filename, /*ModificationTime=*/0,
                llvm::MemoryBuffer::getMemBufferCopy(In.Code, Filename));
   for (const auto &Extra : In.ExtraFiles)

@kadircet kadircet merged commit d5297b7 into llvm:main Jun 14, 2024
10 checks passed
@kadircet kadircet deleted the spell_includes_wd branch June 14, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category clang-tools-extra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants