[PATCH] D154016: [clang][modules] Avoid serializing all diag mappings in non-deterministic order

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir marked an inline comment as done.
benlangmuir added inline comments.


================
Comment at: clang/lib/Serialization/ASTWriter.cpp:3016
       for (const auto &I : *State) {
-        if (I.second.isPragma() || IncludeNonPragmaStates) {
-          Record.push_back(I.first);
-          Record.push_back(I.second.serialize());
-        }
+        // Maybe skip non-pragmas.
+        if (!I.second.isPragma() && !IncludeNonPragmaStates)
----------------
steven_wu wrote:
> Is pragma in this context refer to `#pragma diagnostics push/pop`? Do we have test to cover those to be deterministic?
> Is pragma in this context refer to `#pragma diagnostics push/pop`?

Yeah, this is diagnostic pragmas.

> Do we have test to cover those to be deterministic?

I extended the current test to include some pragmas as well.  It's harder to check them exhaustively because the encoded format gets more complex, but I checked the specific mappings are in order and the `diff` part of the test should help catch any other non-determinism there could be.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154016/new/

https://reviews.llvm.org/D154016