Re: [patch] PR c++/26151: duplicate 'duplicate' diagnostic

[Mark Mitchell <mark at codesourcery dot com>]

Volker Reichelt wrote:

> How about the following then? The patch
> * moves the whole loop that checks for duplicate decl-specifiers
>   from grokdeclarator to the parser, with two exceptions:
>   - count is used instead of decl_specs->specs[(int)ds] in one more place,
>   - the setting of longlong must stay in grokdeclarator, but is moved to
>     a more appropriate place,
> * gets rid of the extra test for 'friend' which caused duplicate
>   error message (without breaking g++.dg/parse/friend3.C).
> 
> The first bullet is a good thing on its own, IMHO.
> Maybe we can move some more sanity tests (e.g. usage of virtual together
> with static) to the parser.

Yes, that should be possible.  My feeling is that consistency checks
like these should be performed as early as possible.  Since
decl-specifiers are always parsed as a complete sequence, we can know
about invalid combinations (like the one you mention) right then, which
I think makes for more intuitive code and avoids the risk of duplicate
messages.

> 	PR c++/26151
> 	* parser.c (cp_parser_decl_specifier_seq): Check for duplicate
> 	decl-specifiers.  Remove extra check for duplicate 'friend'.
> 	* decl.c (grokdeclarator): Remove check for duplicate
> 	decl-specifiers.  Set longlong together with long_p.

This is OK -- but we could make it even more obvious.

Instead of doing things like:

  decl_specs->specs[(int) ds_friend]++;

we could do:

  cp_parser_note_decl_spec(decl_specs, ds_friend);

That function would then do something like:

  if (decl_specs->specs[(int) ds])
    error ("duplicate specifier ...");
  else
    ++decl_specs->specs[(int) ds];

with an appropriate check for "long long".

I don't think you should feel compelled to do that, though; your change
as is is a definite improvement.

> +  longlong = declspecs->specs[(int)ds_long] == 2;

Please use ">= 2".  That way "long long long" will be treated "long
long" for error recovery purposes, whch seems better than just plain "long".

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713