[PATCH] Add PragmaAttr and Pragma Spelling to Tablegen

Tyler Nowicki tnowicki at apple.com
Tue May 27 15:02:11 PDT 2014


Hi Aaron,

Thanks for the review. I agree I will remove the PragmaAttr class.

Many of your other comments revolve around the way Pragma attributes are printed. Can you tell me why you disagree with a custom print method within the Attr? Perhaps a new method in the Attr class could be written to provide a vector of the attribute’s Args and as discussed we can union the Kind and the Value. So a pretty printer could do something like:

OS << “#pragma “ << NAME;
for (auto I : Args) {
  OS << “ “;
  I.printPretty(OS, Policy)
}

What do you think?

>> @@ -192,0 +193 @@ class Keyword<string name> : Spelling<name, "Keyword">;
>> +class Pragma<string name> : Spelling<name, "Pragma">;
> 
> I'm not certain this spelling is quite sufficient as many pragmas have
> a namespace of sorts. As best I can tell, most pragmas come in one of
> four forms:
> 
> #pragma foo bar(args)
> #pragma foo bar args
> #pragma foo(args)
> #pragma foo args
> 
> For instance, your own pragma is #pragma loop vectorize(args) --or--
> #pragma loop interleave(args). So I think we want this to be:
> 
> class Pragma<string prefix, string name> : Spelling<name, "Pragma"> {
>  let Prefix = prefix;
> }

I’m not sure what the prefix is here. Can you give me an example?


>> -    // FIXME: Replace AS_Keyword with Pragma spelling AS_Pragma.
>> @@ -1786 +1785 @@ StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts, bool OnlyStatement,
>> -                     ArgHints, 3, AttributeList::AS_Keyword);
>> +                     ArgHints, 3, AttributeList::AS_Pragma);
> 
> I would bet there's quite a few more attributes that need this modification.

There is only one other stmt attribute right now. It is `fallthrough' for switch case stmts. The rest are declaration attributes and are handled differently that stmt attributes. For instance, a declaration attribute is not even represented with an AST node like attributed stmt.

Tyler



More information about the cfe-commits mailing list