Question about existing tests for [dcl.stc]

[Richard Smith]

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140523/81585f8c/attachment.html>