[PATCH] Attribute parameters and arguments

Richard Smith richard at metafoo.co.uk
Mon Sep 9 15:26:24 PDT 2013


Thanks!

HandleCommonAttributeFeatures: lowercase first letter.
SimpleArgument::writeCtorDefaultInitializers: maybe always just use the
"()" initializer?
You have a C++11 '>>' closing two templates on ClangAttrEmitter.cpp:1475.

The adding of the 'optional' bit and synthesis of new constructors seems to
be largely separate from this change (other than that you use the new bit
to determine whether you should perform an argument count check). In
particular, the new constructors don't seem to be used anywhere. Can you
split off that part of the change?


On Mon, Sep 9, 2013 at 2:54 PM, Aaron Ballman <aaron at aaronballman.com>wrote:

> (Bringing this back on the lists; it seems to have fallen off at some
> point.)
>
> On Mon, Sep 9, 2013 at 5:07 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> > On Mon, Sep 9, 2013 at 11:50 AM, Aaron Ballman <aaron at aaronballman.com>
> > wrote:
> >>
> >> On Mon, Sep 9, 2013 at 2:39 PM, Richard Smith <richard at metafoo.co.uk>
> >> wrote:
> >> > On Mon, Sep 9, 2013 at 11:31 AM, Aaron Ballman <
> aaron at aaronballman.com>
> >> > wrote:
> >> >>
> >> >> On Mon, Sep 9, 2013 at 2:03 PM, Richard Smith <richard at metafoo.co.uk
> >
> >> >> wrote:
> >> >> > On Mon, Sep 9, 2013 at 10:14 AM, Aaron Ballman
> >> >> > <aaron at aaronballman.com>
> >> >> > wrote:
> >> >> >>
> >> >> >> Now that the notion of a parameter has been removed from attribute
> >> >> >> arguments, this is the original patch (sans parameter
> functionality)
> >> >> >> rebased against ToT.  It automates the error checking for the
> number
> >> >> >> of attribute args expected vs given.
> >> >> >
> >> >> >
> >> >> > I'm not a fan of the name 'CommonErrOptOut'. I think what we're
> >> >> > expressing
> >> >> > here is that the attribute has custom parsing rules (that is, the
> >> >> > parsing
> >> >> > rule isn't just "parse the arguments as specified by Args"). Maybe
> >> >> > 'HasCustomParsing'?
> >> >>
> >> >> Hmm, it's a bit different than that to me.  More like
> >> >> HasCustomSemaChecking, since that's what it really specifies -- that
> >> >> sema checking cannot be automated.
> >> >
> >> >
> >> > I think what this flag means is that the Args list reflects the syntax
> >> > of
> >> > the attribute, not just its semantic contents. (It's unfortunate that
> >> > we've
> >> > muddled these up so much in the .td file, but it is at least
> convenient,
> >> > since they usually match exactly.)
> >>
> >> So you would like to see this flag tied tightly to Args?  I was
> >> thinking it might be a more general flag for opting out of automated
> >> semantics checks, with the eventual goal being that the flag goes away
> >> once everything can be table driven.
> >
> >
> > I think of the argument count (and type) checks as being parsing checks
> > rather than semantic checks. It used to be the case that the syntax of
> the
> > attributes was basically
> >
> >   attribute-name ( expression-list[opt] )
> >
> > (with a weird special case for the first argument, which only ever really
> > worked in C). But these days, it's more honest to say that each attribute
> > has its own syntax:
> >
> >   'address_space' ( constant-expression )
> >   'alias' ( string-literal )
> >   'aligned' ( constant-expression )
> >   'availability' ( identifier availability-arg-seq )
> >   ...
> >
> > and this flag essentially says whether we can deduce the attribute's
> grammar
> > from the Args list.
>
> Okay, that makes sense to me.  I've changed the patch to use
> HasCustomParsing instead.
>
> Updated patch attached.
>
> Thanks!
>
> ~Aaron
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130909/e7ed3a9c/attachment.html>


More information about the cfe-commits mailing list