Re: Changes in C++ FE regarding pedwarns to be errors are harmful

["Jonathan Wakely" <jwakely.gcc@gmail.com>]

On 13/01/2008, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> On 12/01/2008, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> > On 12/01/2008, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> > >
> > > Here is an initial patch implementing some of your proposals. I used
> > > pederror as the name of the function. That is, it is an error unless
> > > fpermissive is given.
> >
> > These errors should be independent of -pedantic* unless the if
> > (pedantic) check is also present in the code, but that's a special
> > case.  Have I understood the intention correctly?
>
> Actually, I didn't understand that we wanted to separate fpermissive
> and pedantic-errors completely. (Notice that the internal
> declaration/definition of fpermissive actually mentions pedantic.) But
> I agree with you: they should be unrelated.

I hadn't noticed that in the flag_permissive declaration, thanks for
pointing it out.  I think that comment should be changed to refer to
permerrors instead.  (I'm not sure about the bit about being ignored
if -pedantic is given ... is that true today?

To summarise, the proposal is

Introduce a new class of diagnostic, permerror, which is an error by
default, but -fpermissive makes it a warning.
Change most C++ FE pedwarns to permerrors
Stop using pedantic-errors by default for the C++ FE and CPP

Since most pedwarns are now permerrors, we still get errors by
default, or warnings with -fpermissive. However, now CPP doesn't have
to use pedantic-errors to align with the C++ FE, so the class of CPP
diagnostics that are causing problems will remain as warnings, as per
GCC 4.2
If you want errors from the preprocessor you can use
-fpedantic-errors, consistent with C.

I think this results in consistent behaviour for the front-end and
preprocessor for both C and C++ (which is the behaviour documented in
the manual.)  Most diagnostics will behave equivalently but there are
a few special cases where the C pedwarn semantics are preferable, so
shouldn't be switched to permerrors (most of these cases are only
enabled by -pedantic and give no diagnostic without)

> Your information above was very helpful. But I am still not sure about
> the rest of pedwarns.

for decl.c I'm not so sure which category some of them should be in.
I'll list the ones I think should stay as pedwarns, everything else
should be a permerror (IMHO)

line 3811 check_tag_decl
      if (TREE_CODE (declared_type) != UNION_TYPE && pedantic
	  && !in_system_header)
	pedwarn ("ISO C++ prohibits anonymous structs");
guarded by pedantic, looks like it's allowed as an extension, so leave
as a pedwarn.
This won't change the default behaviour but now when -pedantic is
given this will only be a warning

line 6922 check_static_variable_definition
  else if (pedantic && !INTEGRAL_TYPE_P (type))
    pedwarn ("ISO C++ forbids initialization of member constant "
	     "%qD of non-integral type %qT", decl, type);
Extension, guarded by pedantic, leave it as pedwarn

line 7007 compute_array_index_type
      /* As an extension we allow zero-sized arrays.  We always allow
	 them in system headers because glibc uses them.  */
      else if (integer_zerop (size) && pedantic && !in_system_header)
	{
	  if (name)
	    pedwarn ("ISO C++ forbids zero-size array %qD", name);
	  else
	    pedwarn ("ISO C++ forbids zero-size array");
guarded by pedantic, leave as pedwarn

line 7025 compute_array_index_type
  else if (pedantic && warn_vla != 0)
    {
      if (name)
	pedwarn ("ISO C++ forbids variable length array %qD", name);
      else
	pedwarn ("ISO C++ forbids variable length array");
    }
GNU extension, guarded by pedantic, leave as pedwarn

7655  grokdeclarator
  else if (pedantic || ! is_main)
    pedwarn ("ISO C++ forbids declaration of %qs with no type", name);
  else
    warning (OPT_Wreturn_type,
           "ISO C++ forbids declaration of %qs with no type", name);

interesting one, the comment above says:

      /* We handle `main' specially here, because 'main () { }' is so
	 common.  With no options, it is allowed.  With -Wreturn-type,
	 it is a warning.  It is only an error with -pedantic-errors.  */

That's true, but currently pedantic-errors is on by default.
I think this should become:

  else if (! is_main)
    permerror ("ISO C++ forbids declaration of %qs with no type", name);
  else if (pedantic)
    pedwarn ("ISO C++ forbids declaration of %qs with no type", name);
  else
    warning (OPT_Wreturn_type,
         "ISO C++ forbids declaration of %qs with no type", name);

That would make it an error anywhere except main, but in main it's a
warning with -pedantic or -Wreturn-type.  -pedantic-errors or -Werror
make it an error on main too.

7703 grokdeclarator
    pedwarn ("long, short, signed or unsigned used invalidly for %qs",
       name);
guarded by pedantic, leave as pedwarn

7808 grokdeclarator
      /* This was an error in C++98 (cv-qualifiers cannot be added to
	 a function type), but DR 295 makes the code well-formed by
	 dropping the extra qualifiers. */
      if (pedantic)
	{
	  tree bad_type = build_qualified_type (type, type_quals);
	  pedwarn ("ignoring %qV qualifiers added to function type %qT",
		   bad_type, type);
	}

This is currently allowed by default, but an error with -pedantic,
even though it's well-formed!  Stay as pedwarn so it's only a warning
with -pedantic

9053 grokdeclarator
guarded by pedantic, leave both as pedwarns

9146 grokdeclarator
guarded by pedantic, leave as pedwarn (but change the one above to permerror)

10013 grok_op_properties
guarded by pedantic, leave as pedwarn


I think all others should change to permerrors.


Jon