[PATCH] Declaring standard C++ attributes

[Aaron Ballman]

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?

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

>
> 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.

>
> 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.

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