[PATCH] Add PragmaAttr and Pragma Spelling to Tablegen

Tyler Nowicki tnowicki at apple.com
Mon Jun 2 17:19:12 PDT 2014


Hi Aaron,

Richard suggested moving pragma loop into the clang namespace so that other compilers ignore it. So I added the namespace we talked about in this thread to the Pragma spelling.

I think I understood what you described below. Please let me know if it looks right to you.

Tyler

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pragma_tablegen.patch
Type: application/octet-stream
Size: 7247 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140602/42de8de3/attachment.obj>
-------------- next part --------------

On May 30, 2014, at 10:51 AM, Aaron Ballman <aaron at aaronballman.com> wrote:

> I'm not certain I was clear in the design I was hoping for. I would
> like all consumers to use printPretty. So there should not be an
> exposed printPrettyPragma virtual function. Otherwise, the consumer is
> forced to write code that checks whether the spelling is a pragma or
> not, and pick a function as appropriate. Instead, the tablegenerator
> can do this already when generating printPretty. If you look at the
> generated AttrImpl.inc, you'll see that all printPretty methods are a
> switch method based on the SpellingListIndex. This list index
> specifies what spelling variant was used, so there's a new one for
> pragma that you're adding. So the generated code should look something
> like this:
> 
> case X: { // for whatever X the pragma enum value works out to be
>  OS << "#pragma ";
>  printPrettyPragma(OS, Policy);
>  break;
> }
> 
> The printPrettyPragma function is something that the attribute author
> must provide as an additional member (preferably as a private method).
> 
> let AdditionalMembers = [{
> private:
> void printPrettyPragma(raw_ostream &OS, const PrintingPolicy &Policy) const {
>  // Magic happens here
> }
> }];
> 
> If the author of the attribute fails to provide a printPrettyPragma
> function as an additional member when a pragma spelling is present,
> the code will fail to compile.
> 
> As for the prefix -- don't worry about it. It's an incremental change
> that can be added later.
> 
> ~Aaron
> 
> On Thu, May 29, 2014 at 5:08 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
>> Hi Aaron,
>> 
>> Thanks for all the help with these patches. I’m learning a lot about clang.
>> 
>> Here is an updated patch. I did not add a prefix to the Pragma because I
>> don’t really understand it too well, how to use it or test it. I used the
>> printPrettyPragma approach described below.
>> 
>> If I missed any changes please let me know.
>> 
>> Tyler
>> 
>> 
>> 
>> On May 28, 2014, at 11:59 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> 
>> On Wed, May 28, 2014 at 2:45 PM, Reid Kleckner <rnk at google.com> wrote:
>> 
>> On Wed, May 28, 2014 at 6:42 AM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>> 
>> 
>> On Tue, May 27, 2014 at 6:28 PM, Reid Kleckner <rnk at google.com> wrote:
>> 
>> On Sat, May 24, 2014 at 8:28 AM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>> 
>> 
>> My first idea is to add a tablegen property like "HasPrettyPrinter =
>> 0
>> or 1"
>> that supresses emission of the default printPretty method.  Then
>> again,
>> we
>> kind of want the default version when the GNU and C++11 spellings of
>> the
>> attribute are used.
>> 
>> 
>> That seems like the wrong way to go about it though, since this is a
>> new *spelling*, not a wholly new semantic kind of attribute (which I
>> think we really want to avoid). We could possibly attach a custom
>> pretty printer to the spelling itself (a la a code block), but that
>> seems a bit skeezy because the arguments are rather important and the
>> spelling shouldn't have to care about the arguments.
>> 
>> 
>> 
>> How about this: when generating the prettyPrint implementation, if there
>> is
>> a pragma spelling, call MyAttr::printPrettyPragma(...).  Any attribute
>> with
>> a pragma spelling will have to provide this method in
>> lib/AST/AttrImpl.cpp.
>> 
>> 
>> So my ultimate goal is for pragma attributes to mirror all of the
>> other attributes, where we can infer syntax and semantics just from
>> what's in Attr.td. But it's becoming more obvious to me that the
>> amount of work required to get there is rather large (mostly due to
>> the fact that pragma argument syntax is ALL over the place).
>> 
>> As a stopgap solution, it seems reasonable to me to rely on an
>> AdditionalMember being defined for pragmas (which is basically what
>> you're suggesting).
>> 
>> So if an attribute has a pragma spelling, it is required to have:
>> 
>> let AdditionalMembers = [{
>> void printPrettyPragma(raw_ostream &OS, const PrintingPolicy &Policy)
>> const {
>> }
>> }];
>> 
>> 
>> 
>> Sure, but... I like writing C++ in .cpp files, not .td files.  +1 for
>> defining it out of line in AttrImpl.cpp.  :)
>> 
>> 
>> That's an understandable desire, but a step in the wrong direction for
>> two reasons. 1) Anything declarative that we can put into the .td file
>> belongs in the .td file. There's nothing about this problem which
>> isn't declarative aside from the fact that we don't have the time
>> (yet) to make the proper declarations regarding parameter syntax. 2)
>> Putting it into AttrImpl.cpp decouples the logic from what gets
>> generated, which is far worse than writing some C++ code in a .td
>> file. You would have to write an out-of-line definition for
>> FooAttr::printPrettyPragma where the declaration gets automatically
>> generated for you elsewhere, which is rather ugly as sin when we
>> already have an alternative that suffices.
>> 
>> Also, this isn't a whole lot of complex C++ code either. The less of
>> it you write in the .td file, the more likely it is we will get the
>> declarative portions down without a mess of refactoring.
>> 
>> ~Aaron
>> 
>> 
>> 



More information about the cfe-commits mailing list