[PATCH] D19727: [MS] Improved implementation #pragma pack (MS pragmas, part 2)

[Reid Kleckner via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


================
Comment at: include/clang/Sema/Sema.h:354-357
@@ -356,6 +353,6 @@
     };
     void Act(SourceLocation PragmaLocation,
              PragmaMsStackAction Action,
              llvm::StringRef StackSlotLabel,
              ValueType Value);
 
----------------
Defining a template member function outside of its header is asking for trouble. This is a pre-existing issue that probably shouldn't be dealt with as part of this change, but eventually this needs to move back to the header.

================
Comment at: lib/Sema/SemaAttr.cpp:277-278
@@ -276,4 @@
-      // empty.
-      Diag(PragmaLoc, diag::warn_pragma_pop_failed)
-          << "pack" << (Name ? "no record matching name" : "stack empty");
-
----------------
d.zobnin.bugzilla wrote:
> Here I removed the "no record matching name" diagnostics, which wasn't covered by any test.
> I'm going to rework and restore it in future patch.
Sure, once the pragma stack has support for popping by name.


http://reviews.llvm.org/D19727