Make sure a module file with errors produced via '-fallow-pcm-with-compiler-errors' can be loaded when using implicit modules

[akyrtzi]

Summary

A module with errors would be marked as out-of-date, then the compilerModule action would produce it, but due to the error it would be treated as failure and the resulting PCM would not get used.

rdar://74087062

[akyrtzi]

clang-format change

[bnbarham]

LGTM

clang/include/clang/Serialization/ASTReader.h

enum LoadFailureCapabilities {
 
     /// The client can handle an AST file that cannot load because it
     /// was built with a different version of Clang.
     ARR_VersionMismatch = 0x4,
 
     /// The client can handle an AST file that cannot load because it's
     /// compiled configuration doesn't match that of the context it was
     /// loaded into.
-    ARR_ConfigurationMismatch = 0x8
+    ARR_ConfigurationMismatch = 0x8,
+
+    /// If a module file is marked with errors treat it as out-of-date so the
+    /// caller can rebuild it.
+    ARR_TreatModuleWithErrorsAsOutOfDate = 0x10
   };
 
   /// Load the AST file designated by the given file name.
   ///
   /// \param FileName The name of the AST file to load.
   ///
   /// \param Type The kind of AST being loaded, e.g., PCH, module, main file,
   /// or preamble.

clang/lib/Frontend/CompilerInstance.cpp

compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
   ImportingInstance.getDiagnostics().Report(ImportLoc,
                                             diag::remark_module_build_done)
     << ModuleName;
 
   // Delete any remaining temporary files related to Instance, in case the
   // module generation thread crashed.
   Instance.clearOutputFiles(/*EraseFiles=*/true);
 
-  return !Instance.getDiagnostics().hasErrorOccurred();
+  // If \p AllowPCMWithCompilerErrors is set return 'success' even if errors
+  // occurred.
+  return !Instance.getDiagnostics().hasErrorOccurred() ||
+         Instance.getFrontendOpts().AllowPCMWithCompilerErrors;
 }
 
 static const FileEntry *getPublicModuleMap(const FileEntry *File,
                                            FileManager &FileMgr) {
   StringRef Filename = llvm::sys::path::filename(File->getName());
   SmallString<128> PublicFilename(File->getDir()->getName());
   if (Filename == "module_private.map")
     llvm::sys::path::append(PublicFilename, "module.map");
if (FrontendTimerGroup)
     Timer.init("loading." + ModuleFilename, "Loading " + ModuleFilename,
                *FrontendTimerGroup);
   llvm::TimeRegion TimeLoading(FrontendTimerGroup ? &Timer : nullptr);
   llvm::TimeTraceScope TimeScope("Module Load", ModuleName);
 
   // Try to load the module file. If we are not trying to load from the
   // module cache, we don't know how to rebuild modules.
   unsigned ARRFlags = Source == MS_ModuleCache
-                          ? ASTReader::ARR_OutOfDate | ASTReader::ARR_Missing
+                          ? ASTReader::ARR_OutOfDate | ASTReader::ARR_Missing |
+                                ASTReader::ARR_TreatModuleWithErrorsAsOutOfDate
                           : Source == MS_PrebuiltModulePath
                                 ? 0
                                 : ASTReader::ARR_ConfigurationMismatch;
   switch (getASTReader()->ReadAST(ModuleFilename,
                                   Source == MS_PrebuiltModulePath
                                       ? serialization::MK_PrebuiltModule
                                       : Source == MS_ModuleBuildPragma
                                             ? serialization::MK_ExplicitModule

clang/lib/Serialization/ASTReader.cpp

case METADATA: {
         if ((ClientLoadCapabilities & ARR_VersionMismatch) == 0)
           Diag(Record[0] < VERSION_MAJOR? diag::err_pch_version_too_old
                                         : diag::err_pch_version_too_new);
         return VersionMismatch;
       }
 
       bool hasErrors = Record[6];
       if (hasErrors && !DisableValidation) {
-        // Always rebuild modules from the cache on an error
-        if (F.Kind == MK_ImplicitModule)
+        // If requested by the caller, mark modules on error as out-of-date.
+        if (F.Kind == MK_ImplicitModule &&
+            (ClientLoadCapabilities & ARR_TreatModuleWithErrorsAsOutOfDate))
           return OutOfDate;
 
         if (!AllowASTWithCompilerErrors) {
           Diag(diag::err_pch_with_compiler_errors);
           return HadErrors;
         }
       }
       if (hasErrors) {

clang/test/Modules/load-module-with-errors.m

+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+// pcherror-error@* {{PCH file contains compiler errors}}
+@import error; // notallowerror-error {{could not build module 'error'}}
+// expected-no-diagnostics
+
+void test(Error *x) {
+  [x method];
+}
+
 // RUN: rm -rf %t
 // RUN: mkdir %t
 // RUN: mkdir %t/prebuilt
 
 // RUN: %clang_cc1 -fmodules -fallow-pcm-with-compiler-errors \
 // RUN:   -fmodule-name=error -o %t/prebuilt/error.pcm \
 // RUN:   -x objective-c -emit-module %S/Inputs/error/module.modulemap
 

 // RUN: %clang_cc1 -fsyntax-only -fmodules -fallow-pcm-with-compiler-errors \
 // RUN:   -fmodules-cache-path=%t -fimplicit-module-maps -I %S/Inputs/error \
 // RUN:   -x objective-c -verify %s
 
 // Should rebuild the cached module if it had an error (if it wasn't rebuilt
 // the verify would fail as it would be the PCH error instead)
 // RUN: %clang_cc1 -fsyntax-only -fmodules \
 // RUN:   -fmodules-cache-path=%t -fimplicit-module-maps -I %S/Inputs/error \
-// RUN:   -x objective-c -verify %s
+// RUN:   -x objective-c -verify=notallowerror %s
 
 // allow-pcm-with-compiler-errors should also allow errors in PCH
 // RUN: %clang_cc1 -fallow-pcm-with-compiler-errors -x objective-c \
 // RUN:   -o %t/check.pch -emit-pch %S/Inputs/error/error.h
 
-// pcherror-error@* {{PCH file contains compiler errors}}
-@import error; // expected-error {{could not build module 'error'}}
-
-void test(Error *x) {
-  [x method];
-}
-
 // CHECK: @interface Error
 // CHECK-NEXT: - (int)method;
 // CHECK-NEXT: - (id)method2;
 // CHECK-NEXT: @end
 // CHECK: void test(Error *x)
+
+// RUN: c-index-test -code-completion-at=%s:9:6 %s -fmodules -fmodules-cache-path=%t \
+// RUN:   -Xclang -fallow-pcm-with-compiler-errors -I %S/Inputs/error | FileCheck -check-prefix=COMPLETE %s
+// COMPLETE: ObjCInstanceMethodDecl:{ResultType int}{TypedText method}
+// COMPLETE: ObjCInstanceMethodDecl:{ResultType id}{TypedText method2}
+
+// RUN: c-index-test -test-load-source local %s -fmodules -fmodules-cache-path=%t \
+// RUN:   -Xclang -fallow-pcm-with-compiler-errors -I %S/Inputs/error | FileCheck -check-prefix=SOURCE %s
+// SOURCE: load-module-with-errors.m:8:6: FunctionDecl=test:8:6 (Definition) Extent=[8:1 - 10:2]
+// SOURCE: load-module-with-errors.m:8:18: ParmDecl=x:8:18 (Definition) Extent=[8:11 - 8:19]
+// SOURCE: load-module-with-errors.m:8:11: ObjCClassRef=Error:3:12 Extent=[8:11 - 8:16]
+// SOURCE: load-module-with-errors.m:8:21: CompoundStmt= Extent=[8:21 - 10:2]
+// SOURCE: load-module-with-errors.m:9:3: ObjCMessageExpr=method:4:8 Extent=[9:3 - 9:13]