[PATCH] Add PragmaAttr and Pragma Spelling to Tablegen

Reid Kleckner rnk at google.com
Thu May 29 15:36:56 PDT 2014


+  // Pretty print pragma attribute. Attributes with pragma spellings should
+  // overload this method.

s/overload/override/

+  // FIXME: TableGen should be modified to print attributes with pragma
+  // spellings in a generic way.
+  virtual void printPrettyPragma(raw_ostream &OS,
+                                 const PrintingPolicy &Policy) const {
+    llvm_unreachable("printPrettyPragma must be specified for attributes
with "
+                     "a pragma spelling.");
+  }

Are you sure this needs to be a virtual method?  I feel like we'd get
better compile-time checks if we don't provide this for all Attr subclasses.

Don't you only emit calls to printPrettyPragma from the attribute class
implementation if the attribute has a pragma spelling?


On Thu, May 29, 2014 at 2: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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140529/215e29b7/attachment.html>


More information about the cfe-commits mailing list