Modules: Do not serialize #pragma pack state

The modules side of r299226, which serializes #pragma pack state,
doesn't work well.

The main purpose was to make -include and -include-pch match semantics
(the PCH side).  We also started serializing #pragma pack in PCMs, in
the hopes of making modules and non-modules builds more consistent.  But
consider:

    $ cat a.h
    $ cat b.h
    #pragma pack(push, 2)
    $ cat module.modulemap
    module M {
        module a { header "a.h" }
        module b { header "b.h" }
    }
    $ cat t.cpp
    #include "a.h"
    #pragma pack(show)

As of r299226, the #pragma pack(show) gives "2", even though we've only
included "a.h".

- With -fmodules-local-submodule-visibility, this is clearly wrong.  We
  should get the default state (8 on x86_64).

- Without -fmodules-local-submodule-visibility, this kind of matches how
  other things work (as if include-the-whole-module), but it's still
  really terrible, and it doesn't actually make modules and non-modules
  builds more consistent.

This commit disables the serialization for modules, essentially a
partial revert of r299226.

Going forward:

 1. Having this #pragma pack stuff escape is terrible design (or, more
    often, a horrible bug).  We should prioritize adding warnings (maybe
    -Werror by default?).

 2. If we eventually reintroduce this for modules, it should only apply
    to -fmodules-local-submodule-visibility, and it should be tracked on
    a per-submodule basis.

llvm-svn: 300380

1 files changed, 5 insertions, 0 deletions

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index d52c4fd7706..bb869077f4c 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4186,6 +4186,11 @@ void ASTWriter::WriteMSPointersToMembersPragmaOptions(Sema &SemaRef) {
 
 /// \brief Write the state of 'pragma pack' at the end of the module.
 void ASTWriter::WritePackPragmaOptions(Sema &SemaRef) {
+  // Don't serialize pragma pack state for modules, since it should only take
+  // effect on a per-submodule basis.
+  if (WritingModule)
+    return;
+
   RecordData Record;
   Record.push_back(SemaRef.PackStack.CurrentValue);
   AddSourceLocation(SemaRef.PackStack.CurrentPragmaLocation, Record);