This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/2] Fix C++ side of PR c/70436 (missing -Wparentheses warnings)


On Thu, 31 Mar 2016, Jakub Jelinek wrote:

> On Thu, Mar 31, 2016 at 04:54:04PM -0400, Patrick Palka wrote:
> > I think that covers all the vanilla C++ constructs that this warning has
> > to consider.  As for C++ extensions, we still fail to warn for
> > 
> >   if (a)
> >   #pragma GCC ivdep
> >      while (1)
> >        if (b)
> >          bar ();
> >   else
> >     baz ();
> > 
> > and
> > 
> >   if (a)
> >     _Cilk_for (int i = 0; i < 10; i++)
> >       if (b)
> >         bar ();
> >   else
> >     baz ();
> > 
> > and probably other constructs.  I suppose support for this can be
> > implemented in a subsequent patch if deemed appropriate at this stage.
> > It would probably just involve more threading of the parameter IF_P.
> > 
> > Is this OK to commit after bootstrap + regtesting?
> 
> If this makes it in, I can take care of OpenMP, perhaps Cilk+ too.
> 
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -2104,7 +2104,7 @@ static tree cp_parser_selection_statement
> >  static tree cp_parser_condition
> >    (cp_parser *);
> >  static tree cp_parser_iteration_statement
> > -  (cp_parser *, bool);
> > +  (cp_parser *, bool *if_p, bool);
> 
> I wouldn't add a named argument where all others are unnamed in the
> prototype.
> 
> >  static bool cp_parser_for_init_statement
> >    (cp_parser *, tree *decl);
> >  static tree cp_parser_for
> > @@ -2127,7 +2127,7 @@ static void cp_parser_declaration_statement
> >  static tree cp_parser_implicitly_scoped_statement
> >    (cp_parser *, bool *, const token_indent_info &, vec<tree> * = NULL);
> >  static void cp_parser_already_scoped_statement
> > -  (cp_parser *, const token_indent_info &);
> > +  (cp_parser *, bool *if_p, const token_indent_info &);
> 
> Likewise here.
> 
> > +void
> > +foo (void)
> > +{
> > +  if (a) /* { dg-warning "ambiguous" }  */
> > +    for (;;)
> > +      if (b)
> > +        bar ();
> > +      else
> > +        baz ();
> 
> What about multiple nested for or while loops, like:
>   if (a)
>     for (i = 0; i < 10; i++)
>       for (j = 0; j < 10; j++)
>         if (b)
>           bar ();
>   else
>     baz ();
> and similarly for multiple nested while loops?
> I only see while (1) for (;;) in the test.
> 
> Otherwise, I'll defer to Jason.
> 
> 	Jakub
> 

Thanks, patch updated with your requested changes.  Here's an
incremental diff of the changes (since they are relatively minor):

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index af2a8f2..6dd6280 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -2104,7 +2104,7 @@ static tree cp_parser_selection_statement
 static tree cp_parser_condition
   (cp_parser *);
 static tree cp_parser_iteration_statement
-  (cp_parser *, bool *if_p, bool);
+  (cp_parser *, bool *, bool);
 static bool cp_parser_for_init_statement
   (cp_parser *, tree *decl);
 static tree cp_parser_for
@@ -2127,7 +2127,7 @@ static void cp_parser_declaration_statement
 static tree cp_parser_implicitly_scoped_statement
   (cp_parser *, bool *, const token_indent_info &, vec<tree> * = NULL);
 static void cp_parser_already_scoped_statement
-  (cp_parser *, bool *if_p, const token_indent_info &);
+  (cp_parser *, bool *, const token_indent_info &);
 
 /* Declarations [gram.dcl.dcl] */
 
diff --git a/gcc/testsuite/g++.dg/warn/Wparentheses-29.C b/gcc/testsuite/g++.dg/warn/Wparentheses-29.C
index 125e6b4..7832415 100644
--- a/gcc/testsuite/g++.dg/warn/Wparentheses-29.C
+++ b/gcc/testsuite/g++.dg/warn/Wparentheses-29.C
@@ -1,13 +1,15 @@
 /* PR c/70436  */
 /* { dg-options "-Wparentheses" }  */
 
-int a, b;
+int a, b, c;
 void bar (void);
 void baz (void);
 
 void
 foo (void)
 {
+  int i, j;
+
   if (a) /* { dg-warning "ambiguous" }  */
     for (;;)
       if (b)
@@ -31,6 +33,42 @@ foo (void)
           baz ();
 
   if (a) /* { dg-warning "ambiguous" }  */
+    while (1)
+      while (1)
+        if (b)
+          bar ();
+  else
+    baz ();
+
+  if (a) /* { dg-warning "ambiguous" }  */
+    for (i = 0; i < 10; i++)
+      for (j = 0; j < 10; j++)
+        if (b)
+          bar ();
+  else
+    baz ();
+
+  if (a)
+    for (i = 0; i < 10; i++)
+      if (b) /* { dg-warning "ambiguous" }  */
+        for (j = 0; j < 10; j++)
+          if (c)
+            bar ();
+      else
+        baz ();
+
+  if (a) /* { dg-warning "ambiguous" }  */
+    for (i = 0; i < 10; i++)
+      if (b)
+        for (j = 0; j < 10; j++)
+          if (c)
+            bar ();
+          else
+            baz ();
+  else
+    bar ();
+
+  if (a) /* { dg-warning "ambiguous" }  */
     for (;;)
       if (b)
         while (1)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]