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

[• d.zobnin.bugzilla]

Summary

An attempt to make implementation of #pragma pack consistent with other "stack" pragmas. Also modifies #pragma options align, since it shares the stack with #pragma pack.
It is the first item from todo list in http://reviews.llvm.org/D19361

The patch doesn't change compiler's behavior, so the tests are not modified.
I'm planning to rework the pragmas' diagnostics in next patch -- it should be a common feature of all "stack" pragmas.

[• d.zobnin.bugzilla]

lib/Sema/SemaAttr.cpp

277–278 ↗

(On Diff #55598)

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.

[rnk]

lgtm

include/clang/Sema/Sema.h

354–357 ↗

(On Diff #55598)

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.

lib/Sema/SemaAttr.cpp

277–278 ↗

(On Diff #55598)

Sure, once the pragma stack has support for popping by name.

[aaboud]

cfe/trunk/lib/Sema/SemaAttr.cpp

110

You forgot to initialize "Alignment" in this case.
This is causing a build failure:
http://lab.llvm.org:8011/builders/sanitizer-ppc64be-linux/builds/1758

cfe/trunk/include/clang/Sema/Sema.h

public:
   /// \brief Generally null except when we temporarily switch decl contexts,
   /// like in \see ActOnObjCTemporaryExitContainerContext.
   DeclContext *OriginalLexicalContext;
 
   /// VAListTagName - The declaration name corresponding to __va_list_tag.
   /// This is used as part of a hack to omit that class from ADL results.
   DeclarationName VAListTagName;
 
-  /// PackContext - Manages the stack for \#pragma pack. An alignment
-  /// of 0 indicates default alignment.
-  void *PackContext; // Really a "PragmaPackStack*"
-
   bool MSStructPragmaOn; // True when \#pragma ms_struct on
 
   /// \brief Controls member pointer representation format under the MS ABI.
   LangOptions::PragmaMSPointersToMembersKind
       MSPointerToMemberRepresentationMethod;
 
   /// Stack of active SEH __finally scopes.  Can be empty.
   SmallVector<Scope*, 2> CurrentSEHFinally;
 
   /// \brief Source location for newly created implicit MSInheritanceAttrs
   SourceLocation ImplicitMSInheritanceAttrLoc;
 
   enum PragmaMsStackAction {
     PSK_Reset     = 0x0,                // #pragma ()
     PSK_Set       = 0x1,                // #pragma (value)
     PSK_Push      = 0x2,                // #pragma (push[, id])
     PSK_Pop       = 0x4,                // #pragma (pop[, id])
+    PSK_Show      = 0x8,                // #pragma (show) -- only for "pack"!
     PSK_Push_Set  = PSK_Push | PSK_Set, // #pragma (push[, id], value)
     PSK_Pop_Set   = PSK_Pop | PSK_Set,  // #pragma (pop[, id], value)
   };
 
   template<typename ValueType>
   struct PragmaStack {
     struct Slot {
       llvm::StringRef StackSlotLabel;
public:
   /// C++ ABI.  Possible values are 0, 1, and 2, which mean:
   ///
   /// 0: Suppress all vtordisps
   /// 1: Insert vtordisps in the presence of vbase overrides and non-trivial
   ///    structors
   /// 2: Always insert vtordisps to support RTTI on partially constructed
   ///    objects
   PragmaStack<MSVtorDispAttr::Mode> VtorDispStack;
+  // #pragma pack.
+  // Sentinel to represent when the stack is set to mac68k alignment.
+  static const unsigned kMac68kAlignmentSentinel = ~0U;
+  PragmaStack<unsigned> PackStack;
+  // Segment #pragmas.
   PragmaStack<StringLiteral *> DataSegStack;
   PragmaStack<StringLiteral *> BSSSegStack;
   PragmaStack<StringLiteral *> ConstSegStack;
   PragmaStack<StringLiteral *> CodeSegStack;
-  // TODO: Change implementation of #pragma pack to use PragmaStack<> approach.
 
   // RAII object to push / pop sentinel slots for all MS #pragma stacks.
   // Actions should be performed only if we enter / exit a C++ method body.
   class PragmaStackSentinelRAII {
   public:
     PragmaStackSentinelRAII(Sema &S, StringRef SlotLabel, bool ShouldAct);
     ~PragmaStackSentinelRAII();
 
enum PragmaOptionsAlignKind {
     POAK_Mac68k,  // #pragma options align=mac68k
     POAK_Reset    // #pragma options align=reset
   };
 
   /// ActOnPragmaOptionsAlign - Called on well formed \#pragma options align.
   void ActOnPragmaOptionsAlign(PragmaOptionsAlignKind Kind,
                                SourceLocation PragmaLoc);
 
-  enum PragmaPackKind {
-    PPK_Default, // #pragma pack([n])
-    PPK_Show,    // #pragma pack(show), only supported by MSVC.
-    PPK_Push,    // #pragma pack(push, [identifier], [n])
-    PPK_Pop      // #pragma pack(pop, [identifier], [n])
-  };
-
   /// ActOnPragmaPack - Called on well formed \#pragma pack(...).
-  void ActOnPragmaPack(PragmaPackKind Kind,
-                       IdentifierInfo *Name,
-                       Expr *Alignment,
-                       SourceLocation PragmaLoc,
-                       SourceLocation LParenLoc,
-                       SourceLocation RParenLoc);
+  void ActOnPragmaPack(SourceLocation PragmaLoc, PragmaMsStackAction Action,
+                       StringRef SlotLabel, Expr *Alignment);
 
   /// ActOnPragmaMSStruct - Called on well formed \#pragma ms_struct [on|off].
   void ActOnPragmaMSStruct(PragmaMSStructKind Kind);
 
   /// ActOnPragmaMSComment - Called on well formed
   /// \#pragma comment(kind, "arg").
   void ActOnPragmaMSComment(SourceLocation CommentLoc, PragmaMSCommentKind Kind,
                             StringRef Arg);

cfe/trunk/lib/Parse/ParsePragma.cpp

void Parser::HandlePragmaVisibility() {
   const IdentifierInfo *VisType =
     static_cast<IdentifierInfo *>(Tok.getAnnotationValue());
   SourceLocation VisLoc = ConsumeToken();
   Actions.ActOnPragmaVisibility(VisType, VisLoc);
 }
 
 namespace {
 struct PragmaPackInfo {
-  Sema::PragmaPackKind Kind;
-  IdentifierInfo *Name;
+  Sema::PragmaMsStackAction Action;
+  StringRef SlotLabel;
   Token Alignment;
-  SourceLocation LParenLoc;
-  SourceLocation RParenLoc;
 };
 } // end anonymous namespace
 
 void Parser::HandlePragmaPack() {
   assert(Tok.is(tok::annot_pragma_pack));
   PragmaPackInfo *Info =
     static_cast<PragmaPackInfo *>(Tok.getAnnotationValue());
   SourceLocation PragmaLoc = ConsumeToken();
   ExprResult Alignment;
   if (Info->Alignment.is(tok::numeric_constant)) {
     Alignment = Actions.ActOnNumericConstant(Info->Alignment);
     if (Alignment.isInvalid())
       return;
   }
-  Actions.ActOnPragmaPack(Info->Kind, Info->Name, Alignment.get(), PragmaLoc,
-                          Info->LParenLoc, Info->RParenLoc);
+  Actions.ActOnPragmaPack(PragmaLoc, Info->Action, Info->SlotLabel,
+                          Alignment.get());
 }
 
 void Parser::HandlePragmaMSStruct() {
   assert(Tok.is(tok::annot_pragma_msstruct));
   PragmaMSStructKind Kind = static_cast<PragmaMSStructKind>(
       reinterpret_cast<uintptr_t>(Tok.getAnnotationValue()));
   Actions.ActOnPragmaMSStruct(Kind);
   ConsumeToken(); // The annotation token.
void PragmaPackHandler::HandlePragma(Preprocessor &PP,
 
   Token Tok;
   PP.Lex(Tok);
   if (Tok.isNot(tok::l_paren)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_lparen) << "pack";
     return;
   }
 
-  Sema::PragmaPackKind Kind = Sema::PPK_Default;
-  IdentifierInfo *Name = nullptr;
+  Sema::PragmaMsStackAction Action = Sema::PSK_Reset;
+  StringRef SlotLabel;
   Token Alignment;
   Alignment.startToken();
-  SourceLocation LParenLoc = Tok.getLocation();
   PP.Lex(Tok);
   if (Tok.is(tok::numeric_constant)) {
     Alignment = Tok;
 
     PP.Lex(Tok);
 
     // In MSVC/gcc, #pragma pack(4) sets the alignment without affecting
     // the push/pop stack.
     // In Apple gcc, #pragma pack(4) is equivalent to #pragma pack(push, 4)
-    if (PP.getLangOpts().ApplePragmaPack)
-      Kind = Sema::PPK_Push;
+    Action =
+        PP.getLangOpts().ApplePragmaPack ? Sema::PSK_Push_Set : Sema::PSK_Set;
   } else if (Tok.is(tok::identifier)) {
     const IdentifierInfo *II = Tok.getIdentifierInfo();
     if (II->isStr("show")) {
-      Kind = Sema::PPK_Show;
+      Action = Sema::PSK_Show;
       PP.Lex(Tok);
     } else {
       if (II->isStr("push")) {
-        Kind = Sema::PPK_Push;
+        Action = Sema::PSK_Push;
       } else if (II->isStr("pop")) {
-        Kind = Sema::PPK_Pop;
+        Action = Sema::PSK_Pop;
       } else {
         PP.Diag(Tok.getLocation(), diag::warn_pragma_invalid_action) << "pack";
         return;
       }
       PP.Lex(Tok);
 
       if (Tok.is(tok::comma)) {
         PP.Lex(Tok);
 
         if (Tok.is(tok::numeric_constant)) {
+          Action = (Sema::PragmaMsStackAction)(Action | Sema::PSK_Set);
           Alignment = Tok;
 
           PP.Lex(Tok);
         } else if (Tok.is(tok::identifier)) {
-          Name = Tok.getIdentifierInfo();
+          SlotLabel = Tok.getIdentifierInfo()->getName();
           PP.Lex(Tok);
 
           if (Tok.is(tok::comma)) {
             PP.Lex(Tok);
 
             if (Tok.isNot(tok::numeric_constant)) {
               PP.Diag(Tok.getLocation(), diag::warn_pragma_pack_malformed);
               return;
             }
 
+            Action = (Sema::PragmaMsStackAction)(Action | Sema::PSK_Set);
             Alignment = Tok;
 
             PP.Lex(Tok);
           }
         } else {
           PP.Diag(Tok.getLocation(), diag::warn_pragma_pack_malformed);
           return;
         }
       }
     }
   } else if (PP.getLangOpts().ApplePragmaPack) {
     // In MSVC/gcc, #pragma pack() resets the alignment without affecting
     // the push/pop stack.
     // In Apple gcc #pragma pack() is equivalent to #pragma pack(pop).
-    Kind = Sema::PPK_Pop;
+    Action = Sema::PSK_Pop;
   }
 
   if (Tok.isNot(tok::r_paren)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_expected_rparen) << "pack";
     return;
   }
 
   SourceLocation RParenLoc = Tok.getLocation();
   PP.Lex(Tok);
   if (Tok.isNot(tok::eod)) {
     PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol) << "pack";
     return;
   }
 
   PragmaPackInfo *Info =
       PP.getPreprocessorAllocator().Allocate<PragmaPackInfo>(1);
-  Info->Kind = Kind;
-  Info->Name = Name;
+  Info->Action = Action;
+  Info->SlotLabel = SlotLabel;
   Info->Alignment = Alignment;
-  Info->LParenLoc = LParenLoc;
-  Info->RParenLoc = RParenLoc;
 
   MutableArrayRef<Token> Toks(PP.getPreprocessorAllocator().Allocate<Token>(1),
                               1);
   Toks[0].startToken();
   Toks[0].setKind(tok::annot_pragma_pack);
   Toks[0].setLocation(PackLoc);
   Toks[0].setAnnotationEndLoc(RParenLoc);
   Toks[0].setAnnotationValue(static_cast<void*>(Info));

cfe/trunk/lib/Sema/Sema.cpp

Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
            TranslationUnitKind TUKind,
            CodeCompleteConsumer *CodeCompleter)
   : ExternalSource(nullptr),
     isMultiplexExternalSource(false), FPFeatures(pp.getLangOpts()),
     LangOpts(pp.getLangOpts()), PP(pp), Context(ctxt), Consumer(consumer),
     Diags(PP.getDiagnostics()), SourceMgr(PP.getSourceManager()),
     CollectStats(false), CodeCompleter(CodeCompleter),
     CurContext(nullptr), OriginalLexicalContext(nullptr),
-    PackContext(nullptr), MSStructPragmaOn(false),
+    MSStructPragmaOn(false),
     MSPointerToMemberRepresentationMethod(
         LangOpts.getMSPointerToMemberRepresentationMethod()),
     VtorDispStack(MSVtorDispAttr::Mode(LangOpts.VtorDispMode)),
-    DataSegStack(nullptr), BSSSegStack(nullptr), ConstSegStack(nullptr),
-    CodeSegStack(nullptr), CurInitSeg(nullptr), VisContext(nullptr),
+    PackStack(0), DataSegStack(nullptr), BSSSegStack(nullptr),
+    ConstSegStack(nullptr), CodeSegStack(nullptr), CurInitSeg(nullptr),
+    VisContext(nullptr),
     IsBuildingRecoveryCallExpr(false),
     ExprNeedsCleanups(false), LateTemplateParser(nullptr),
     LateTemplateParserCleanup(nullptr),
     OpaqueParser(nullptr), IdResolver(pp), StdInitializerList(nullptr),
     CXXTypeInfoDecl(nullptr), MSVCGuidDecl(nullptr),
     NSNumberDecl(nullptr), NSValueDecl(nullptr),
     NSStringDecl(nullptr), StringWithUTF8StringMethod(nullptr),
     ValueWithBytesObjCTypeMethod(nullptr),
void Sema::Initialize() {
 
   DeclarationName BuiltinVaList = &Context.Idents.get("__builtin_va_list");
   if (IdResolver.begin(BuiltinVaList) == IdResolver.end())
     PushOnScopeChains(Context.getBuiltinVaListDecl(), TUScope);
 }
 
 Sema::~Sema() {
   llvm::DeleteContainerSeconds(LateParsedTemplateMap);
-  if (PackContext) FreePackedContext();
   if (VisContext) FreeVisContext();
   // Kill all the active scopes.
   for (unsigned I = 1, E = FunctionScopes.size(); I != E; ++I)
     delete FunctionScopes[I];
   if (FunctionScopes.size() == 1)
     delete FunctionScopes[0];
 
   // Tell the SemaConsumer to forget about us; we're going out of scope.

cfe/trunk/lib/Sema/SemaAttr.cpp

 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/Lookup.h"
 using namespace clang;
 
 //===----------------------------------------------------------------------===//
 // Pragma 'pack' and 'options align'
 //===----------------------------------------------------------------------===//
 
-namespace {
-  struct PackStackEntry {
-    // We just use a sentinel to represent when the stack is set to mac68k
-    // alignment.
-    static const unsigned kMac68kAlignmentSentinel = ~0U;
-
-    unsigned Alignment;
-    IdentifierInfo *Name;
-  };
-
-  /// PragmaPackStack - Simple class to wrap the stack used by #pragma
-  /// pack.
-  class PragmaPackStack {
-    typedef std::vector<PackStackEntry> stack_ty;
-
-    /// Alignment - The current user specified alignment.
-    unsigned Alignment;
-
-    /// Stack - Entries in the #pragma pack stack, consisting of saved
-    /// alignments and optional names.
-    stack_ty Stack;
-
-  public:
-    PragmaPackStack() : Alignment(0) {}
-
-    void setAlignment(unsigned A) { Alignment = A; }
-    unsigned getAlignment() { return Alignment; }
-
-    /// push - Push the current alignment onto the stack, optionally
-    /// using the given \arg Name for the record, if non-zero.
-    void push(IdentifierInfo *Name) {
-      PackStackEntry PSE = { Alignment, Name };
-      Stack.push_back(PSE);
-    }
-
-    /// pop - Pop a record from the stack and restore the current
-    /// alignment to the previous value. If \arg Name is non-zero then
-    /// the first such named record is popped, otherwise the top record
-    /// is popped. Returns true if the pop succeeded.
-    bool pop(IdentifierInfo *Name, bool IsReset);
-  };
-}  // end anonymous namespace.
-
-bool PragmaPackStack::pop(IdentifierInfo *Name, bool IsReset) {
-  // If name is empty just pop top.
-  if (!Name) {
-    // An empty stack is a special case...
-    if (Stack.empty()) {
-      // If this isn't a reset, it is always an error.
-      if (!IsReset)
-        return false;
-
-      // Otherwise, it is an error only if some alignment has been set.
-      if (!Alignment)
-        return false;
-
-      // Otherwise, reset to the default alignment.
-      Alignment = 0;
-    } else {
-      Alignment = Stack.back().Alignment;
-      Stack.pop_back();
-    }
-
-    return true;
-  }
-
-  // Otherwise, find the named record.
-  for (unsigned i = Stack.size(); i != 0; ) {
-    --i;
-    if (Stack[i].Name == Name) {
-      // Found it, pop up to and including this record.
-      Alignment = Stack[i].Alignment;
-      Stack.erase(Stack.begin() + i, Stack.end());
-      return true;
-    }
-  }
-
-  return false;
-}
-
 Sema::PragmaStackSentinelRAII::PragmaStackSentinelRAII(Sema &S,
                                                        StringRef SlotLabel,
                                                        bool ShouldAct)
     : S(S), SlotLabel(SlotLabel), ShouldAct(ShouldAct) {
   if (ShouldAct) {
     S.VtorDispStack.SentinelAction(PSK_Push, SlotLabel);
     S.DataSegStack.SentinelAction(PSK_Push, SlotLabel);
     S.BSSSegStack.SentinelAction(PSK_Push, SlotLabel);
     S.ConstSegStack.SentinelAction(PSK_Push, SlotLabel);
     S.CodeSegStack.SentinelAction(PSK_Push, SlotLabel);
   }
 }
 
 Sema::PragmaStackSentinelRAII::~PragmaStackSentinelRAII() {
   if (ShouldAct) {
     S.VtorDispStack.SentinelAction(PSK_Pop, SlotLabel);
     S.DataSegStack.SentinelAction(PSK_Pop, SlotLabel);
     S.BSSSegStack.SentinelAction(PSK_Pop, SlotLabel);
     S.ConstSegStack.SentinelAction(PSK_Pop, SlotLabel);
     S.CodeSegStack.SentinelAction(PSK_Pop, SlotLabel);
   }
 }
 
-/// FreePackedContext - Deallocate and null out PackContext.
-void Sema::FreePackedContext() {
-  delete static_cast<PragmaPackStack*>(PackContext);
-  PackContext = nullptr;
-}
-
 void Sema::AddAlignmentAttributesForRecord(RecordDecl *RD) {
-  // If there is no pack context, we don't need any attributes.
-  if (!PackContext)
+  // If there is no pack value, we don't need any attributes.
+  if (!PackStack.CurrentValue)
     return;
 
-  PragmaPackStack *Stack = static_cast<PragmaPackStack*>(PackContext);
-
   // Otherwise, check to see if we need a max field alignment attribute.
-  if (unsigned Alignment = Stack->getAlignment()) {
-    if (Alignment == PackStackEntry::kMac68kAlignmentSentinel)
+  if (unsigned Alignment = PackStack.CurrentValue) {
+    if (Alignment == Sema::kMac68kAlignmentSentinel)
       RD->addAttr(AlignMac68kAttr::CreateImplicit(Context));
     else
       RD->addAttr(MaxFieldAlignmentAttr::CreateImplicit(Context,
                                                         Alignment * 8));
   }
 }
 
 void Sema::AddMsStructLayoutForRecord(RecordDecl *RD) {
   if (MSStructPragmaOn)
     RD->addAttr(MSStructAttr::CreateImplicit(Context));
 
   // FIXME: We should merge AddAlignmentAttributesForRecord with
   // AddMsStructLayoutForRecord into AddPragmaAttributesForRecord, which takes
   // all active pragmas and applies them as attributes to class definitions.
   if (VtorDispStack.CurrentValue != getLangOpts().VtorDispMode)
     RD->addAttr(
         MSVtorDispAttr::CreateImplicit(Context, VtorDispStack.CurrentValue));
 }
 
 void Sema::ActOnPragmaOptionsAlign(PragmaOptionsAlignKind Kind,
                                    SourceLocation PragmaLoc) {
-  if (!PackContext)
-    PackContext = new PragmaPackStack();
-
-  PragmaPackStack *Context = static_cast<PragmaPackStack*>(PackContext);
-
+  PragmaMsStackAction Action = Sema::PSK_Reset;
+  unsigned Alignment;
   switch (Kind) {
     // For all targets we support native and natural are the same.
     //
     // FIXME: This is not true on Darwin/PPC.
   case POAK_Native:
   case POAK_Power:
   case POAK_Natural:
-    Context->push(nullptr);
-    Context->setAlignment(0);
+    Action = Sema::PSK_Push_Set;
+    Alignment = 0;
     break;
 
     // Note that '#pragma options align=packed' is not equivalent to attribute
     // packed, it has a different precedence relative to attribute aligned.
   case POAK_Packed:
-    Context->push(nullptr);
-    Context->setAlignment(1);
+    Action = Sema::PSK_Push_Set;
+    Alignment = 1;
     break;
 
   case POAK_Mac68k:
     // Check if the target supports this.
     if (!this->Context.getTargetInfo().hasAlignMac68kSupport()) {
       Diag(PragmaLoc, diag::err_pragma_options_align_mac68k_target_unsupported);
       return;
     }
-    Context->push(nullptr);
-    Context->setAlignment(PackStackEntry::kMac68kAlignmentSentinel);
+    Action = Sema::PSK_Push_Set;
+    Alignment = Sema::kMac68kAlignmentSentinel;
     break;
 
   case POAK_Reset:
     // Reset just pops the top of the stack, or resets the current alignment to
     // default.
-    if (!Context->pop(nullptr, /*IsReset=*/true)) {
+    Action = Sema::PSK_Pop;
+    if (PackStack.Stack.empty()) {
+      if (PackStack.CurrentValue) {
+        Action = Sema::PSK_Reset;
+      } else {
         Diag(PragmaLoc, diag::warn_pragma_options_align_reset_failed)
             << "stack empty";
+        return;
+      }
     }
     break;
   }
+
+  PackStack.Act(PragmaLoc, Action, StringRef(), Alignment);
 }
 
-void Sema::ActOnPragmaPack(PragmaPackKind Kind, IdentifierInfo *Name,
-                           Expr *alignment, SourceLocation PragmaLoc,
-                           SourceLocation LParenLoc, SourceLocation RParenLoc) {
+void Sema::ActOnPragmaPack(SourceLocation PragmaLoc, PragmaMsStackAction Action,
+                           StringRef SlotLabel, Expr *alignment) {
   Expr *Alignment = static_cast<Expr *>(alignment);
 
   // If specified then alignment must be a "small" power of two.
   unsigned AlignmentVal = 0;
   if (Alignment) {
     llvm::APSInt Val;
 
     // pack(0) is like pack(), which just works out since that is what
     // we use 0 for in PackAttr.
     if (Alignment->isTypeDependent() ||
         Alignment->isValueDependent() ||
         !Alignment->isIntegerConstantExpr(Val, Context) ||
         !(Val == 0 || Val.isPowerOf2()) ||
         Val.getZExtValue() > 16) {
       Diag(PragmaLoc, diag::warn_pragma_pack_invalid_alignment);
       return; // Ignore
     }
 
     AlignmentVal = (unsigned) Val.getZExtValue();
   }
-
-  if (!PackContext)
-    PackContext = new PragmaPackStack();
-
-  PragmaPackStack *Context = static_cast<PragmaPackStack*>(PackContext);
-
-  switch (Kind) {
-  case Sema::PPK_Default: // pack([n])
-    Context->setAlignment(AlignmentVal);
-    break;
-
-  case Sema::PPK_Show: // pack(show)
+  if (Action == Sema::PSK_Show) {
     // Show the current alignment, making sure to show the right value
     // for the default.
-    AlignmentVal = Context->getAlignment();
     // FIXME: This should come from the target.
+    AlignmentVal = PackStack.CurrentValue;
     if (AlignmentVal == 0)
       AlignmentVal = 8;
-    if (AlignmentVal == PackStackEntry::kMac68kAlignmentSentinel)
+    if (AlignmentVal == Sema::kMac68kAlignmentSentinel)
       Diag(PragmaLoc, diag::warn_pragma_pack_show) << "mac68k";
     else
       Diag(PragmaLoc, diag::warn_pragma_pack_show) << AlignmentVal;
-    break;
-
-  case Sema::PPK_Push: // pack(push [, id] [, [n])
-    Context->push(Name);
-    // Set the new alignment if specified.
-    if (Alignment)
-      Context->setAlignment(AlignmentVal);
-    break;
-
-  case Sema::PPK_Pop: // pack(pop [, id] [,  n])
-    // MSDN, C/C++ Preprocessor Reference > Pragma Directives > pack:
-    // "#pragma pack(pop, identifier, n) is undefined"
-    if (Alignment && Name)
+  }
+  if (Action & Sema::PSK_Pop) {
+    if (Alignment && !SlotLabel.empty())
       Diag(PragmaLoc, diag::warn_pragma_pack_pop_identifer_and_alignment);
-
-    // Do the pop.
-    if (!Context->pop(Name, /*IsReset=*/false)) {
-      // If a name was specified then failure indicates the name
-      // wasn't found. Otherwise failure indicates the stack was
-      // empty.
-      Diag(PragmaLoc, diag::warn_pragma_pop_failed)
-          << "pack" << (Name ? "no record matching name" : "stack empty");
-
-      // FIXME: Warn about popping named records as MSVC does.
-    } else {
-      // Pop succeeded, set the new alignment if specified.
-      if (Alignment)
-        Context->setAlignment(AlignmentVal);
-    }
-    break;
+    if (PackStack.Stack.empty())
+      Diag(PragmaLoc, diag::warn_pragma_pop_failed) << "pack" << "stack empty";
   }
+
+  PackStack.Act(PragmaLoc, Action, SlotLabel, AlignmentVal);
 }
 
 void Sema::ActOnPragmaMSStruct(PragmaMSStructKind Kind) { 
   MSStructPragmaOn = (Kind == PMSST_ON);
 }
 
 void Sema::ActOnPragmaMSComment(SourceLocation CommentLoc,
                                 PragmaMSCommentKind Kind, StringRef Arg) {