On Thu, Mar 31, 2016 at 04:53:45PM -0400, Patrick Palka wrote:
> This patch fixes the new -Wparentheses warnings (implemented by the
> subsequent patch) that are encountered during bootstrap:
>
> /home/patrick/code/gcc/gcc/omp-low.c: In function ‘void
> scan_sharing_clauses(tree, omp_context*, bool)’:
> /home/patrick/code/gcc/gcc/omp-low.c:2381:6: error: suggest explicit braces
> to avoid ambiguous ‘else’ [-Werror=parentheses]
> if (scan_array_reductions)
> ^
> /home/patrick/code/gcc/gcc/gimplify.c: In function ‘gimple*
> gimplify_omp_ordered(tree, gimple_seq)’:
> /home/patrick/code/gcc/gcc/gimplify.c:9880:6: error: suggest explicit braces
> to avoid ambiguous ‘else’ [-Werror=parentheses]
> if (gimplify_omp_ctxp)
> ^
I imagine you noticed, but these warnings aren't that friendly, it seems
like it would be nice to point at the else, and maybe what it is an else
for and the thing it might seem to be an else for?
Trev
> In file included from /home/patrick/code/gcc/gcc/cp/optimize.c:25:0:
> /home/patrick/code/gcc/gcc/cp/optimize.c: In function ‘void
> populate_clone_array(tree, tree_node**)’:
> /home/patrick/code/gcc/gcc/cp/cp-tree.h:2529:6: error: suggest explicit
> braces to avoid ambiguous ‘else’ [-Werror=parentheses]
> if (TREE_CODE (FN) == FUNCTION_DECL \
> ^
> /home/patrick/code/gcc/gcc/cp/optimize.c:222:3: note: in expansion of macro
> ‘FOR_EACH_CLONE’
> FOR_EACH_CLONE (clone, fn)
> ^~~~~~~~~~~~~~
> /home/patrick/code/gcc/gcc/fortran/openmp.c: In function ‘gfc_omp_udr*
> gfc_find_omp_udr(gfc_namespace*, const char*, gfc_typespec*)’:
> /home/patrick/code/gcc/gcc/fortran/openmp.c:177:10: error: suggest explicit
> braces to avoid ambiguous ‘else’ [-Werror=parentheses]
> if (st != NULL)
> ^
>
> In each case I think the warning is harmless since the indentation of
> the code in question corresponds to how the "else" is actually parsed
> so I fixed each case simply by enclosing the entire body of the outer
> "if" in braces.
>
> The FOR_EACH_CLONE change resolves the cp/optimize.c warning. It
> adjusts the layout of the macro from
>
> if (p)
> for (...)
>
> to
>
> if (!p)
> ;
> else for (...)
>
> so that an "else" encountered in the body of the for-statement can no
> longer possibly bind to the outer "if (p)" conditional.
>
> Is this OK to commit after bootstrap + regtesting?
>
> gcc/cp/ChangeLog:
>
> PR c/70436
> * cp-tree.h (FOR_EACH_CLONE): Restructure macro to avoid
> potentially generating a future -Wparentheses warning in its
> callers.
>
> gcc/fortran/ChangeLog:
>
> PR c/70436
> * openmp.c (gfc_find_omp_udr): Add explicit braces to resolve a
> future -Wparentheses warning.
>
> gcc/ChangeLog:
>
> PR c/70436
> * gimplify.c (gimplify_omp_ordered): Add explicit braces to
> resolve a future -Wparentheses warning.
> * omp-low.c (scan_sharing_clauses): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> PR c/70436
> * g++.dg/plugin/pragma_plugin.c (handle_pragma_sayhello): Add
> explicit braces to resolve a future -Wparentheses warning.
> ---
> gcc/cp/cp-tree.h | 13 ++--
> gcc/fortran/openmp.c | 36 +++++-----
> gcc/gimplify.c | 108
> ++++++++++++++--------------
> gcc/omp-low.c | 28 ++++----
> gcc/testsuite/g++.dg/plugin/pragma_plugin.c | 12 ++--
> 5 files changed, 103 insertions(+), 94 deletions(-)
>
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index b7b770f..65f5693 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -2526,12 +2526,13 @@ struct GTY(()) lang_decl {
>
> */
> #define FOR_EACH_CLONE(CLONE, FN) \
> - if (TREE_CODE (FN) == FUNCTION_DECL \
> - && (DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (FN) \
> - || DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (FN))) \
> - for (CLONE = DECL_CHAIN (FN); \
> - CLONE && DECL_CLONED_FUNCTION_P (CLONE); \
> - CLONE = DECL_CHAIN (CLONE))
> + if (!(TREE_CODE (FN) == FUNCTION_DECL \
> + && (DECL_MAYBE_IN_CHARGE_CONSTRUCTOR_P (FN) \
> + || DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (FN))))\
> + ; \
> + else for (CLONE = DECL_CHAIN (FN); \
> + CLONE && DECL_CLONED_FUNCTION_P (CLONE); \
> + CLONE = DECL_CHAIN (CLONE))
>
> /* Nonzero if NODE has DECL_DISCRIMINATOR and not DECL_ACCESS. */
> #define DECL_DISCRIMINATOR_P(NODE) \
> diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
> index a6c39cd..0dd1a92 100644
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -175,24 +175,26 @@ gfc_find_omp_udr (gfc_namespace *ns, const char *name,
> gfc_typespec *ts)
>
> st = gfc_find_symtree (ns->omp_udr_root, name);
> if (st != NULL)
> - for (omp_udr = st->n.omp_udr; omp_udr; omp_udr = omp_udr->next)
> - if (ts == NULL)
> - return omp_udr;
> - else if (gfc_compare_types (&omp_udr->ts, ts))
> - {
> - if (ts->type == BT_CHARACTER)
> - {
> - if (omp_udr->ts.u.cl->length == NULL)
> - return omp_udr;
> - if (ts->u.cl->length == NULL)
> - continue;
> - if (gfc_compare_expr (omp_udr->ts.u.cl->length,
> - ts->u.cl->length,
> - INTRINSIC_EQ) != 0)
> - continue;
> - }
> + {
> + for (omp_udr = st->n.omp_udr; omp_udr; omp_udr = omp_udr->next)
> + if (ts == NULL)
> return omp_udr;
> - }
> + else if (gfc_compare_types (&omp_udr->ts, ts))
> + {
> + if (ts->type == BT_CHARACTER)
> + {
> + if (omp_udr->ts.u.cl->length == NULL)
> + return omp_udr;
> + if (ts->u.cl->length == NULL)
> + continue;
> + if (gfc_compare_expr (omp_udr->ts.u.cl->length,
> + ts->u.cl->length,
> + INTRINSIC_EQ) != 0)
> + continue;
> + }
> + return omp_udr;
> + }
> + }
>
> /* Don't escape an interface block. */
> if (ns && !ns->has_import_set
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 26b5a10..1c824fa 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -9878,64 +9878,66 @@ gimplify_omp_ordered (tree expr, gimple_seq body)
> tree sink_c = NULL_TREE;
>
> if (gimplify_omp_ctxp)
> - for (c = OMP_ORDERED_CLAUSES (expr); c; c = OMP_CLAUSE_CHAIN (c))
> - if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEPEND
> - && gimplify_omp_ctxp->loop_iter_var.is_empty ()
> - && (OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SINK
> - || OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SOURCE))
> - {
> - error_at (OMP_CLAUSE_LOCATION (c),
> - "%<ordered%> construct with %<depend%> clause must be "
> - "closely nested inside a loop with %<ordered%> clause "
> - "with a parameter");
> - failures++;
> - }
> - else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEPEND
> - && OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SINK)
> - {
> - bool fail = false;
> - for (decls = OMP_CLAUSE_DECL (c), i = 0;
> - decls && TREE_CODE (decls) == TREE_LIST;
> - decls = TREE_CHAIN (decls), ++i)
> - if (i >= gimplify_omp_ctxp->loop_iter_var.length () / 2)
> - continue;
> - else if (TREE_VALUE (decls)
> - != gimplify_omp_ctxp->loop_iter_var[2 * i])
> + {
> + for (c = OMP_ORDERED_CLAUSES (expr); c; c = OMP_CLAUSE_CHAIN (c))
> + if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEPEND
> + && gimplify_omp_ctxp->loop_iter_var.is_empty ()
> + && (OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SINK
> + || OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SOURCE))
> + {
> + error_at (OMP_CLAUSE_LOCATION (c),
> + "%<ordered%> construct with %<depend%> clause must be "
> + "closely nested inside a loop with %<ordered%> clause "
> + "with a parameter");
> + failures++;
> + }
> + else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEPEND
> + && OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SINK)
> + {
> + bool fail = false;
> + for (decls = OMP_CLAUSE_DECL (c), i = 0;
> + decls && TREE_CODE (decls) == TREE_LIST;
> + decls = TREE_CHAIN (decls), ++i)
> + if (i >= gimplify_omp_ctxp->loop_iter_var.length () / 2)
> + continue;
> + else if (TREE_VALUE (decls)
> + != gimplify_omp_ctxp->loop_iter_var[2 * i])
> + {
> + error_at (OMP_CLAUSE_LOCATION (c),
> + "variable %qE is not an iteration "
> + "of outermost loop %d, expected %qE",
> + TREE_VALUE (decls), i + 1,
> + gimplify_omp_ctxp->loop_iter_var[2 * i]);
> + fail = true;
> + failures++;
> + }
> + else
> + TREE_VALUE (decls)
> + = gimplify_omp_ctxp->loop_iter_var[2 * i + 1];
> + if (!fail && i != gimplify_omp_ctxp->loop_iter_var.length () / 2)
> + {
> + error_at (OMP_CLAUSE_LOCATION (c),
> + "number of variables in %<depend(sink)%> "
> + "clause does not match number of "
> + "iteration variables");
> + failures++;
> + }
> + sink_c = c;
> + }
> + else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEPEND
> + && OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SOURCE)
> + {
> + if (source_c)
> {
> error_at (OMP_CLAUSE_LOCATION (c),
> - "variable %qE is not an iteration "
> - "of outermost loop %d, expected %qE",
> - TREE_VALUE (decls), i + 1,
> - gimplify_omp_ctxp->loop_iter_var[2 * i]);
> - fail = true;
> + "more than one %<depend(source)%> clause on an "
> + "%<ordered%> construct");
> failures++;
> }
> else
> - TREE_VALUE (decls)
> - = gimplify_omp_ctxp->loop_iter_var[2 * i + 1];
> - if (!fail && i != gimplify_omp_ctxp->loop_iter_var.length () / 2)
> - {
> - error_at (OMP_CLAUSE_LOCATION (c),
> - "number of variables in %<depend(sink)%> "
> - "clause does not match number of "
> - "iteration variables");
> - failures++;
> - }
> - sink_c = c;
> - }
> - else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEPEND
> - && OMP_CLAUSE_DEPEND_KIND (c) == OMP_CLAUSE_DEPEND_SOURCE)
> - {
> - if (source_c)
> - {
> - error_at (OMP_CLAUSE_LOCATION (c),
> - "more than one %<depend(source)%> clause on an "
> - "%<ordered%> construct");
> - failures++;
> - }
> - else
> - source_c = c;
> - }
> + source_c = c;
> + }
> + }
> if (source_c && sink_c)
> {
> error_at (OMP_CLAUSE_LOCATION (source_c),
> diff --git a/gcc/omp-low.c b/gcc/omp-low.c
> index 3fd6eb3..df328f9 100644
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -2379,19 +2379,21 @@ scan_sharing_clauses (tree clauses, omp_context *ctx,
> gcc_checking_assert (!scan_array_reductions
> || !is_gimple_omp_oacc (ctx->stmt));
> if (scan_array_reductions)
> - for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
> - if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_REDUCTION
> - && OMP_CLAUSE_REDUCTION_PLACEHOLDER (c))
> - {
> - scan_omp (&OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c), ctx);
> - scan_omp (&OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c), ctx);
> - }
> - else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LASTPRIVATE
> - && OMP_CLAUSE_LASTPRIVATE_GIMPLE_SEQ (c))
> - scan_omp (&OMP_CLAUSE_LASTPRIVATE_GIMPLE_SEQ (c), ctx);
> - else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LINEAR
> - && OMP_CLAUSE_LINEAR_GIMPLE_SEQ (c))
> - scan_omp (&OMP_CLAUSE_LINEAR_GIMPLE_SEQ (c), ctx);
> + {
> + for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
> + if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_REDUCTION
> + && OMP_CLAUSE_REDUCTION_PLACEHOLDER (c))
> + {
> + scan_omp (&OMP_CLAUSE_REDUCTION_GIMPLE_INIT (c), ctx);
> + scan_omp (&OMP_CLAUSE_REDUCTION_GIMPLE_MERGE (c), ctx);
> + }
> + else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LASTPRIVATE
> + && OMP_CLAUSE_LASTPRIVATE_GIMPLE_SEQ (c))
> + scan_omp (&OMP_CLAUSE_LASTPRIVATE_GIMPLE_SEQ (c), ctx);
> + else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_LINEAR
> + && OMP_CLAUSE_LINEAR_GIMPLE_SEQ (c))
> + scan_omp (&OMP_CLAUSE_LINEAR_GIMPLE_SEQ (c), ctx);
> + }
> }
>
> /* Create a new name for omp child function. Returns an identifier. If
> diff --git a/gcc/testsuite/g++.dg/plugin/pragma_plugin.c
> b/gcc/testsuite/g++.dg/plugin/pragma_plugin.c
> index 940c302..6794c95 100644
> --- a/gcc/testsuite/g++.dg/plugin/pragma_plugin.c
> +++ b/gcc/testsuite/g++.dg/plugin/pragma_plugin.c
> @@ -32,14 +32,16 @@ handle_pragma_sayhello (cpp_reader *dummy)
> return;
> }
> if (TREE_STRING_LENGTH (message) > 1)
> - if (cfun)
> - warning (OPT_Wpragmas,
> - "%<pragma GCCPLUGIN sayhello%> from function %qE: %s",
> - cfun->decl, TREE_STRING_POINTER (message));
> + {
> + if (cfun)
> + warning (OPT_Wpragmas,
> + "%<pragma GCCPLUGIN sayhello%> from function %qE: %s",
> + cfun->decl, TREE_STRING_POINTER (message));
> else
> - warning (OPT_Wpragmas,
> + warning (OPT_Wpragmas,
> "%<pragma GCCPLUGIN sayhello%> outside of function: %s",
> TREE_STRING_POINTER (message));
> + }
> }
>
> /* Plugin callback called during pragma registration */
> --
> 2.8.0.rc3.27.gade0865
>
