diff mbox

restore pedantic warning on flexible array members (c++/71290)

Message ID e9f12189-a4a6-6885-97c5-2d03ab74454f@gmail.com
State New
Headers show

Commit Message

Martin Sebor Jan. 25, 2017, 5:02 p.m. UTC
The improvements to the handling of flexible array members in
C++ in GCC 6 inadvertently removed the pedantic warnings GCC
used to issue for their declarations.  The attached patch
restores it.

Martin

Comments

Jason Merrill Jan. 25, 2017, 5:44 p.m. UTC | #1
OK.

On Wed, Jan 25, 2017 at 12:02 PM, Martin Sebor <msebor@gmail.com> wrote:
> The improvements to the handling of flexible array members in
> C++ in GCC 6 inadvertently removed the pedantic warnings GCC
> used to issue for their declarations.  The attached patch
> restores it.
>
> Martin
Jakub Jelinek Jan. 25, 2017, 11:53 p.m. UTC | #2
On Wed, Jan 25, 2017 at 10:02:23AM -0700, Martin Sebor wrote:
> --- gcc/cp/decl.c	(revision 244844)
> +++ gcc/cp/decl.c	(working copy)
> @@ -11798,6 +11798,17 @@ grokdeclarator (const cp_declarator *declarator,
>  	      }
>  	    else 
>  	      {
> +		/* Array is a flexible member.  */
> +		if (in_system_header_at (input_location))
> +		  /* Do not warn flexible them in system headers because glibc
> +		     uses them.  */;

The comment is weird.  Did you mean warn about them, or warn about
flexible array members or something similar?

	Jakub
Martin Sebor Jan. 26, 2017, 12:02 a.m. UTC | #3
On 01/25/2017 04:53 PM, Jakub Jelinek wrote:
> On Wed, Jan 25, 2017 at 10:02:23AM -0700, Martin Sebor wrote:
>> --- gcc/cp/decl.c	(revision 244844)
>> +++ gcc/cp/decl.c	(working copy)
>> @@ -11798,6 +11798,17 @@ grokdeclarator (const cp_declarator *declarator,
>>  	      }
>>  	    else
>>  	      {
>> +		/* Array is a flexible member.  */
>> +		if (in_system_header_at (input_location))
>> +		  /* Do not warn flexible them in system headers because glibc
>> +		     uses them.  */;
>
> The comment is weird.  Did you mean warn about them, or warn about
> flexible array members or something similar?

It sure is.  I must have mangled it while copying the whole block
from the 5 branch.  I just fixed it.  Thanks for pointing it out!

Martin
Rainer Orth Jan. 27, 2017, 9:45 a.m. UTC | #4
Martin Sebor <msebor@gmail.com> writes:

> The improvements to the handling of flexible array members in
> C++ in GCC 6 inadvertently removed the pedantic warnings GCC
> used to issue for their declarations.  The attached patch
> restores it.

After this patch, I get

FAIL: obj-c++.dg/property/at-property-23.mm -fgnu-runtime (test for excess erro
rs)

on 32 and 64-bit Solaris, sparc and x86:

Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/obj-c++.dg/property/at-property-23.mm:16:17: error: ISO C++ forbids flexible array member 'c' [-Wpedantic]

	Rainer
Martin Sebor Jan. 27, 2017, 4:56 p.m. UTC | #5
On 01/27/2017 02:45 AM, Rainer Orth wrote:
> Martin Sebor <msebor@gmail.com> writes:
>
>> The improvements to the handling of flexible array members in
>> C++ in GCC 6 inadvertently removed the pedantic warnings GCC
>> used to issue for their declarations.  The attached patch
>> restores it.
>
> After this patch, I get
>
> FAIL: obj-c++.dg/property/at-property-23.mm -fgnu-runtime (test for excess erro
> rs)
>
> on 32 and 64-bit Solaris, sparc and x86:
>
> Excess errors:
> /vol/gcc/src/hg/trunk/local/gcc/testsuite/obj-c++.dg/property/at-property-23.mm:16:17: error: ISO C++ forbids flexible array member 'c' [-Wpedantic]

I saw this yesterday even on x86_64 but didn't have a chance to look
into what was causing the failure.  The commit adjusts the test to
expect the excess error so I'm not sure why it's not being handled...

Ah.  I now see where the problem is.  I committed r244990 with a fix.

The dg-error directive I added ended split over two lines as I copied
and pasted it between two windows:

/* { dg-error "forbids flexible array member" "flexible array member" { 
target \
*-*-* }  } */

Because the backslash is in column 79 it looks the same as the backslash
Emacs uses to show lines that are longer than that.  So while it worked
fine on the machine where it was tested it and looked fine on the one
I copied it into it got broken in transit.  I normally retest changes
just before committing them on the same machine but I don't do a full
bootstrap.  I just do stage1-bubble and run a subset of tests.  But
Objective C++ doesn't build with stage1-bubble and I don't
know/remember what all I need to build it to run Objective C++ tests.

FWIW, I could avoid this awkward setup if I had just one repo for all
my changes, shared across my various machines.  Unfortunately, for it
to really work I would also need to be able to commit using Git rather
than Subversion which doesn't work.  At least not according to the
instructions on the Wiki.

Martin
Andreas Schwab Jan. 27, 2017, 5:42 p.m. UTC | #6
On Jan 27 2017, Martin Sebor <msebor@gmail.com> wrote:

> FWIW, I could avoid this awkward setup if I had just one repo for all
> my changes, shared across my various machines.  Unfortunately, for it
> to really work I would also need to be able to commit using Git rather
> than Subversion which doesn't work.  At least not according to the
> instructions on the Wiki.

To transfer a commit from a git worktree to an svn checkout, you can use
`git -C $git_worktree format-patch -1 --stdout | patch -p1'.

Andreas.
Jason Merrill Jan. 27, 2017, 6:20 p.m. UTC | #7
On Fri, Jan 27, 2017 at 11:56 AM, Martin Sebor <msebor@gmail.com> wrote:
> FWIW, I could avoid this awkward setup if I had just one repo for all
> my changes, shared across my various machines.  Unfortunately, for it
> to really work I would also need to be able to commit using Git rather
> than Subversion which doesn't work.  At least not according to the
> instructions on the Wiki.

What's the problem?

Jason
Jeff Law Jan. 27, 2017, 7:44 p.m. UTC | #8
On 01/27/2017 11:20 AM, Jason Merrill wrote:
> On Fri, Jan 27, 2017 at 11:56 AM, Martin Sebor <msebor@gmail.com> wrote:
>> FWIW, I could avoid this awkward setup if I had just one repo for all
>> my changes, shared across my various machines.  Unfortunately, for it
>> to really work I would also need to be able to commit using Git rather
>> than Subversion which doesn't work.  At least not according to the
>> instructions on the Wiki.
>
> What's the problem?
Yea, what exactly is the problem?

I commit via GIT using the instructions on the web page.

jeff
Martin Sebor Jan. 27, 2017, 9:28 p.m. UTC | #9
On 01/27/2017 11:20 AM, Jason Merrill wrote:
> On Fri, Jan 27, 2017 at 11:56 AM, Martin Sebor <msebor@gmail.com> wrote:
>> FWIW, I could avoid this awkward setup if I had just one repo for all
>> my changes, shared across my various machines.  Unfortunately, for it
>> to really work I would also need to be able to commit using Git rather
>> than Subversion which doesn't work.  At least not according to the
>> instructions on the Wiki.
>
> What's the problem?

We discussed it in the thread below:

   https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00335.html

I think Jonathan and/or Ville had some suggestions on IRC for how
to work around the broken setup.  I never got it to work but I also
didn't try too hard, thinking the Git conversion would be done soon.

I don't remember the errors I was getting then but I just tried to
follow the Wiki steps again.  On tor git svn init fails like so:

$ git svn init -s --prefix=svn/ svn+ssh://msebor@gcc.gnu.org/svn/gcc
Network connection closed unexpectedly: Unable to connect to a 
repository at URL 'svn+ssh://msebor@gcc.gnu.org/svn/gcc': To better 
debug SSH connection problems, remove the -q option from 'ssh' in the 
[tunnels] section of your Subversion configuration file. at 
/usr/share/perl5/vendor_perl/Git/SVN.pm line 310.

(Removing the -q option from the [tunne;s] section of my
~/.subversion/config file has no effect.)

On my machine, git svn init and the rest of the steps succeeded (as
they did before) but commit failed (as it did before, though possibly
with a different error). The one I get now is:

$ git svn dcommit
Cannot dcommit with a dirty index.  Commit your changes first, or stash 
them with `git stash'.
  at /usr/libexec/git-core/git-svn line 836.

I did a quick search online for the error.  I see others as confused
by it as I am and various suggestions to run various commands none
of which works.

I understand you can commit from your existing repo but does following
all the steps as I did work for you?  How about on tor?

Martin
Jeff Law Jan. 28, 2017, 12:01 a.m. UTC | #10
On 01/27/2017 02:28 PM, Martin Sebor wrote:
> On 01/27/2017 11:20 AM, Jason Merrill wrote:
>> On Fri, Jan 27, 2017 at 11:56 AM, Martin Sebor <msebor@gmail.com> wrote:
>>> FWIW, I could avoid this awkward setup if I had just one repo for all
>>> my changes, shared across my various machines.  Unfortunately, for it
>>> to really work I would also need to be able to commit using Git rather
>>> than Subversion which doesn't work.  At least not according to the
>>> instructions on the Wiki.
>>
>> What's the problem?
>
> We discussed it in the thread below:
>
>   https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00335.html
>
> I think Jonathan and/or Ville had some suggestions on IRC for how
> to work around the broken setup.  I never got it to work but I also
> didn't try too hard, thinking the Git conversion would be done soon.
>
> I don't remember the errors I was getting then but I just tried to
> follow the Wiki steps again.  On tor git svn init fails like so:
>
> $ git svn init -s --prefix=svn/ svn+ssh://msebor@gcc.gnu.org/svn/gcc
> Network connection closed unexpectedly: Unable to connect to a
> repository at URL 'svn+ssh://msebor@gcc.gnu.org/svn/gcc': To better
> debug SSH connection problems, remove the -q option from 'ssh' in the
> [tunnels] section of your Subversion configuration file. at
> /usr/share/perl5/vendor_perl/Git/SVN.pm line 310.
>
> (Removing the -q option from the [tunne;s] section of my
> ~/.subversion/config file has no effect.)
>
> On my machine, git svn init and the rest of the steps succeeded (as
> they did before) but commit failed (as it did before, though possibly
> with a different error). The one I get now is:
>
> $ git svn dcommit
> Cannot dcommit with a dirty index.  Commit your changes first, or stash
> them with `git stash'.
>  at /usr/libexec/git-core/git-svn line 836.
>
> I did a quick search online for the error.  I see others as confused
> by it as I am and various suggestions to run various commands none
> of which works.
>
> I understand you can commit from your existing repo but does following
> all the steps as I did work for you?  How about on tor?
So I would start with a checkout of a git tree, then use the 
instructions to turn that into a SVN-writable tree.  Those have worked 
flawless for me in the past.

You can also do something like
ssh -v msebor@gcc.gnu.org ls

To see if anything useful comes up.

THe dirty index means you've got local changes that are not committed. 
What does a "git status" report?

jeff
>
> Martin
Martin Sebor Jan. 28, 2017, 12:32 a.m. UTC | #11
On 01/27/2017 05:01 PM, Jeff Law wrote:
> On 01/27/2017 02:28 PM, Martin Sebor wrote:
>> On 01/27/2017 11:20 AM, Jason Merrill wrote:
>>> On Fri, Jan 27, 2017 at 11:56 AM, Martin Sebor <msebor@gmail.com> wrote:
>>>> FWIW, I could avoid this awkward setup if I had just one repo for all
>>>> my changes, shared across my various machines.  Unfortunately, for it
>>>> to really work I would also need to be able to commit using Git rather
>>>> than Subversion which doesn't work.  At least not according to the
>>>> instructions on the Wiki.
>>>
>>> What's the problem?
>>
>> We discussed it in the thread below:
>>
>>   https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00335.html
>>
>> I think Jonathan and/or Ville had some suggestions on IRC for how
>> to work around the broken setup.  I never got it to work but I also
>> didn't try too hard, thinking the Git conversion would be done soon.
>>
>> I don't remember the errors I was getting then but I just tried to
>> follow the Wiki steps again.  On tor git svn init fails like so:
>>
>> $ git svn init -s --prefix=svn/ svn+ssh://msebor@gcc.gnu.org/svn/gcc
>> Network connection closed unexpectedly: Unable to connect to a
>> repository at URL 'svn+ssh://msebor@gcc.gnu.org/svn/gcc': To better
>> debug SSH connection problems, remove the -q option from 'ssh' in the
>> [tunnels] section of your Subversion configuration file. at
>> /usr/share/perl5/vendor_perl/Git/SVN.pm line 310.
>>
>> (Removing the -q option from the [tunne;s] section of my
>> ~/.subversion/config file has no effect.)
>>
>> On my machine, git svn init and the rest of the steps succeeded (as
>> they did before) but commit failed (as it did before, though possibly
>> with a different error). The one I get now is:
>>
>> $ git svn dcommit
>> Cannot dcommit with a dirty index.  Commit your changes first, or stash
>> them with `git stash'.
>>  at /usr/libexec/git-core/git-svn line 836.
>>
>> I did a quick search online for the error.  I see others as confused
>> by it as I am and various suggestions to run various commands none
>> of which works.
>>
>> I understand you can commit from your existing repo but does following
>> all the steps as I did work for you?  How about on tor?
> So I would start with a checkout of a git tree, then use the
> instructions to turn that into a SVN-writable tree.  Those have worked
> flawless for me in the past.
>
> You can also do something like
> ssh -v msebor@gcc.gnu.org ls
>
> To see if anything useful comes up.
>
> THe dirty index means you've got local changes that are not committed.
> What does a "git status" report?

Aha!  That's the missing step from the Wiki!  Once I commit the changes
to my local repository git svn dcommit succeeds and pushes them to the
FSF repository.  Let me update the  Wiki to make this clear.

Thanks for the hint.

Martin
diff mbox

Patch

PR c++/71290 - [6/7 Regression] Flexible array member is not diagnosed with -pedantic

gcc/cp/ChangeLog:

	PR c++/71290
	* decl.c (grokdeclarator): Warn on flexible array members.

gcc/testsuite/ChangeLog:

	PR c++/71290
	* g++.dg/ext/flexarray-mangle-2.C: Adjust.
	* g++.dg/ext/flexarray-mangle.C: Adjust.
	* g++.dg/ext/flexarray-subst.C: Adjust.
	* g++.dg/ext/flexary10.C: Adjust.
	* g++.dg/ext/flexary11.C: Adjust.
	* g++.dg/ext/flexary14.C: Adjust.
	* g++.dg/ext/flexary16.C: Adjust.
	* g++.dg/ext/flexary18.C: Adjust.
	* g++.dg/ext/flexary19.C: Adjust.
	* g++.dg/ext/flexary7.C: Adjust.
	* g++.dg/ext/pr71290.C: New test.

Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 244844)
+++ gcc/cp/decl.c	(working copy)
@@ -11798,6 +11798,17 @@  grokdeclarator (const cp_declarator *declarator,
 	      }
 	    else 
 	      {
+		/* Array is a flexible member.  */
+		if (in_system_header_at (input_location))
+		  /* Do not warn flexible them in system headers because glibc
+		     uses them.  */;
+		else if (name)
+		  pedwarn (input_location, OPT_Wpedantic,
+			   "ISO C++ forbids flexible array member %<%s%>", name);
+		else
+		  pedwarn (input_location, OPT_Wpedantic,
+			   "ISO C++ forbids flexible array members");
+
 		/* Flexible array member has a null domain.  */
 		type = build_cplus_array_type (TREE_TYPE (type), NULL_TREE);
 	      }
Index: gcc/testsuite/g++.dg/ext/flexarray-mangle-2.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexarray-mangle-2.C	(revision 244844)
+++ gcc/testsuite/g++.dg/ext/flexarray-mangle-2.C	(working copy)
@@ -1,9 +1,10 @@ 
 // PR c++/69277 - [6 Regression] ICE mangling a flexible array member
 // { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wno-error=pedantic" }
 
 struct A {
   int n;
-  char a [];
+  char a[];   // { dg-warning "forbids flexible array member" }
 };
 
 // Declare but do not define function templates.
Index: gcc/testsuite/g++.dg/ext/flexarray-mangle.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexarray-mangle.C	(revision 244844)
+++ gcc/testsuite/g++.dg/ext/flexarray-mangle.C	(working copy)
@@ -1,9 +1,10 @@ 
 // PR c++/69277 - [6 Regression] ICE mangling a flexible array member
 // { dg-do compile }
+// { dg-additional-options "-Wno-error=pedantic" }
 
 struct A {
   int n;
-  char a [];
+  char a[];   // { dg-warning "forbids flexible array member" }
 };
 
 // Declare but do not define function templates.
Index: gcc/testsuite/g++.dg/ext/flexarray-subst.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexarray-subst.C	(revision 244844)
+++ gcc/testsuite/g++.dg/ext/flexarray-subst.C	(working copy)
@@ -1,8 +1,12 @@ 
 // PR c++/69251 - [6 Regression] ICE (segmentation fault) in unify_array_domain
 // on i686-linux-gnu
 // { dg-do compile }
+// { dg-additional-options "-Wno-error=pedantic" }
 
-struct A { int n; char a[]; };
+struct A {
+  int n;
+  char a[];   // { dg-warning "forbids flexible array member" }
+};
 
 template <class>
 struct B;
Index: gcc/testsuite/g++.dg/ext/flexary10.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexary10.C	(revision 244844)
+++ gcc/testsuite/g++.dg/ext/flexary10.C	(working copy)
@@ -4,7 +4,7 @@ 
 
 struct A {
   int n;
-  int a [];
+  int a[];  // { dg-warning "forbids flexible array member" }
 };
 
 struct A foo (void)
Index: gcc/testsuite/g++.dg/ext/flexary11.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexary11.C	(revision 244844)
+++ gcc/testsuite/g++.dg/ext/flexary11.C	(working copy)
@@ -4,7 +4,7 @@ 
 
 struct A {
   int n;
-  char a [];
+  char a[];   // { dg-error "forbids flexible array member" }
 };
 
 void f ()
Index: gcc/testsuite/g++.dg/ext/flexary14.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexary14.C	(revision 244844)
+++ gcc/testsuite/g++.dg/ext/flexary14.C	(working copy)
@@ -9,7 +9,9 @@  struct A<T[]> { typedef int X; };
 
 template <class T> int foo (T&, typename A<T>::X = 0) { return 0; }
 
-struct B { int n, a[]; };
+struct B {
+  int n, a[];     // { dg-error "forbids flexible array member" }
+};
 
 void bar (B *b)
 {
Index: gcc/testsuite/g++.dg/ext/flexary16.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexary16.C	(revision 244844)
+++ gcc/testsuite/g++.dg/ext/flexary16.C	(working copy)
@@ -1,6 +1,7 @@ 
 // PR c++/71147 - [6 Regression] Flexible array member wrongly rejected
 //   in template
 // { dg-do compile }
+// { dg-options "-Wpedantic -Wno-error=pedantic" }
 
 template <typename>
 struct container
@@ -11,7 +12,7 @@  struct container
 
   struct incomplete {
     int x;
-    elem array[];
+    elem array[];  // { dg-warning "forbids flexible array member" }
   };
 };
 
@@ -26,7 +27,7 @@  struct D: container<T>
 {
   struct S {
     int x;
-    typename container<T>::elem array[];
+    typename container<T>::elem array[];  // { dg-warning "forbids flexible array member" }
   };
 };
 
Index: gcc/testsuite/g++.dg/ext/flexary18.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexary18.C	(revision 244844)
+++ gcc/testsuite/g++.dg/ext/flexary18.C	(working copy)
@@ -211,3 +211,5 @@  struct StructUnion9 {                       // { d
     } b2;                                   // { dg-warning "invalid use" }
   } a2;                                     // { dg-message "next member" }
 };
