[PATCH] D30954: Modules: Simulate diagnostic settings for implicit modules

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 14 14:57:39 PDT 2017


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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30954.91773.patch
Type: text/x-patch
Size: 9783 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170314/470b0273/attachment-0001.bin>


More information about the cfe-commits mailing list