[PATCH] D154134: [clang] Fix ASTUnit working directory handling

2023-06-30 Thread Ben Barham via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG62e4c22c95a9: [clang] Fix ASTUnit working directory handling 
(authored by hamishknight, committed by bnbarham).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154134/new/

https://reviews.llvm.org/D154134

Files:
  clang/lib/Frontend/ASTUnit.cpp
  clang/unittests/Frontend/ASTUnitTest.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/ReparseWorkingDirTest.cpp

Index: clang/unittests/Frontend/ReparseWorkingDirTest.cpp
===
--- /dev/null
+++ clang/unittests/Frontend/ReparseWorkingDirTest.cpp
@@ -0,0 +1,118 @@
+//-- unittests/Frontend/ReparseWorkingDirTest.cpp - FrontendAction tests =//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/FrontendOptions.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+class ReparseWorkingDirTest : public ::testing::Test {
+  IntrusiveRefCntPtr VFS;
+  std::shared_ptr PCHContainerOpts;
+
+public:
+  void SetUp() override { VFS = new vfs::InMemoryFileSystem(); }
+  void TearDown() override {}
+
+  void setWorkingDirectory(StringRef Path) {
+VFS->setCurrentWorkingDirectory(Path);
+  }
+
+  void AddFile(const std::string , const std::string ) {
+::time_t now;
+::time();
+VFS->addFile(Filename, now,
+ MemoryBuffer::getMemBufferCopy(Contents, Filename));
+  }
+
+  std::unique_ptr ParseAST(StringRef EntryFile) {
+PCHContainerOpts = std::make_shared();
+auto CI = std::make_shared();
+CI->getFrontendOpts().Inputs.push_back(FrontendInputFile(
+EntryFile, FrontendOptions::getInputKindForExtension(
+   llvm::sys::path::extension(EntryFile).substr(1;
+
+CI->getHeaderSearchOpts().AddPath("headers",
+  frontend::IncludeDirGroup::Quoted,
+  /*isFramework*/ false,
+  /*IgnoreSysRoot*/ false);
+
+CI->getFileSystemOpts().WorkingDir = *VFS->getCurrentWorkingDirectory();
+CI->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+
+IntrusiveRefCntPtr Diags(
+CompilerInstance::createDiagnostics(new DiagnosticOptions,
+new DiagnosticConsumer));
+
+FileManager *FileMgr = new FileManager(CI->getFileSystemOpts(), VFS);
+
+std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation(
+CI, PCHContainerOpts, Diags, FileMgr, false, CaptureDiagsKind::None,
+/*PrecompilePreambleAfterNParses=*/1);
+return AST;
+  }
+
+  bool ReparseAST(const std::unique_ptr ) {
+bool reparseFailed =
+AST->Reparse(PCHContainerOpts, /*RemappedFiles*/ {}, VFS);
+return !reparseFailed;
+  }
+};
+
+TEST_F(ReparseWorkingDirTest, ReparseWorkingDir) {
+  // Setup the working directory path.
+  SmallString<16> WorkingDir;
+#ifdef _WIN32
+  WorkingDir = "C:\\";
+#else
+  WorkingDir = "/";
+#endif
+  llvm::sys::path::append(WorkingDir, "root");
+  setWorkingDirectory(WorkingDir);
+
+  SmallString<32> Header;
+  llvm::sys::path::append(Header, WorkingDir, "headers", "header.h");
+
+  SmallString<32> MainName;
+  llvm::sys::path::append(MainName, WorkingDir, "main.cpp");
+
+  AddFile(MainName.str().str(), R"cpp(
+#include "header.h"
+int main() { return foo(); }
+)cpp");
+  AddFile(Header.str().str(), R"h(
+static int foo() { return 0; }
+)h");
+
+  // Parse the main file, ensuring we can include the header.
+  std::unique_ptr AST(ParseAST(MainName.str()));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Reparse and check that the working directory was preserved.
+  ASSERT_TRUE(ReparseAST(AST));
+
+  const auto  = AST->getFileManager();
+  const auto  = FM.getVirtualFileSystem();
+  ASSERT_EQ(FM.getFileSystemOpts().WorkingDir, WorkingDir);
+  ASSERT_EQ(*FS.getCurrentWorkingDirectory(), WorkingDir);
+}
+
+} // end anonymous namespace
Index: clang/unittests/Frontend/CMakeLists.txt
===
--- clang/unittests/Frontend/CMakeLists.txt
+++ 

[PATCH] D154134: [clang] Fix ASTUnit working directory handling

2023-06-30 Thread Hamish Knight via Phabricator via cfe-commits
hamishknight updated this revision to Diff 536163.
hamishknight added a comment.

Updated ReparseWorkingDirTest to fix Windows CI


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154134/new/

https://reviews.llvm.org/D154134

Files:
  clang/lib/Frontend/ASTUnit.cpp
  clang/unittests/Frontend/ASTUnitTest.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/ReparseWorkingDirTest.cpp

Index: clang/unittests/Frontend/ReparseWorkingDirTest.cpp
===
--- /dev/null
+++ clang/unittests/Frontend/ReparseWorkingDirTest.cpp
@@ -0,0 +1,118 @@
+//-- unittests/Frontend/ReparseWorkingDirTest.cpp - FrontendAction tests =//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/FrontendOptions.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+class ReparseWorkingDirTest : public ::testing::Test {
+  IntrusiveRefCntPtr VFS;
+  std::shared_ptr PCHContainerOpts;
+
+public:
+  void SetUp() override { VFS = new vfs::InMemoryFileSystem(); }
+  void TearDown() override {}
+
+  void setWorkingDirectory(StringRef Path) {
+VFS->setCurrentWorkingDirectory(Path);
+  }
+
+  void AddFile(const std::string , const std::string ) {
+::time_t now;
+::time();
+VFS->addFile(Filename, now,
+ MemoryBuffer::getMemBufferCopy(Contents, Filename));
+  }
+
+  std::unique_ptr ParseAST(StringRef EntryFile) {
+PCHContainerOpts = std::make_shared();
+auto CI = std::make_shared();
+CI->getFrontendOpts().Inputs.push_back(FrontendInputFile(
+EntryFile, FrontendOptions::getInputKindForExtension(
+   llvm::sys::path::extension(EntryFile).substr(1;
+
+CI->getHeaderSearchOpts().AddPath("headers",
+  frontend::IncludeDirGroup::Quoted,
+  /*isFramework*/ false,
+  /*IgnoreSysRoot*/ false);
+
+CI->getFileSystemOpts().WorkingDir = *VFS->getCurrentWorkingDirectory();
+CI->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+
+IntrusiveRefCntPtr Diags(
+CompilerInstance::createDiagnostics(new DiagnosticOptions,
+new DiagnosticConsumer));
+
+FileManager *FileMgr = new FileManager(CI->getFileSystemOpts(), VFS);
+
+std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation(
+CI, PCHContainerOpts, Diags, FileMgr, false, CaptureDiagsKind::None,
+/*PrecompilePreambleAfterNParses=*/1);
+return AST;
+  }
+
+  bool ReparseAST(const std::unique_ptr ) {
+bool reparseFailed =
+AST->Reparse(PCHContainerOpts, /*RemappedFiles*/ {}, VFS);
+return !reparseFailed;
+  }
+};
+
+TEST_F(ReparseWorkingDirTest, ReparseWorkingDir) {
+  // Setup the working directory path.
+  SmallString<16> WorkingDir;
+#ifdef _WIN32
+  WorkingDir = "C:\\";
+#else
+  WorkingDir = "/";
+#endif
+  llvm::sys::path::append(WorkingDir, "root");
+  setWorkingDirectory(WorkingDir);
+
+  SmallString<32> Header;
+  llvm::sys::path::append(Header, WorkingDir, "headers", "header.h");
+
+  SmallString<32> MainName;
+  llvm::sys::path::append(MainName, WorkingDir, "main.cpp");
+
+  AddFile(MainName.str().str(), R"cpp(
+#include "header.h"
+int main() { return foo(); }
+)cpp");
+  AddFile(Header.str().str(), R"h(
+static int foo() { return 0; }
+)h");
+
+  // Parse the main file, ensuring we can include the header.
+  std::unique_ptr AST(ParseAST(MainName.str()));
+  ASSERT_TRUE(AST.get());
+  ASSERT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+
+  // Reparse and check that the working directory was preserved.
+  ASSERT_TRUE(ReparseAST(AST));
+
+  const auto  = AST->getFileManager();
+  const auto  = FM.getVirtualFileSystem();
+  ASSERT_EQ(FM.getFileSystemOpts().WorkingDir, WorkingDir);
+  ASSERT_EQ(*FS.getCurrentWorkingDirectory(), WorkingDir);
+}
+
+} // end anonymous namespace
Index: clang/unittests/Frontend/CMakeLists.txt
===
--- clang/unittests/Frontend/CMakeLists.txt
+++ clang/unittests/Frontend/CMakeLists.txt
@@ -12,6 +12,7 @@
   CodeGenActionTest.cpp
   ParsedSourceLocationTest.cpp
   PCHPreambleTest.cpp
+  ReparseWorkingDirTest.cpp
   OutputStreamTest.cpp
   

[PATCH] D154134: [clang] Fix ASTUnit working directory handling

2023-06-29 Thread Hamish Knight via Phabricator via cfe-commits
hamishknight created this revision.
hamishknight added reviewers: bnbarham, benlangmuir.
hamishknight added a project: clang.
Herald added a project: All.
hamishknight requested review of this revision.
Herald added a subscriber: cfe-commits.

Fix a couple of issues with the handling of the current working directory in 
ASTUnit:

- Use `createPhysicalFileSystem` instead of `getRealFileSystem` to avoid 
affecting the process' current working directory, and set it at the top of 
`ASTUnit::LoadFromCommandLine` such that the driver used for argument parsing 
and the ASTUnit share the same VFS. This ensures that '-working-directory' 
correctly sets the VFS working directory in addition to the FileManager working 
directory.
- Ensure we preserve the FileSystemOptions set on the FileManager when 
re-creating it (as `ASTUnit::Reparse` will clear the currently set FileManager).

rdar://110697657


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154134

Files:
  clang/lib/Frontend/ASTUnit.cpp
  clang/unittests/Frontend/ASTUnitTest.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/ReparseWorkingDirTest.cpp

Index: clang/unittests/Frontend/ReparseWorkingDirTest.cpp
===
--- /dev/null
+++ clang/unittests/Frontend/ReparseWorkingDirTest.cpp
@@ -0,0 +1,116 @@
+//-- unittests/Frontend/ReparseWorkingDirTest.cpp - FrontendAction tests =//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/FrontendOptions.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+class ReparseWorkingDirTest : public ::testing::Test {
+  IntrusiveRefCntPtr VFS;
+  std::shared_ptr PCHContainerOpts;
+
+public:
+  void SetUp() override { VFS = new vfs::InMemoryFileSystem(); }
+  void TearDown() override {}
+
+  void setWorkingDirectory(StringRef Path) {
+VFS->setCurrentWorkingDirectory(Path);
+  }
+
+  void AddFile(const std::string , const std::string ) {
+::time_t now;
+::time();
+VFS->addFile(Filename, now,
+ MemoryBuffer::getMemBufferCopy(Contents, Filename));
+  }
+
+  std::unique_ptr ParseAST(StringRef EntryFile) {
+PCHContainerOpts = std::make_shared();
+auto CI = std::make_shared();
+CI->getFrontendOpts().Inputs.push_back(FrontendInputFile(
+EntryFile, FrontendOptions::getInputKindForExtension(
+   llvm::sys::path::extension(EntryFile).substr(1;
+
+CI->getHeaderSearchOpts().AddPath("headers",
+  frontend::IncludeDirGroup::Quoted,
+  /*isFramework*/ false,
+  /*IgnoreSysRoot*/ false);
+
+CI->getFileSystemOpts().WorkingDir = *VFS->getCurrentWorkingDirectory();
+CI->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+
+IntrusiveRefCntPtr Diags(
+CompilerInstance::createDiagnostics(new DiagnosticOptions,
+new DiagnosticConsumer));
+
+FileManager *FileMgr = new FileManager(CI->getFileSystemOpts(), VFS);
+
+std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation(
+CI, PCHContainerOpts, Diags, FileMgr, false, CaptureDiagsKind::None,
+/*PrecompilePreambleAfterNParses=*/1);
+return AST;
+  }
+
+  bool ReparseAST(const std::unique_ptr ) {
+bool reparseFailed =
+AST->Reparse(PCHContainerOpts, /*RemappedFiles*/ {}, VFS);
+return !reparseFailed;
+  }
+};
+
+TEST_F(ReparseWorkingDirTest, ReparseWorkingDir) {
+  // Setup the working directory path. We use '//root/' to allow the path to be
+  // valid on both Windows and Unix. We need the trailing slash for the path
+  // to be treated as absolute.
+  SmallString<16> WorkingDir;
+  llvm::sys::path::append(WorkingDir, "//root",
+  llvm::sys::path::get_separator());
+  setWorkingDirectory(WorkingDir);
+
+  SmallString<32> Header;
+  llvm::sys::path::append(Header, WorkingDir, "headers", "header.h");
+
+  SmallString<32> MainName;
+  llvm::sys::path::append(MainName, WorkingDir, "main.cpp");
+
+  AddFile(MainName.str().str(), R"cpp(
+#include "header.h"
+int main() { return foo(); }
+)cpp");
+  AddFile(Header.str().str(), R"h(
+static int foo() { return 0; }
+)h");
+
+  // Parse the main