+
+// { dg-prune-output "forbids flexible array member" }
Index: gcc/testsuite/g++.dg/ext/flexary19.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexary19.C	(revision 244844)
+++ gcc/testsuite/g++.dg/ext/flexary19.C	(working copy)
@@ -341,3 +341,4 @@  struct S35
   typedef D D2;
 };
 
+// { dg-prune-output "forbids flexible array member" }
Index: gcc/testsuite/g++.dg/ext/flexary7.C
===================================================================
--- gcc/testsuite/g++.dg/ext/flexary7.C	(revision 244844)
+++ gcc/testsuite/g++.dg/ext/flexary7.C	(working copy)
@@ -5,7 +5,7 @@ 
 
 struct FlexChar {
     int n;
-    char a[];
+    char a[];       // { dg-warning "forbids flexible array member" }
 };
 
 struct FlexChar ac =
@@ -18,7 +18,7 @@  typedef __WCHAR_TYPE__ wchar_t;
 
 struct FlexWchar {
     int n;
-    wchar_t a[];
+    wchar_t a[];    // { dg-warning "forbids flexible array member" }
 };
 
 struct FlexWchar awc =
@@ -27,7 +27,7 @@  struct FlexWchar awc =
 
 struct FlexInt {
     int n;
-    int a[];
+    int a[];        // { dg-warning "forbids flexible array member" }
 };
 
 // Verify that no warning is issued for the case when a flexible array
@@ -48,7 +48,7 @@  struct FlexInt ai2 =
 template <class T>
 struct FlexT {
     int n;
-    T a[];
+    T a[];          // { dg-warning "forbids flexible array member" }
 };
 
 struct FlexT<char> atc =
Index: gcc/testsuite/g++.dg/ext/pr71290.C
===================================================================
--- gcc/testsuite/g++.dg/ext/pr71290.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/pr71290.C	(working copy)
@@ -0,0 +1,17 @@ 
+// PR c++/71290 - [6/7 Regression] Flexible array member is not diagnosed
+// with -pedantic
+
+// { dg-do compile }s
+// { dg-options "-Wall -Wpedantic" }
+
+struct A
+{
+  int i;
+  int arr[];   // { dg-warning "forbids flexible array member .arr." }
+};
+
+template <class T>
+struct B {
+  T n;
+  T a[];       // { dg-warning "forbids flexible array member .a." }
+};