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