[PATCH] Add PragmaAttr and Pragma Spelling to Tablegen

[Aaron Ballman]

On Tue, May 27, 2014 at 6:02 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
> 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?

The printPretty method itself is generated from ClangAttrEmitter.cpp.
What I think would be ideal would be to modify ClangAttrEmitter.cpp to
generate a printPretty function that correctly handles when a pragma
spelling is used. See writePrettyPrintFunction for a good start -- it
already has logic which looks at the spelling list index for which
spelling is used, so you should be able to alter it for pragmas. The
trickiest part will be determining how best to standardize pragma
argument printing from a tablegen syntax perspective.

>
>>> @@ -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?

#pragma STDC FENV_ACCESS ON

STDC is a prefix, FENV_ACCESS is the name, and ON is the parameter.

#pragma listing on "..\listing.dir"

There is no prefix, listing is the name, on and the string literal are
parameters.

#pragma warning(push, 1)

There is no prefix, warning is the name, push and the integer literal
are parameters.

>
>
>>> -    // 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.

That's a good point about this being for statement attributes.

~Aaron