On Fri, 2017-07-21 at 11:56 -0400, David Malcolm wrote:
> On Thu, 2017-07-20 at 18:35 +0200, Volker Reichelt wrote:
> > Hi,
> >
> > the following patch introduces a new C++ warning option
> > -Wduplicated-access-specifiers that warns about redundant
> > access-specifiers in classes, e.g.
> >
> > class B
> > {
> > public:
> > B();
> >
> > private:
> > void foo();
> > private:
> > int i;
> > };
> >
> > test.cc:8:5: warning: duplicate 'private' access-specifier [
> > -Wduplicated-access-specifiers]
> > private:
> > ^~~~~~~
> > -------
> > test.cc:6:5: note: access-specifier was previously given here
> > private:
> > ^~~~~~~
>
> Thanks for working on this.
>
> I'm not able to formally review, but you did CC me; various comments
> below throughout.
>
> > The test is implemented by tracking the location of the last
> > access-specifier together with the access-specifier itself.
> > The location serves two purposes: the obvious one is to be able to
> > print the location in the note that comes with the warning.
> > The second purpose is to be able to distinguish an access-specifier
> > given by the user from the default of the class type (i.e. public
> > for
> > 'struct' and private for 'class') where the location is set to
> > UNKNOWN_LOCATION. The warning is only issued if the user gives the
> > access-specifier twice, i.e. if the previous location is not
> > UNKNOWN_LOCATION.
>
> Presumably given
>
> struct foo
> {
> public:
> /* ... *
> };
>
> we could issue something like:
>
> warning: access-specifier 'public' is redundant within 'struct'
>
> for the first; similarly for:
>
> class bar
> {
> private:
> };
>
> we could issue:
>
> warning: access-specifier 'private' is redundant within 'class'
>
>
> > One could actually make this a two-level warning so that on the
> > higher level also the default class-type settings are taken into
> > account. Would that be helpful? I could prepare a second patch for
> > that.
>
> I'm not sure how best to structure it.
Maybe combine the two ideas, and call it
-Wredundant-access-specifiers
?
Or just "-Waccess-specifiers" ?
[...snip...]
Dave
