d.zobnin.bugzilla created this revision. d.zobnin.bugzilla added a reviewer: rnk. d.zobnin.bugzilla added subscribers: thakis, cfe-commits.
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. http://reviews.llvm.org/D19727 Files: include/clang/Sema/Sema.h lib/Parse/ParsePragma.cpp lib/Sema/Sema.cpp lib/Sema/SemaAttr.cpp
Index: lib/Parse/ParsePragma.cpp =================================================================== --- lib/Parse/ParsePragma.cpp +++ lib/Parse/ParsePragma.cpp @@ -337,11 +337,9 @@ namespace { struct PragmaPackInfo { - Sema::PragmaPackKind Kind; - IdentifierInfo *Name; + Sema::PragmaMsStackAction Action; + StringRef SlotLabel; Token Alignment; - SourceLocation LParenLoc; - SourceLocation RParenLoc; }; } // end anonymous namespace @@ -356,8 +354,8 @@ 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() { @@ -962,11 +960,10 @@ 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; @@ -976,18 +973,18 @@ // 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; @@ -998,11 +995,12 @@ 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)) { @@ -1013,6 +1011,7 @@ return; } + Action = (Sema::PragmaMsStackAction)(Action | Sema::PSK_Set); Alignment = Tok; PP.Lex(Tok); @@ -1027,7 +1026,7 @@ // 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)) { @@ -1044,11 +1043,9 @@ 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); Index: lib/Sema/Sema.cpp =================================================================== --- lib/Sema/Sema.cpp +++ lib/Sema/Sema.cpp @@ -79,12 +79,13 @@ 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), @@ -252,7 +253,6 @@ 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) Index: lib/Sema/SemaAttr.cpp =================================================================== --- lib/Sema/SemaAttr.cpp +++ lib/Sema/SemaAttr.cpp @@ -25,86 +25,6 @@ // 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) @@ -128,22 +48,14 @@ } } -/// 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, @@ -165,53 +77,57 @@ 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)) { - Diag(PragmaLoc, diag::warn_pragma_options_align_reset_failed) - << "stack empty"; + 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. @@ -232,59 +148,28 @@ 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) + } + // MSDN, C/C++ Preprocessor Reference > Pragma Directives > pack: + // "#pragma pack(pop, identifier, n) is undefined" + 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) { Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -317,10 +317,6 @@ /// 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. @@ -338,6 +334,7 @@ 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) }; @@ -400,11 +397,15 @@ /// 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. @@ -7657,20 +7658,9 @@ 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);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits