[PATCH] Declaring standard C++ attributes

[Richard Smith]

On Wed Jan 29 2014 at 1:17:50 PM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Wed, Jan 29, 2014 at 4:01 PM, Richard Smith <metafoo at gmail.com> wrote:
> > On Wed Jan 29 2014 at 12:54:14 PM, Aaron Ballman <aaron at aaronballman.com
> >
> > wrote:
> >>
> >> On Wed, Jan 29, 2014 at 3:45 PM, Richard Smith <metafoo at gmail.com>
> wrote:
> >> > I'm not really sure what this is for: don't we want to have a flag for
> >> > 'cannot have parens' rather than 'is a standardized C++ attribute'?
> >>
> >> Since standardized C++ attributes all have custom parsing rules (some
> >> can have parens, some can't, and the future holds mystery), I think it
> >> makes sense to have two cases: things which the standard mandates the
> >> parsing rules for, and things which can be generically parsed.
> >
> >
> > So this flag is (at best) misnamed, then, since we want the generic
> parsing
> > logic for [[deprecated]]?
>
> We don't want generic parsing logic for [[deprecated]] because it's a
> standards-based attribute (in C++1y). We need to error out at parse
> time if it's ill-formed, don't we?
>

We need to diagnose it somewhere. It could be in Sema -- that's what we do
for all other attributes, and I don't see any need to be inconsistent here.


> I think the flag is properly-named based on what I was intending --
> identify which attributes are C++ standards-based attributes.
>

When is that useful?


> >
> > It's also causing us to do the wrong thing for [[clang::fallthrough]];
> > (which starts to accept arguments with this patch, but should not).
>
> It is true that this would change behavior for fallthrough. Inn
> handleFallThroughAttr, I should be ensuring the attribute accepted no
> arguments.
>

We don't want to allow parens here, either.


> >
> > It seems like what we want is a flag that means "may not have an argument
> > clause".
>
> It's my (perhaps totally wrong) understanding that with
> standards-blessed C++ attributes, we want to handle these sort of
> things at parse time, but with implementation-specific attributes, we
> can handle it in sema. Eg) we need to error out at parse time with
> [[noreturn()]] or [[deprecated(12)]], but we can error out at sema
> time with [[clang::fallthrough("foo")]] and even accept
> [[clang::fallthrough()]].
>
> >>
> >> > Also, it
> >> > seems easy to detect standard attributes: they're C++ attributes with
> no
> >> > scope token. We don't need to generate a flag for that.
> >>
> >> Except there's nothing mandating that (for instance, gcc or MSVC
> >> [someday] could arbitrarily add a C++ style attribute without a scope,
> >> ignoring the non-normative note to the contrary). This gives us
> >> flexibility to handle it, though it's not required today.
> >
> >
> > If they do, we should publicly shame them and refuse to implement it. =)
>
> :-)
>
> >
> >>
> >> > It looks like this causes a regression: we start rejecting
> >> > [[deprecated("foo")]] with it, don't we?
> >>
> >> We don't support [[deprecated("foo")]] yet, nor are there any tests
> >> for it. We only support [[deprecated]] currently.
> >
> >
> > We do support it, and it works (but I agree, we seem to have forgotten to
> > handle this properly and there are no tests for it). The argument clause
> is
> > ignored entirely for '[[deprecated]]' today, which is not entirely
> > conforming, since we're required to reject arguments that aren't
> > string-literals, but is more conforming than rejecting such arguments.
>
> I don't see this as regressing anything; we didn't handle C++11-style
> deprecated with a parameter list previously, and this patch doesn't
> change that. But it's a moot point.
>

I don't know what you mean. We used to accept it. With your patch we reject
it. That seems like a regression to me! =)

My goal was to be a bit more declarative instead of having a
> hard-coded list, but really, I think that list is somewhat unavoidable
> and very slow to grow. So I don't think this patch is as useful as I
> had originally expected.
>
> ~Aaron
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140129/1b5bc946/attachment.html>