[PATCH] D18542: [OPENMP] Parsing and Sema support for 'omp declare target' directive

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 31 09:11:12 PDT 2016


aaron.ballman added a comment.

In http://reviews.llvm.org/D18542#387934, @DmitryPolukhin wrote:

> Could you please take a look to OMPDeclareTargetDecl attribute implementation and printPrettyEnd approach in general?
>  For post print mechanism alternative approach is to use ad hoc solution in DeclPrinter.


Attributes are meant to be wholly-contained entities syntactically, so I'm not entirely comfortable with the notion of an attribute having an "ending" because that breaks the model. However, pragmas and statement-level attributes are examples of attribute constructs that sometimes want to be paired together, so I see utility in the overall idea. I sort of feel like what we need is the notion of grouping related attributes together (we already need this for mutual exclusion of attributes, but I see no reason why it couldn't also cover mutual inclusion). Then pretty printing has a natural place for "end" logic within this declarative container.

If you don't feel like doing all that design work (which I'm not attempting to sign you up for!), I think the ad hoc solution may be preferable to the proposed approach. Do you intend to add several more pragmas that need this same sort of behavior, or is this a one-off?


================
Comment at: include/clang/Basic/Attr.td:2250
@@ +2249,3 @@
+def OMPDeclareTargetDecl : Attr {
+  // This attribute has no spellings as it is only ever created implicitly.
+  let Spellings = [Pragma<"omp", "declare target", "end declare target">];
----------------
It's a bit odd to say it has no spellings and then give it a spelling list. ;-)


http://reviews.llvm.org/D18542





More information about the cfe-commits mailing list