Question about existing tests for [dcl.stc]

[Aaron Ballman]

On Fri, May 23, 2014 at 1:00 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On 23 May 2014 09:53, "Aaron Ballman" <aaron at aaronballman.com> wrote:
>>
>> On Thu, May 22, 2014 at 9:04 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> > On Mon, May 19, 2014 at 2:38 PM, Aaron Ballman <aaron at aaronballman.com>
>> > wrote:
>> >>
>> >> On Mon, May 19, 2014 at 5:32 PM, Richard Smith <richard at metafoo.co.uk>
>> >> wrote:
>> >> > You're citing the wrong section of the standard in your patch. The
>> >> > relevant
>> >> > wording is [dcl.stc]p1: "If a storage-class-specifier appears in a
>> >> > decl-specifier-seq, [...] the init-declarator-list of the declaration
>> >> > shall
>> >> > not be empty [...]."
>> >>
>> >> Ah, good to know. I made it down to p9 because we were declaring a
>> >> type instead of a member variable, but that makes sense.
>> >>
>> >> > Do we really need to change the code here? Is the new diagnostic
>> >> > better
>> >> > than
>> >> > the old one?
>> >>
>> >> Old diagnostic: ext-warn "'mutable' is not permitted on a declaration
>> >> of a
>> >> type"
>> >> Proposed diagnostic: error "'mutable' can only be applied to member
>> >> variables"
>> >>
>> >> Honestly, either works for me in terms of wording.
>> >>
>> >> > Do we have a reason to promote this from ExtWarn to Error just
>> >> > for mutable and not for other storage class specifiers?
>> >>
>> >> I was going with error because the other mutable constraint violations
>> >> are also errors (including this existing one which I was reusing).
>> >> Also, it seems weird to treat this as an extension -- what does this
>> >> extension actually provide in terms of benefit for the mutable
>> >> keyword? What does a mutable type declaration even mean?
>> >
>> >
>> > It's weird that we allow this for other storage classes too. What does a
>> > static type declaration mean? =)
>> >
>> > No objection to the patch; I think I slightly prefer consistent
>> > treatment of
>> > 'mutable' over consistent treatment of storage classes on type
>> > declarations.
>> > But I wonder why we ExtWarn rather than Error'ing here in the other
>> > cases,
>> > and I'm slightly cautious about changing this without knowing the
>> > history.
>>
>> I'm uncertain of the history, but for context:
>>
>> // C++ [dcl.stc]p1:
>> //   If a storage-class-specifier appears in a decl-specifier-seq, [...]
>> the
>> //   init-declarator-list of the declaration shall not be empty.
>> // C++ [dcl.fct.spec]p1:
>> //   If a cv-qualifier appears in a decl-specifier-seq, the
>> //   init-declarator-list of the declaration shall not be empty.
>> //
>> // Spurious qualifiers here appear to be valid in C.
>> unsigned DiagID = diag::warn_standalone_specifier;
>> if (getLangOpts().CPlusPlus)
>>   DiagID = diag::ext_standalone_specifier;
>>
>> So I think we treat it as an extension because of C. But that makes no
>> sense for mutable anyway, which is why I am overriding that in my
>> patch and turning it into an error.
>>
>> I've attached a revised patch that updates the comments to reflect my
>> understanding of why this is an error instead of an ExtWarn.
>
> OK, LGTM.
>
> By the same argument, we can probably reject any storage class on a member
> declaration.

Thanks! Applied in r209635. I'll look into the member declaration one
when I have the chance -- I mostly wanted this one to get in so the
entire test wasn't XFAILed. ;-)

~Aaron