[clang] 7c97c57 - [Modules] Recreate file manager for ftime-trace when compiling a module

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-02-06T17:17:09+08:00
New Revision: 7c97c574cc795705737cd0b73daf6867406fe624

URL: https://github.com/llvm/llvm-project/commit/7c97c574cc795705737cd0b73daf6867406fe624
DIFF: https://github.com/llvm/llvm-project/commit/7c97c574cc795705737cd0b73daf6867406fe624.diff

LOG: [Modules] Recreate file manager for ftime-trace when compiling a module

Close https://github.com/llvm/llvm-project/issues/60544.

The root cause for the issue is that when we compile a module unit, the
file manager (and proprocessor and source manager) are owned by AST
instead of the compilaton instance. So the file manager may be invalid
when we want to create a time-report file for -ftime-trace when we are
compiling a module unit.

This patch tries to recreate the file manager for -ftime-trace if we
find the file manager is not valid.

Added: 
    clang/test/Modules/ftime-trace.cppm

Modified: 
    clang/lib/Frontend/CompilerInstance.cpp
    clang/lib/Frontend/FrontendAction.cpp
    clang/tools/driver/cc1_main.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index ecc1c4cf51c18..9c79e856dad4f 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -851,6 +851,9 @@ CompilerInstance::createOutputFileImpl(StringRef OutputPath, bool Binary,
   // relative to that.
   std::optional<SmallString<128>> AbsPath;
   if (OutputPath != "-" && !llvm::sys::path::is_absolute(OutputPath)) {
+    assert(hasFileManager() &&
+           "File Manager is required to fix up relative path.\n");
+
     AbsPath.emplace(OutputPath);
     FileMgr->FixupRelativePath(*AbsPath);
     OutputPath = *AbsPath;

diff  --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index 1e276642016d1..3551967106c5a 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -1117,6 +1117,9 @@ void FrontendAction::EndSourceFile() {
   // FrontendAction.
   CI.clearOutputFiles(/*EraseFiles=*/shouldEraseOutputFiles());
 
+  // The resources are owned by AST when the current file is AST.
+  // So we reset the resources here to avoid users accessing it
+  // accidently.
   if (isCurrentFileAST()) {
     if (DisableFree) {
       CI.resetAndLeakPreprocessor();

diff  --git a/clang/test/Modules/ftime-trace.cppm b/clang/test/Modules/ftime-trace.cppm
new file mode 100644
index 0000000000000..48cd4113ec782
--- /dev/null
+++ b/clang/test/Modules/ftime-trace.cppm
@@ -0,0 +1,13 @@
+// Tests that we can use modules and ftime-trace together.
+// Address https://github.com/llvm/llvm-project/issues/60544
+//
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/a.pcm -ftime-trace=%t/a.json -o -
+// RUN: ls %t | grep "a.json"
+
+//--- a.cppm
+export module a;

diff  --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp
index c79306b6f7d59..cb33692cd2623 100644
--- a/clang/tools/driver/cc1_main.cpp
+++ b/clang/tools/driver/cc1_main.cpp
@@ -266,6 +266,19 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
         llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
       Path.assign(TracePath);
     }
+
+    // It is possible that the compiler instance doesn't own a file manager here
+    // if we're compiling a module unit. Since the file manager are owned by AST
+    // when we're compiling a module unit. So the file manager may be invalid
+    // here.
+    //
+    // It should be fine to create file manager here since the file system
+    // options are stored in the compiler invocation and we can recreate the VFS
+    // from the compiler invocation.
+    if (!Clang->hasFileManager())
+      Clang->createFileManager(createVFSFromCompilerInvocation(
+          Clang->getInvocation(), Clang->getDiagnostics()));
+
     if (auto profilerOutput = Clang->createOutputFile(
             Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
             /*useTemporary=*/false)) {