This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 2/2] Fix C++ side of PR c/70436 (missing -Wparentheses warnings)
- From: Patrick Palka <patrick at parcs dot ath dot cx>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Patrick Palka <patrick at parcs dot ath dot cx>, gcc-patches at gcc dot gnu dot org, jason at redhat dot com
- Date: Thu, 31 Mar 2016 17:57:08 -0400 (EDT)
- Subject: Re: [PATCH 2/2] Fix C++ side of PR c/70436 (missing -Wparentheses warnings)
- Authentication-results: sourceware.org; auth=none
- References: <1459457644-20663-1-git-send-email-patrick at parcs dot ath dot cx> <20160331213759 dot GQ3017 at tucnak dot redhat dot com>
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)