This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb714f73defc8: Frontend: Add 
-f{,no-}implicit-modules-uses-lock and -Rmodule-lock (authored by dexonsmith).

Changed prior to commit:
  https://reviews.llvm.org/D95583?vs=319738&id=366142#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95583

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Frontend/CompilerInstance.cpp
  clang/test/Modules/implicit-modules-use-lock.m

Index: clang/test/Modules/implicit-modules-use-lock.m
===================================================================
--- /dev/null
+++ clang/test/Modules/implicit-modules-use-lock.m
@@ -0,0 +1,23 @@
+// Confirm -fimplicit-modules-use-lock and -fno-implicit-modules-use-lock control
+// whether building a module triggers -Rmodule-lock, indirectly checking whether
+// a lock manager is being used.
+//
+// RUN: rm -rf %t.cache
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \
+// RUN:   -fimplicit-modules-use-lock -Rmodule-lock \
+// RUN:   -fmodules-cache-path=%t.cache -I%S/Inputs/system-out-of-date \
+// RUN:   -fsyntax-only %s -Wnon-modular-include-in-framework-module \
+// RUN:   -Werror=non-modular-include-in-framework-module 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-LOCKS
+//
+// RUN: rm -rf %t.cache
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \
+// RUN:   -fno-implicit-modules-use-lock -Rmodule-lock \
+// RUN:   -fmodules-cache-path=%t.cache -I%S/Inputs/system-out-of-date \
+// RUN:   -fsyntax-only %s -Wnon-modular-include-in-framework-module \
+// RUN:   -Werror=non-modular-include-in-framework-module 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-NO-LOCKS -allow-empty
+
+// CHECK-NO-LOCKS-NOT: remark:
+// CHECK-LOCKS: remark: locking '{{.*}}.pcm' to build module 'X' [-Rmodule-lock]
+@import X;
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1331,6 +1331,9 @@
     SourceLocation ModuleNameLoc, Module *Module, StringRef ModuleFileName) {
   DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics();
 
+  Diags.Report(ModuleNameLoc, diag::remark_module_lock)
+      << ModuleFileName << Module->Name;
+
   // FIXME: have LockFileManager return an error_code so that we can
   // avoid the mkdir when the directory already exists.
   StringRef Dir = llvm::sys::path::parent_path(ModuleFileName);
@@ -1389,6 +1392,25 @@
   }
 }
 
+/// Compile a module in a separate compiler instance and read the AST,
+/// returning true if the module compiles without errors, potentially using a
+/// lock manager to avoid building the same module in multiple compiler
+/// instances.
+static bool compileModuleAndReadAST(CompilerInstance &ImportingInstance,
+                                    SourceLocation ImportLoc,
+                                    SourceLocation ModuleNameLoc,
+                                    Module *Module, StringRef ModuleFileName) {
+  return ImportingInstance.getInvocation()
+                 .getFrontendOpts()
+                 .BuildingImplicitModuleUsesLock
+             ? compileModuleAndReadASTBehindLock(ImportingInstance, ImportLoc,
+                                                 ModuleNameLoc, Module,
+                                                 ModuleFileName)
+             : compileModuleAndReadASTImpl(ImportingInstance, ImportLoc,
+                                           ModuleNameLoc, Module,
+                                           ModuleFileName);
+}
+
 /// Diagnose differences between the current definition of the given
 /// configuration macro and the definition provided on the command line.
 static void checkConfigMacro(Preprocessor &PP, StringRef ConfigMacro,
@@ -1866,8 +1888,8 @@
   }
 
   // Try to compile and then read the AST.
-  if (!compileModuleAndReadASTBehindLock(*this, ImportLoc, ModuleNameLoc, M,
-                                         ModuleFilename)) {
+  if (!compileModuleAndReadAST(*this, ImportLoc, ModuleNameLoc, M,
+                               ModuleFilename)) {
     assert(getDiagnostics().hasErrorOccurred() &&
            "undiagnosed error in compileModuleAndReadAST");
     if (getPreprocessorOpts().FailedModules)
Index: clang/include/clang/Frontend/FrontendOptions.h
===================================================================
--- clang/include/clang/Frontend/FrontendOptions.h
+++ clang/include/clang/Frontend/FrontendOptions.h
@@ -289,6 +289,9 @@
   /// Whether we are performing an implicit module build.
   unsigned BuildingImplicitModule : 1;
 
+  /// Whether to use a filesystem lock when building implicit modules.
+  unsigned BuildingImplicitModuleUsesLock : 1;
+
   /// Whether we should embed all used files into the PCM file.
   unsigned ModulesEmbedAllFiles : 1;
 
@@ -461,9 +464,9 @@
         SkipFunctionBodies(false), UseGlobalModuleIndex(true),
         GenerateGlobalModuleIndex(true), ASTDumpDecls(false),
         ASTDumpLookups(false), BuildingImplicitModule(false),
-        ModulesEmbedAllFiles(false), IncludeTimestamps(true),
-        UseTemporary(true), AllowPCMWithCompilerErrors(false),
-        TimeTraceGranularity(500) {}
+        BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false),
+        IncludeTimestamps(true), UseTemporary(true),
+        AllowPCMWithCompilerErrors(false), TimeTraceGranularity(500) {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file
   /// extension. For example, "c" would return Language::C.
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5301,6 +5301,12 @@
   HelpText<"Embed the contents of all files read by this compilation into "
            "the produced module file.">,
   MarshallingInfoFlag<FrontendOpts<"ModulesEmbedAllFiles">>;
+defm fimplicit_modules_use_lock : BoolOption<"f", "implicit-modules-use-lock",
+  FrontendOpts<"BuildingImplicitModuleUsesLock">, DefaultTrue,
+  NegFlag<SetFalse>,
+  PosFlag<SetTrue, [],
+          "Use filesystem locks for implicit modules builds to avoid "
+          "duplicating work in competing clang invocations.">>;
 // FIXME: We only need this in C++ modules / Modules TS if we might textually
 // enter a different module (eg, when building a header unit).
 def fmodules_local_submodule_visibility :
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -465,6 +465,7 @@
 def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">;
 def MismatchedTags : DiagGroup<"mismatched-tags">;
 def MissingFieldInitializers : DiagGroup<"missing-field-initializers">;
+def ModuleLock : DiagGroup<"module-lock">;
 def ModuleBuild : DiagGroup<"module-build">;
 def ModuleImport : DiagGroup<"module-import">;
 def ModuleConflict : DiagGroup<"module-conflict">;
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -229,6 +229,8 @@
   InGroup<ModuleBuild>;
 def remark_module_build_done : Remark<"finished building module '%0'">,
   InGroup<ModuleBuild>;
+def remark_module_lock : Remark<"locking '%0' to build module '%1'">,
+  InGroup<ModuleLock>;
 def err_modules_embed_file_not_found :
   Error<"file '%0' specified by '-fmodules-embed-file=' not found">,
   DefaultFatal;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D95583: Fr... Duncan P. N. Exon Smith via Phabricator via cfe-commits

Reply via email to