[PING#3][PATCH][RFC] Warning for C++ malformed typedef [PR c++/23689]

[PING#3][PATCH][RFC] Warning for C++ malformed typedef [PR c++/23689]

[Simon Baldwin]

The following change remains unreviewed and un-commented-on:

http://gcc.gnu.org/ml/gcc-patches/2007-02/msg00114.html

It's a two line change, accompanied by a tiny bit of test code.

Please tell me who I need to sleep with to get a review on this :-)  Thanks.

Re: [PING#3][PATCH][RFC] Warning for C++ malformed typedef [PR c++/23689]

[Mike Stump]

On Feb 27, 2007, at 2:04 PM, Simon Baldwin wrote:
> The following change remains unreviewed and un-commented-on:
>
> http://gcc.gnu.org/ml/gcc-patches/2007-02/msg00114.html
>
> It's a two line change, accompanied by a tiny bit of test code.
>
> Please tell me who I need to sleep with to get a review on this :-)   
> Thanks.

Gosh, I'd hate to think we need to sell our bodies to get a code  
review...  I'll cc a couple of relevant folks that can approve your  
change, as maybe they missed it the first couple of time it was sent  
to the list.

Thanks for being patient.

Re: [PING#3][PATCH][RFC] Warning for C++ malformed typedef [PR c++/23689]

[Nathan Sidwell]

Simon Baldwin wrote:
> The following change remains unreviewed and un-commented-on:
> 
> http://gcc.gnu.org/ml/gcc-patches/2007-02/msg00114.html
> 
> It's a two line change, accompanied by a tiny bit of test code.

The patch is ok, but can you check the error message.  IIRC the ' chars should 
be %< and %> or similar.  ok with that change, if it's needed.

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

Re: [PING#3][PATCH][RFC] Warning for C++ malformed typedef [PR c++/23689]

[Manuel López-Ibáñez]

On 28/02/07, Nathan Sidwell <nathan@codesourcery.com> wrote:
> Simon Baldwin wrote:
> > The following change remains unreviewed and un-commented-on:
> >
> > http://gcc.gnu.org/ml/gcc-patches/2007-02/msg00114.html
> >
> > It's a two line change, accompanied by a tiny bit of test code.
>
> The patch is ok, but can you check the error message.  IIRC the ' chars should
> be %< and %> or similar.  ok with that change, if it's needed.
>

In short,

- 	warning (0, "'typedef' was ignored in this declaration");
+     warning (0, "%<typedef%> was ignored in this declaration");

Also, Simon says:

"Two of the existing
tests also generate the warning: in typedef2, the warning is not noted by
dejagnu because of an error at the same line;"

This is likely because DejaGNU is too greedy right now, so the
dg-error matches both messages. This is a bug that hopefully will be
fixed someday, so we should not rely on this behaviour to lazily avoid
adding the correct dg-* options. So, please, try to match each message
with the appropriate dg-error / dg-warning. That is, if there is an
error and a warning, then there should be a dg-error and a dg-warning.

I am not a maintainer, so unless a maintainer agrees with me, you may
ignore my comments.

Cheers,

Manuel.

Re: [PING#3][PATCH][RFC] Warning for C++ malformed typedef [PR c++/23689]

[Simon Baldwin]

[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]

Manuel López-Ibáñez wrote:

> On 28/02/07, Nathan Sidwell <nathan@codesourcery.com> wrote:
>
>> Simon Baldwin wrote:
>> > The following change remains unreviewed and un-commented-on:
>> >
>> > http://gcc.gnu.org/ml/gcc-patches/2007-02/msg00114.html
>> >
>> > It's a two line change, accompanied by a tiny bit of test code.
>>
>> The patch is ok, but can you check the error message.  IIRC the ' 
>> chars should
>> be %< and %> or similar.  ok with that change, if it's needed.
>>
>
> In short,
>
> -     warning (0, "'typedef' was ignored in this declaration");
> +     warning (0, "%<typedef%> was ignored in this declaration");


Done.  Thanks for the correction; revised patch attached.

> Also, Simon says:
>
> "Two of the existing
> tests also generate the warning: in typedef2, the warning is not noted by
> dejagnu because of an error at the same line;"
>
> This is likely because DejaGNU is too greedy right now, so the
> dg-error matches both messages. This is a bug that hopefully will be
> fixed someday, so we should not rely on this behaviour to lazily avoid
> adding the correct dg-* options. So, please, try to match each message
> with the appropriate dg-error / dg-warning. That is, if there is an
> error and a warning, then there should be a dg-error and a dg-warning.
>
> I am not a maintainer, so unless a maintainer agrees with me, you may
> ignore my comments.


Thanks for the note.  Because DejaGNU seems to be limited to one 
dg-something per line, it's a little messy to handle, but it's not terrible.

I've modified the testsuite part of the patch slightly to filter typedef 
warnings from existing tests a little better than in the original 
version.  Please take a look at the revised patch, attached, and see if 
you think it'll be a bit more robust in the face of potential DejaGNU fixes.

--S


[-- Attachment #2: pr23689.patch --]
[-- Type: text/x-patch, Size: 4257 bytes --]

Attached is a proposed small patch to address PR c++/23689.  It adds a warning
to g++ for what are either malformed typedef's or ordinary declarations with a
useless prepended "typedef" qualifier.

The warning is unconditional; that is, it is printed no matter what warning
controls are selected by command line options.  This matches the behavior of
gcc under similar circumstances, although the text of the warning message in
g++ differs from gcc's -- this warning tries to be a little more explicit.

There is a new test in the testsuite for this warning.  Two of the existing
tests also generate the warning: template/typedef2 and init/ctor8.  Both these
tests are modified slightly to filter out the new warning, allowing them to
continue to pass.


2007-02-01  Simon Baldwin <simonb@google.com>

	PR c++/23689
	* decl.c (check_tag_decl): Added new warning for typedef ignored
	when it precedes an otherwise valid non-typedef declaration.

2007-02-01  Simon Baldwin <simonb@google.com>

	PR c++/23689
	* warn/ignored_typedef.C: New.
	* init/ctor8.C: Added dg-warning to consume ignored typedef warning.
	* template/typedef2.C: Ditto.  Moved dg-error onto its own line and
	added an explicit line number.


diff -Nrpc3 gcc-4.3-20070126_orig/gcc/cp/decl.c gcc-4.3-20070126/gcc/cp/decl.c
*** gcc-4.3-20070126_orig/gcc/cp/decl.c	Tue Jan 23 14:33:51 2007
--- gcc-4.3-20070126/gcc/cp/decl.c	Wed Feb 28 10:51:59 2007
*************** check_tag_decl (cp_decl_specifier_seq *d
*** 3694,3699 ****
--- 3694,3701 ----
  	       || declspecs->specs[(int)ds_thread])
  	error ("qualifiers can only be specified for objects "
  	       "and functions");
+       else if (saw_typedef)
+ 	warning (0, "%<typedef%> was ignored in this declaration");
      }
  
    return declared_type;
diff -Nrpc3 gcc-4.3-20070126_orig/gcc/testsuite/g++.dg/init/ctor8.C gcc-4.3-20070126/gcc/testsuite/g++.dg/init/ctor8.C
*** gcc-4.3-20070126_orig/gcc/testsuite/g++.dg/init/ctor8.C	Tue Oct 17 15:35:29 2006
--- gcc-4.3-20070126/gcc/testsuite/g++.dg/init/ctor8.C	Wed Feb 28 15:48:28 2007
***************
*** 2,11 ****
  
  typedef struct S { // { dg-error "reference" }
    int &r; 
! };
  
  S f () {
    return S (); // { dg-error "synthesized" }
  }
- 
- 
--- 2,9 ----
  
  typedef struct S { // { dg-error "reference" }
    int &r; 
! }; // { dg-warning "'typedef' was ignored" }
  
  S f () {
    return S (); // { dg-error "synthesized" }
  }
diff -Nrpc3 gcc-4.3-20070126_orig/gcc/testsuite/g++.dg/template/typedef2.C gcc-4.3-20070126/gcc/testsuite/g++.dg/template/typedef2.C
*** gcc-4.3-20070126_orig/gcc/testsuite/g++.dg/template/typedef2.C	Tue Nov  2 18:48:44 2004
--- gcc-4.3-20070126/gcc/testsuite/g++.dg/template/typedef2.C	Wed Feb 28 15:48:15 2007
***************
*** 1,3 ****
  // PR c++/18155
  
! template<int> typedef struct A; // { dg-error "" }
--- 1,4 ----
  // PR c++/18155
  
! template<int> typedef struct A; // { dg-warning "'typedef' was ignored" }
!                                 // { dg-error "" "" { target *-*-* } 3 }
diff -Nrpc3 gcc-4.3-20070126_orig/gcc/testsuite/g++.dg/warn/ignored_typedef.C gcc-4.3-20070126/gcc/testsuite/g++.dg/warn/ignored_typedef.C
*** gcc-4.3-20070126_orig/gcc/testsuite/g++.dg/warn/ignored_typedef.C	Wed Dec 31 16:00:00 1969
--- gcc-4.3-20070126/gcc/testsuite/g++.dg/warn/ignored_typedef.C	Wed Feb 28 10:47:46 2007
***************
*** 0 ****
--- 1,22 ----
+ // PR c++/23689
+ // Test that malformed typedef's produce a compiler warning.
+ 
+ typedef char valid_0;
+ typedef int valid_1;
+ typedef long valid_2;
+ typedef float valid_3;
+ typedef double valid_4;
+ typedef unsigned valid_5;
+ typedef int *valid_6;
+ typedef struct valid_7 {} valid_8;
+ typedef struct {} valid_9;
+ typedef int temp_0; typedef temp_0 valid_10;
+ struct temp_1 {}; typedef temp_1 valid_11;
+ union temp_2 {}; typedef temp_2 valid_12;
+ typedef void (*valid_13) (int); 
+ 
+ typedef struct invalid_0 {};  // { dg-warning "'typedef' was ignored" }
+ typedef class invalid_1 {};  // { dg-warning "'typedef' was ignored" }
+ typedef enum invalid_2 { INVALID_2 };  // { dg-warning "'typedef' was ignored" }
+ typedef enum { INVALID_3 };  // { dg-warning "'typedef' was ignored" }
+ typedef union invalid_4 {};  // { dg-warning "'typedef' was ignored" }

Re: [PING#3][PATCH][RFC] Warning for C++ malformed typedef [PR c++/23689]

[Manuel López-Ibáñez]

On 01/03/07, Simon Baldwin <simonb@google.com> wrote:
>
> Thanks for the note.  Because DejaGNU seems to be limited to one
> dg-something per line, it's a little messy to handle, but it's not terrible.

Don't be shy, you can say it: It is ugly. ;-)

> I've modified the testsuite part of the patch slightly to filter typedef
> warnings from existing tests a little better than in the original
> version.  Please take a look at the revised patch, attached, and see if
> you think it'll be a bit more robust in the face of potential DejaGNU fixes.

If typedef2.C and the other testcases pass with those changes, I think
they are OK.

Since the patch was already approved, I believe you don't need further approval.

Thanks for implementing my suggestion.

Cheers,

Manuel.

Re: [PING#3][PATCH][RFC] Warning for C++ malformed typedef [PR c++/23689]

[Nathan Sidwell]

Simon Baldwin wrote:

> I've modified the testsuite part of the patch slightly to filter typedef 
> warnings from existing tests a little better than in the original 
> version.  Please take a look at the revised patch, attached, and see if 
> you think it'll be a bit more robust in the face of potential DejaGNU 
> fixes.

for avoidance of doubt, the revised patch is ok too :)

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk