dexonsmith created this revision.
This is a follow-up to r293123 that makes it work with implicit modules.
Some background:
An implicit module build (using Clang's internal build system) uses the same
PCM file location for different `-Werror` levels.
E.g., if a TU has `-Werror=format` and tries to load a PCM built without
`-Werror=format`, a new PCM will be built in its place (and the new PCM should
have the same signature, since r297655). In the other direction, if a TU does
not have `-Werror=format` and tries to load a PCM built with `-Werror=format`,
it should "just work".
The idea is to evolve the PCM toward the strictest -Werror flags that anyone
tries.
---
r293123 started serializing the diagnostic state for each PCM, which broke the
implicit build model.
This commit filters the diagnostic state to match the current compilation's
diagnostic settings.
- Ignore the module's serialized first diagnostic state. Use this TU's state
instead.
- If a pragma warning was upgraded to error/fatal when generating the PCM
(e.g., due to `-Werror` on the command-line), check whether it should still be
upgraded in its current context.
Some feedback I'd like on this approach:
1. The patch-as-is checks whether pragmas should be demoted to warnings for all
AST files, not just implicit modules. I can add a bit of logic to
ReadPragmaDiagnosticMappings that limits it to `F.Kind == MK_ImplicitModule`,
but I'm not sure it's necessary. Maybe it hits when reading PCH files, but no
tests fail, and I'm not sure this is worse... thoughts?
2. If `ReadDiagState` sees a back-reference, it doesn't bother to check whether
pragmas should be demoted; it assumes it should match whatever was done with
the back-reference.
- I think this could be exercised with `-Werror=X` on the command-line and
pragmas modifying -WX (first "ignored", then "error", then "warning"). Perhaps
I should add a FIXME or a comment, but otherwise I think this is okay to miss...
- It could be a back-reference to `CurDiagState`, which current gets
(de)serialized before the pragma mappings. If we instead (de)serialize
`CurDiagState` last, I think this one goes away. Is there a problem with that?
https://reviews.llvm.org/D30954
Files:
clang/include/clang/Basic/Diagnostic.h
clang/include/clang/Basic/DiagnosticIDs.h
clang/lib/Basic/Diagnostic.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/Inputs/implicit-built-Werror-using-W/convert.h
clang/test/Modules/Inputs/implicit-built-Werror-using-W/module.modulemap
clang/test/Modules/implicit-built-Werror-using-W.cpp
Index: clang/test/Modules/implicit-built-Werror-using-W.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-built-Werror-using-W.cpp
@@ -0,0 +1,42 @@
+// Check that implicit modules builds give correct diagnostics, even when
+// reusing a module built with strong -Werror flags.
+//
+// Clear the caches.
+// RUN: rm -rf %t.cache %t-pragma.cache
+//
+// Build with -Werror, then with -W, and then with neither.
+// RUN: not %clang_cc1 -triple x86_64-apple-darwin16 -fsyntax-only -fmodules \
+// RUN: -Werror=shorten-64-to-32 \
+// RUN: -I%S/Inputs/implicit-built-Werror-using-W -fimplicit-module-maps \
+// RUN: -fmodules-cache-path=%t.cache -x c++ %s 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ERROR
+// RUN: %clang_cc1 -triple x86_64-apple-darwin16 -fsyntax-only -fmodules \
+// RUN: -Wshorten-64-to-32 \
+// RUN: -I%S/Inputs/implicit-built-Werror-using-W -fimplicit-module-maps \
+// RUN: -fdisable-module-hash \
+// RUN: -fmodules-cache-path=%t.cache -x c++ %s 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-WARN
+// RUN: %clang_cc1 -triple x86_64-apple-darwin16 -fsyntax-only -fmodules \
+// RUN: -I%S/Inputs/implicit-built-Werror-using-W -fimplicit-module-maps \
+// RUN: -fmodules-cache-path=%t.cache -x c++ %s 2>&1 \
+// RUN: | FileCheck %s -allow-empty
+//
+// In the presence of a warning pragma, build with -Werror and then without.
+// RUN: not %clang_cc1 -triple x86_64-apple-darwin16 -fsyntax-only -fmodules \
+// RUN: -DUSE_PRAGMA -Werror=shorten-64-to-32 \
+// RUN: -I%S/Inputs/implicit-built-Werror-using-W -fimplicit-module-maps \
+// RUN: -fmodules-cache-path=%t-pragma.cache -x c++ %s 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-ERROR
+// RUN: %clang_cc1 -triple x86_64-apple-darwin16 -fsyntax-only -fmodules \
+// RUN: -DUSE_PRAGMA \
+// RUN: -I%S/Inputs/implicit-built-Werror-using-W -fimplicit-module-maps \
+// RUN: -fmodules-cache-path=%t-pragma.cache -x c++ %s 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-WARN
+#include
+
+long long foo() { return convert(0); }
+
+// CHECK-ERROR: error: implicit conversion
+// CHECK-WARN: warning: implicit conversion
+// CHECK-NOT: error: implicit conversion
+//
