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 <[email protected]> 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>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
