[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?



-------------- 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