[PATCH] Implementation of #pragma (data|code|const|bss)_seg
David Majnemer
david.majnemer at gmail.com
Thu Apr 3 07:52:32 PDT 2014
I don't think __declspec(code_seg) should be part of this, it's complexity
is more on the order of dllimport/dllexport as it has complex inheritance
rules.
On Thursday, April 3, 2014, Aaron Ballman <aaron at aaronballman.com> wrote:
> Would it make sense to also add __declspec(code_seg) as part of this
> patch? http://msdn.microsoft.com/en-us/library/dn636922.aspx
> Especially since MSDN recommends it over #pragma code_seg.
>
> The patch is missing parser test cases.
>
> More comments below.
>
> > Index: include/clang/Basic/Attr.td
> > ===================================================================
> > --- include/clang/Basic/Attr.td
> > +++ include/clang/Basic/Attr.td
> > @@ -1069,7 +1069,7 @@
> > }
> >
> > def Section : InheritableAttr {
> > - let Spellings = [GCC<"section">];
> > + let Spellings = [GCC<"section">, Declspec<"allocate">];
> > let Args = [StringArgument<"Name">];
> > let Subjects = SubjectList<[Function, GlobalVar,
> > ObjCMethod, ObjCProperty], ErrorDiag,
>
> I'd love to see:
>
> + let Documentation = [Anything other than Undocumented];
>
> ;-)
>
> > Index: include/clang/Basic/DiagnosticParseKinds.td
> > ===================================================================
> > --- include/clang/Basic/DiagnosticParseKinds.td
> > +++ include/clang/Basic/DiagnosticParseKinds.td
> > @@ -771,6 +771,8 @@
> > "missing ')' after '#pragma %0' - ignoring">, InGroup<IgnoredPragmas>;
> > def warn_pragma_expected_identifier : Warning<
> > "expected identifier in '#pragma %0' - ignored">,
> InGroup<IgnoredPragmas>;
> > +def warn_pragma_expected_string : Warning<
> > + "expected string literal in '#pragma %0' - ignored">,
> InGroup<IgnoredPragmas>;
>
> This could be combined with warn_pragma_expected_identifier using a
> %select.
>
> > def warn_pragma_expected_integer : Warning<
> > "expected integer between %0 and %1 inclusive in '#pragma %2' -
> ignored">,
> > InGroup<IgnoredPragmas>;
> > Index: include/clang/Basic/DiagnosticSemaKinds.td
> > ===================================================================
> > --- include/clang/Basic/DiagnosticSemaKinds.td
> > +++ include/clang/Basic/DiagnosticSemaKinds.td
> > @@ -516,6 +516,7 @@
> > Warning<"ms_struct may not produce MSVC-compatible layouts for
> classes "
> > "with base classes or virtual functions">,
> > DefaultError, InGroup<IncompatibleMSStruct>;
> > +def err_section_conflict : Error<"%0 causes a section type conflict
> with %1">;
> >
> > def warn_pragma_unused_undeclared_var : Warning<
> > "undeclared variable %0 used as an argument for '#pragma unused'">,
> > Index: include/clang/Basic/TokenKinds.def
> > ===================================================================
> > --- include/clang/Basic/TokenKinds.def
> > +++ include/clang/Basic/TokenKinds.def
> > @@ -684,6 +684,11 @@
> > // handles them.
> > ANNOTATION(pragma_ms_vtordisp)
> >
> > +// Annotation for all microsoft #pragmas...
> > +// The lexer produces these so that they only take effect when the
> parser
> > +// handles them.
> > +ANNOTATION(pragma_ms_pragma)
> > +
> > // Annotation for #pragma OPENCL EXTENSION...
> > // The lexer produces these so that they only take effect when the
> parser
> > // handles them.
> > Index: include/clang/Parse/Parser.h
> > ===================================================================
> > --- include/clang/Parse/Parser.h
> > +++ include/clang/Parse/Parser.h
> > @@ -154,6 +154,11 @@
> > std::unique_ptr<PragmaHandler> MSDetectMismatchHandler;
> > std::unique_ptr<PragmaHandler> MSPointersToMembers;
> > std::unique_ptr<PragmaHandler> MSVtorDisp;
> > + std::unique_ptr<PragmaHandler> MSDataSeg;
> > + std::unique_ptr<PragmaHandler> MSBSSSeg;
> > + std::unique_ptr<PragmaHandler> MSConstSeg;
> > + std::unique_ptr<PragmaHandler> MSCodeSeg;
> > + std::unique_ptr<PragmaHandler> MSSection;
> >
> > std::unique_ptr<CommentHandler> CommentSemaHandler;
> >
> > @@ -475,6 +480,12 @@
> >
> > void HandlePragmaMSVtorDisp();
> >
> > + void HandlePragmaMSPragma();
> > + unsigned HandlePragmaMSSection(llvm::StringRef PragmaName,
> > + SourceLocation PragmaLocation);
> > + unsigned HandlePragmaMSXXXXSeg(llvm::StringRef PragmaName,
> > + SourceLocation PragmaLocation);
>
> I'm not too keen on XXXXSeg as the name. Perhaps
> HandlePragmaMSSegment? Sorry, did I mention I like my bikesheds red?
>
> > +
> > /// \brief Handle the annotation token produced for
> > /// #pragma align...
> > void HandlePragmaAlign();
> > Index: include/clang/Sema/Sema.h
> > ===================================================================
> > --- include/clang/Sema/Sema.h
> > +++ include/clang/Sema/Sema.h
> > @@ -274,6 +274,15 @@
> > PVDK_Reset ///< #pragma vtordisp()
> > };
> >
> > + enum PragmaMsStackAction {
> > + PSK_Reset, // #pragma ()
> > + PSK_Set, // #pragma ("name")
> > + PSK_Push, // #pragma (push[, id])
> > + PSK_Push_Set, // #pragma (push[, id], "name")
> > + PSK_Pop, // #pragma (pop[, id])
> > + PSK_Pop_Set, // #pragma (pop[, id], "name")
> > + };
> > +
> > /// \brief Whether to insert vtordisps prior to virtual bases in the
> Microsoft
> > /// C++ ABI. Possible values are 0, 1, and 2, which mean:
> > ///
> > @@ -289,6 +298,30 @@
> > /// \brief Source location for newly created implicit
> MSInheritanceAttrs
> > SourceLocation ImplicitMSInheritanceAttrLoc;
> >
> > + template<typename ValueType>
> > + struct PragmaStack {
> > + struct Slot {
> > + llvm::StringRef StackSlotLabel;
> > + ValueType Value;
> > + SourceLocation PragmaLocation;
> > + };
> > + void Act(SourceLocation PragmaLocation,
> > + PragmaMsStackAction Action,
> > + llvm::StringRef StackSlotLabel,
> > + ValueType Value);
> > + explicit PragmaStack(const ValueType& Value)
>
> According to the style guidelines, the reference goes with the
> identifier, not the type. This applies across the entire patch (same
> with pointers).
>
> > + : CurrentValue(Value) {}
> > + SmallVector<Slot, 2> Stack;
> > + ValueType CurrentValue;
> > + SourceLocation CurrentPragmaLocation;
> > + };
> > + // FIXME: We should serialize / deserialize these if they occur in a
> PCH (but
> > + // we shouldn't do so if they're in a module).
> > + PragmaStack<StringLiteral *> DataSegStack;
> > + PragmaStack<StringLiteral *> BSSSegStack;
> > + PragmaStack<StringLiteral *> ConstSegStack;
> > + PragmaStack<StringLiteral *> CodeSegStack;
> > +
> > /// VisContext - Manages the stack for \#pragma GCC visibility.
> > void *VisContext; // Really a "PragmaVisStack*"
> >
> > @@ -7030,6 +7063,46 @@
> > void ActOnPragmaMSVtorDisp(PragmaVtorDispKind Kind, SourceLocation
> PragmaLoc,
> > MSVtorDispAttr::Mode Value);
> >
> > + enum PragmaMSKind {
> > + PSK_DataSeg,
> > + PSK_BSSSeg,
> > + PSK_ConstSeg,
> > + PSK_CodeSeg,
> > + };
> > +
> > + enum PragmaSectionFlag {
> > + PSF_None = 0,
> > + PSF_Read = 1,
> > + PSF_Write = 2,
> > + PSF_Execute = 4,
> > + PSF_Implicit = 8,
> > + };
> > +
> > + struct SectionInfo {
> > + DeclaratorDecl* Decl;
> > + SourceLocation PragmaSectionLocation;
> > + int SectionFlags;
> > + };
> > +
> > + llvm::StringMap<SectionInfo> SectionInfos;
>
> This doesn't need to be a public member, does it?
>
> > + bool UnifySection(const StringRef& SectionName,
> > + int SectionFlags,
> > + DeclaratorDecl* TheDecl);
> > + bool UnifySection(const StringRef& SectionName,
> > + int SectionFlags,
> > + SourceLocation PragmaSectionLocation);
> > +
> > + /// \brief Called on well formed \#pragma
> bss_seg/data_seg/const_seg/code_seg.
> > + void ActOnPragmaMSSeg(SourceLocation PragmaLocation,
> > + PragmaMsStackAction Action,
> > + llvm::StringRef StackSlotLabel,
> > + StringLiteral *SegmentName,
> > + llvm::StringRef PragmaName);
> > +
> > + /// \brief Called on well formed \#pragma section().
> > + void ActOnPragmaMSSection(SourceLocation PragmaLocation,
> > + int SectionFlags, StringLiteral
> *SegmentName);
> > +
> > /// ActOnPragmaDetectMismatch - Call on well-formed \#pragma
> detect_mismatch
> > void ActOnPragmaDetectMismatch(StringRef Name, StringRef Value);
> >
> > Index: lib/Parse/ParseDeclCXX.cpp
> > ===================================================================
> > --- lib/Parse/ParseDeclCXX.cpp
> > +++ lib/Parse/ParseDeclCXX.cpp
> > @@ -2646,6 +2646,11 @@
> > continue;
> > }
> >
> > + if (Tok.is(tok::annot_pragma_ms_pragma)) {
> > + HandlePragmaMSPragma();
> > + continue;
> > + }
> > +
> > // If we see a namespace here, a close brace was missing
> somewhere.
> > if (Tok.is(tok::kw_namespace)) {
> > DiagnoseUnexpectedNamespace(cast<NamedDecl>(TagDecl));
> > Index: lib/Parse/ParsePragma.cpp
> > ===================================================================
> > --- lib/Parse/ParsePragma.cpp
> > +++ lib/Parse/ParsePragma.cpp
> > @@ -11,6 +11,7 @@
> > //
> >
> //===----------------------------------------------------------------------===//
> >
> > +#include "RAIIObjectsForParser.h"
> > #include "clang/Lex/Preprocessor.h"
> > #include "clang/Parse/ParseDiagnostic.h"
> > #include "clang/Parse/Parser.h"
> > @@ -124,6 +125,12 @@
> > Token &FirstToken) override;
> > };
> >
> > +struct PragmaMSPragma : public PragmaHandler {
> > + explicit PragmaMSPragma(const char *name) : PragmaHandler(name) {}
> > + virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind
> Introducer,
> > + Token &FirstToken);
>
> Marked as override and drop the virtual?
>
> > +};
> > +
> > } // end namespace
> >
> > void Parser::initializePragmaHandlers() {
> > @@ -175,6 +182,16 @@
> > PP.AddPragmaHandler(MSPointersToMembers.get());
> > MSVtorDisp.reset(new PragmaMSVtorDisp());
> > PP.AddPragmaHandler(MSVtorDisp.get());
> > + MSDataSeg.reset(new PragmaMSPragma("data_seg"));
> > + PP.AddPragmaHandler(MSDataSeg.get());
> > + MSBSSSeg.reset(new PragmaMSPragma("bss_seg"));
> > + PP.AddPragmaHandler(MSBSSSeg.get());
> > + MSConstSeg.reset(new PragmaMSPragma("const_seg"));
> > + PP.AddPragmaHandler(MSConstSeg.get());
> > + MSCodeSeg.reset(new PragmaMSPragma("code_seg"));
> > + PP.AddPragmaHandler(MSCodeSeg.get());
> > + MSSection.reset(new PragmaMSPragma("section"));
> > + PP.AddPragmaHandler(MSSection.get());
> > }
> > }
> >
> > @@ -214,6 +231,16 @@
> > MSPointersToMembers.reset();
> > PP.RemovePragmaHandler(MSVtorDisp.get());
> > MSVtorDisp.reset();
> > + PP.RemovePragmaHandler(MSDataSeg.get());
> > + MSDataSeg.reset();
> > + PP.RemovePragmaHandler(MSBSSSeg.get());
> > + MSBSSSeg.reset();
> > + PP.RemovePragmaHandler(MSConstSeg.get());
> > + MSConstSeg.reset();
> > + PP.RemovePragmaHandler(MSCodeSeg.get());
> > + MSCodeSeg.reset();
> > + PP.RemovePragmaHandler(MSSection.get());
> > + MSSection.reset();
> > }
> >
> > PP.RemovePragmaHandler("STDC", FPContractHandler.get());
> > @@ -400,6 +427,120 @@
> > Actions.ActOnPragmaMSVtorDisp(Kind, PragmaLoc, Mode);
> > }
> >
> > +void Parser::HandlePragmaMSPragma() {
> > + assert(Tok.is(tok::annot_pragma_ms_pragma));
> > + // Grab the tokens out of the annotation and enter them into the
> stream.
> > + auto TheTokens = (std::pair<Token*, size_t>
> *)Tok.getAnnotationValue();
> > + PP.EnterTokenStream(TheTokens->first, TheTokens->second, true, true);
> > + SourceLocation PragmaLocation = ConsumeToken(); // The annotation
> token.
> > + assert(Tok.isAnyIdentifier());
> > + llvm::StringRef PragmaName = Tok.getIdentifierInfo()->getName();
> > + PP.Lex(Tok); // pragma kind
> > + // Figure out which #pragma we're dealing with. The switch has no
> default
> > + // because lex shouldn't emit the annotation token for unrecognized
> pragmas.
> > + typedef unsigned (Parser::*PragmaHandler)(llvm::StringRef,
> SourceLocation);
> > + PragmaHandler Handler = llvm::StringSwitch<PragmaHandler>(PragmaName)
> > + .Case("data_seg", &Parser::HandlePragmaMSXXXXSeg)
> > + .Case("bss_seg", &Parser::HandlePragmaMSXXXXSeg)
> > + .Case("const_seg", &Parser::HandlePragmaMSXXXXSeg)
> > + .Case("code_seg", &Parser::HandlePragmaMSXXXXSeg)
> > + .Case("section", &Parser::HandlePragmaMSSection);
> > + if (auto DiagID = (this->*Handler)(PragmaName, PragmaLocation)) {
> > + PP.Diag(PragmaLocation, DiagID) << PragmaName;
> > + SkipUntil(tok::eof);
> > + PP.Lex(Tok); // tok::eof
> > + }
> > +}
> > +
> > +unsigned Parser::HandlePragmaMSSection(llvm::StringRef PragmaName,
> > + SourceLocation PragmaLocation) {
>
> This could use Parser tests.
>
> > + BalancedDelimiterTracker Parens(*this, tok::l_paren, tok::eof);
> > + if (Parens.consumeOpen())
> > + return diag::warn_pragma_expected_lparen;
> > + // Parsing code for pragma section
> > + if (Tok.isNot(tok::string_literal))
>
> Should you be calling Parser::isTokenStringLiteral() here instead? If
> not, there should be a test that shows wide string literals fail to
> parse.
>
> > + return diag::warn_pragma_expected_string;
> > + StringLiteral *SegmentName =
> > + cast<StringLiteral>(ParseStringLiteralExpression().get());
> > + int SectionFlags = 0;
> > + while (Tok.is(tok::comma)) {
> > + PP.Lex(Tok); // ,
> > + if (!Tok.isAnyIdentifier())
> > + return diag::warn_pragma_invalid_action;
> > + Sema::PragmaSectionFlag Flag =
> > + llvm::StringSwitch<Sema::PragmaSectionFlag>(
> > + Tok.getIdentifierInfo()->getName())
> > + .Case("read", Sema::PSF_Read)
> > + .Case("write", Sema::PSF_Write)
> > + .Case("execute", Sema::PSF_Execute)
>
> What about shared, nopage, nocache, discard and remove? I know they're
> covered by returning PSF_None, but I would
> > + BalancedDelimiterTracker Parens(*this, tok::l_paren, tok::eof);
> > + if (Parens.consumeOpen())
> > + return diag::warn_pragma_expected_lparen;
> > + Sema::PragmaMsStack>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140403/18068967/attachment.html>
More information about the cfe-commits
mailing list