[clang] a2c1054 - [ASTReader] Always rebuild a cached module that has errors

Argyrios Kyrtzidis via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 3 22:14:56 PST 2021


Author: Ben Barham
Date: 2021-02-03T22:06:46-08:00
New Revision: a2c1054c303f20be006e9ef20739dbb88bd9ae02

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

LOG: [ASTReader] Always rebuild a cached module that has errors

A module in the cache with an error should just be a cache miss. If
allowing errors (with -fallow-pcm-with-compiler-errors), a rebuild is
needed so that the appropriate diagnostics are output and in case search
paths have changed. If not allowing errors, the module was built
*allowing* errors and thus should be rebuilt regardless.

Reviewed By: akyrtzi

Differential Revision: https://reviews.llvm.org/D95989

Added: 
    clang/test/Modules/Inputs/error/error.h
    clang/test/Modules/Inputs/error/module.modulemap

Modified: 
    clang/lib/Serialization/ASTReader.cpp
    clang/test/Modules/Inputs/module.map
    clang/test/Modules/load-module-with-errors.m

Removed: 
    clang/test/Modules/Inputs/error.h


################################################################################
diff  --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 85add2e2740b..99579f7956ed 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2750,9 +2750,15 @@ ASTReader::ReadControlBlock(ModuleFile &F,
       }
 
       bool hasErrors = Record[6];
-      if (hasErrors && !DisableValidation && !AllowASTWithCompilerErrors) {
-        Diag(diag::err_pch_with_compiler_errors);
-        return HadErrors;
+      if (hasErrors && !DisableValidation) {
+        // Always rebuild modules from the cache on an error
+        if (F.Kind == MK_ImplicitModule)
+          return OutOfDate;
+
+        if (!AllowASTWithCompilerErrors) {
+          Diag(diag::err_pch_with_compiler_errors);
+          return HadErrors;
+        }
       }
       if (hasErrors) {
         Diags.ErrorOccurred = true;

diff  --git a/clang/test/Modules/Inputs/error.h b/clang/test/Modules/Inputs/error.h
deleted file mode 100644
index 7201003d28c3..000000000000
--- a/clang/test/Modules/Inputs/error.h
+++ /dev/null
@@ -1,8 +0,0 @@
- at import undefined
-
- at interface Error
-- (int)method;
-undefined
- at end
-
-undefined

diff  --git a/clang/test/Modules/Inputs/error/error.h b/clang/test/Modules/Inputs/error/error.h
new file mode 100644
index 000000000000..1b27b21dfd63
--- /dev/null
+++ b/clang/test/Modules/Inputs/error/error.h
@@ -0,0 +1,9 @@
+ at import undefined;
+
+ at interface Error
+- (int)method;
+- (undefined)method2;
+undefined;
+ at end
+
+undefined

diff  --git a/clang/test/Modules/Inputs/error/module.modulemap b/clang/test/Modules/Inputs/error/module.modulemap
new file mode 100644
index 000000000000..e18239af113b
--- /dev/null
+++ b/clang/test/Modules/Inputs/error/module.modulemap
@@ -0,0 +1,3 @@
+module error {
+  header "error.h"
+}

diff  --git a/clang/test/Modules/Inputs/module.map b/clang/test/Modules/Inputs/module.map
index 2aa0733537bc..e7cb4b27bc08 100644
--- a/clang/test/Modules/Inputs/module.map
+++ b/clang/test/Modules/Inputs/module.map
@@ -483,4 +483,3 @@ module template_nontrivial1 {
   header "template-nontrivial1.h"
   export *
 }
-module error { header "error.h" }

diff  --git a/clang/test/Modules/load-module-with-errors.m b/clang/test/Modules/load-module-with-errors.m
index 64575482d091..3dceb545ad9c 100644
--- a/clang/test/Modules/load-module-with-errors.m
+++ b/clang/test/Modules/load-module-with-errors.m
@@ -1,25 +1,68 @@
 // RUN: rm -rf %t
 // RUN: mkdir %t
+// RUN: mkdir %t/prebuilt
 
-// Write out a module with errors make sure it can be read
 // RUN: %clang_cc1 -fmodules -fallow-pcm-with-compiler-errors \
-// RUN:   -fmodules-cache-path=%t -x objective-c -emit-module \
-// RUN:   -fmodule-name=error %S/Inputs/module.map
-// RUN: %clang_cc1 -fmodules -fallow-pcm-with-compiler-errors \
-// RUN:   -fmodules-cache-path=%t -x objective-c -I %S/Inputs \
-// RUN:   -fimplicit-module-maps -ast-print %s | FileCheck %s
+// 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:   -fprebuilt-module-path=%t/prebuilt -fmodules-cache-path=%t \
+// RUN:   -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -fmodules \
+// RUN:   -fprebuilt-module-path=%t/prebuilt -fmodules-cache-path=%t \
+// RUN:   -verify=pcherror %s
+
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fallow-pcm-with-compiler-errors \
+// RUN:   -fmodule-file=error=%t/prebuilt/error.pcm -fmodules-cache-path=%t \
+// RUN:   -ast-print %s | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -fmodules \
+// RUN:   -fmodule-file=error=%t/prebuilt/error.pcm -fmodules-cache-path=%t \
+// RUN:   -verify=pcherror %s
+
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fallow-pcm-with-compiler-errors \
+// RUN:   -fmodule-file=%t/prebuilt/error.pcm -fmodules-cache-path=%t \
+// RUN:   -ast-print %s | FileCheck %s
+// RUN: not %clang_cc1 -fsyntax-only -fmodules \
+// RUN:   -fmodule-file=%t/prebuilt/error.pcm -fmodules-cache-path=%t \
+// RUN:   -verify=pcherror %s
+
+// Shouldn't build the cached module (that has errors) when not allowing errors
+// RUN: not %clang_cc1 -fsyntax-only -fmodules \
+// RUN:   -fmodules-cache-path=%t -fimplicit-module-maps -I %S/Inputs/error \
+// RUN:   -x objective-c %s
+// RUN: find %t -name "error-*.pcm" | not grep error
+
+// Should build the cached module when allowing errors
+// 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
+// RUN: find %t -name "error-*.pcm" | grep error
+
+// Make sure there is still an error after the module is already in the cache
+// 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
 
 // allow-pcm-with-compiler-errors should also allow errors in PCH
-// RUN: %clang_cc1 -fallow-pcm-with-compiler-errors -x c++ -emit-pch \
-// RUN:   -o %t/check.pch %S/Inputs/error.h
+// RUN: %clang_cc1 -fallow-pcm-with-compiler-errors -x objective-c \
+// RUN:   -o %t/check.pch -emit-pch %S/Inputs/error/error.h
 
- at import error;
+// pcherror-error@* {{PCH file contains compiler errors}}
+ at import error; // expected-error {{could not build module 'error'}}
 
-void test(id x) {
+void test(Error *x) {
   [x method];
 }
 
 // CHECK: @interface Error
 // CHECK-NEXT: - (int)method;
+// CHECK-NEXT: - (id)method2;
 // CHECK-NEXT: @end
-// CHECK: void test(id x)
+// CHECK: void test(Error *x)


        


More information about the cfe-commits mailing list