Message ID | 20151006172959.GT6184@redhat.com |
---|---|
State | New |
Headers | show |
On Tue, Oct 6, 2015 at 7:29 PM, Marek Polacek <polacek@redhat.com> wrote: > This patch makes the C FE use the protected_set_expr_location helper > where appropriate. > > Bootstrapped/regtested on x86_64-linux, applying to trunk. > > 2015-10-06 Marek Polacek <polacek@redhat.com> > > * c-parser.c (c_parser_statement_after_labels): Use > protected_set_expr_location. > (c_parser_omp_clause_num_gangs): Likewise. > (c_parser_omp_clause_num_threads): Likewise. > (c_parser_omp_clause_num_workers): Likewise. > (c_parser_omp_clause_vector_length): Likewise. > (c_parser_omp_clause_num_teams): Likewise. > (c_parser_omp_clause_thread_limit): Likewise. > * c-typeck.c (build_c_cast): Likewise. > (c_cast_expr): Likewise. > > diff --git gcc/c/c-parser.c gcc/c/c-parser.c > index 0df7d7b..2d24c21 100644 > --- gcc/c/c-parser.c > +++ gcc/c/c-parser.c > @@ -5141,9 +5141,8 @@ c_parser_statement_after_labels (c_parser *parser, vec<tree> *chain) > (recursively) all of the component statements should already have > line numbers assigned. ??? Can we discard no-op statements > earlier? */ > - if (CAN_HAVE_LOCATION_P (stmt) > - && EXPR_LOCATION (stmt) == UNKNOWN_LOCATION) > - SET_EXPR_LOCATION (stmt, loc); > + if (EXPR_LOCATION (stmt) == UNKNOWN_LOCATION) > + protected_set_expr_location (stmt, loc); This one doesn't look like an improvement though as EXPR_LOCATION tests CAN_HAVE_LOCATION_P and returns UNKNOWN_LOCATION if not. > > parser->in_if_block = in_if_block; > } > @@ -10681,8 +10680,7 @@ c_parser_omp_clause_num_gangs (c_parser *parser, tree list) > /* Attempt to statically determine when the number isn't positive. */ > c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, > build_int_cst (TREE_TYPE (t), 0)); > - if (CAN_HAVE_LOCATION_P (c)) > - SET_EXPR_LOCATION (c, expr_loc); > + protected_set_expr_location (c, expr_loc); > if (c == boolean_true_node) > { > warning_at (expr_loc, 0, > @@ -10726,8 +10724,7 @@ c_parser_omp_clause_num_threads (c_parser *parser, tree list) > /* Attempt to statically determine when the number isn't positive. */ > c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, > build_int_cst (TREE_TYPE (t), 0)); > - if (CAN_HAVE_LOCATION_P (c)) > - SET_EXPR_LOCATION (c, expr_loc); > + protected_set_expr_location (c, expr_loc); > if (c == boolean_true_node) > { > warning_at (expr_loc, 0, > @@ -10771,8 +10768,7 @@ c_parser_omp_clause_num_workers (c_parser *parser, tree list) > /* Attempt to statically determine when the number isn't positive. */ > c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, > build_int_cst (TREE_TYPE (t), 0)); > - if (CAN_HAVE_LOCATION_P (c)) > - SET_EXPR_LOCATION (c, expr_loc); > + protected_set_expr_location (c, expr_loc); > if (c == boolean_true_node) > { > warning_at (expr_loc, 0, > @@ -11116,8 +11112,7 @@ c_parser_omp_clause_vector_length (c_parser *parser, tree list) > /* Attempt to statically determine when the number isn't positive. */ > c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, > build_int_cst (TREE_TYPE (t), 0)); > - if (CAN_HAVE_LOCATION_P (c)) > - SET_EXPR_LOCATION (c, expr_loc); > + protected_set_expr_location (c, expr_loc); > if (c == boolean_true_node) > { > warning_at (expr_loc, 0, > @@ -11193,8 +11188,7 @@ c_parser_omp_clause_num_teams (c_parser *parser, tree list) > /* Attempt to statically determine when the number isn't positive. */ > c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, > build_int_cst (TREE_TYPE (t), 0)); > - if (CAN_HAVE_LOCATION_P (c)) > - SET_EXPR_LOCATION (c, expr_loc); > + protected_set_expr_location (c, expr_loc); > if (c == boolean_true_node) > { > warning_at (expr_loc, 0, "%<num_teams%> value must be positive"); > @@ -11237,8 +11231,7 @@ c_parser_omp_clause_thread_limit (c_parser *parser, tree list) > /* Attempt to statically determine when the number isn't positive. */ > c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, > build_int_cst (TREE_TYPE (t), 0)); > - if (CAN_HAVE_LOCATION_P (c)) > - SET_EXPR_LOCATION (c, expr_loc); > + protected_set_expr_location (c, expr_loc); > if (c == boolean_true_node) > { > warning_at (expr_loc, 0, "%<thread_limit%> value must be positive"); > diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c > index 10cad36..df3245a4 100644 > --- gcc/c/c-typeck.c > +++ gcc/c/c-typeck.c > @@ -5216,8 +5216,7 @@ build_c_cast (location_t loc, tree type, tree expr) > || TREE_CODE (expr) == COMPLEX_CST))) > value = build1 (NOP_EXPR, type, value); > > - if (CAN_HAVE_LOCATION_P (value)) > - SET_EXPR_LOCATION (value, loc); > + protected_set_expr_location (value, loc); > return value; > } > > @@ -5255,8 +5254,8 @@ c_cast_expr (location_t loc, struct c_type_name *type_name, tree expr) > SET_EXPR_LOCATION (ret, loc); > } > > - if (CAN_HAVE_LOCATION_P (ret) && !EXPR_HAS_LOCATION (ret)) > - SET_EXPR_LOCATION (ret, loc); > + if (!EXPR_HAS_LOCATION (ret)) > + protected_set_expr_location (ret, loc); > > /* C++ does not permits types to be defined in a cast, but it > allows references to incomplete types. */ > > Marek
On Wed, Oct 07, 2015 at 10:14:54AM +0200, Richard Biener wrote: > > --- gcc/c/c-parser.c > > +++ gcc/c/c-parser.c > > @@ -5141,9 +5141,8 @@ c_parser_statement_after_labels (c_parser *parser, vec<tree> *chain) > > (recursively) all of the component statements should already have > > line numbers assigned. ??? Can we discard no-op statements > > earlier? */ > > - if (CAN_HAVE_LOCATION_P (stmt) > > - && EXPR_LOCATION (stmt) == UNKNOWN_LOCATION) > > - SET_EXPR_LOCATION (stmt, loc); > > + if (EXPR_LOCATION (stmt) == UNKNOWN_LOCATION) > > + protected_set_expr_location (stmt, loc); > > This one doesn't look like an improvement though as EXPR_LOCATION tests > CAN_HAVE_LOCATION_P and returns UNKNOWN_LOCATION if not. Yeah, but protected_set_expr_location tests CAN_HAVE_LOCATION_P so we wouldn't set the location anyway. But I can surely revert that bit if you prefer. Marek
On Wed, Oct 7, 2015 at 1:02 PM, Marek Polacek <polacek@redhat.com> wrote: > On Wed, Oct 07, 2015 at 10:14:54AM +0200, Richard Biener wrote: >> > --- gcc/c/c-parser.c >> > +++ gcc/c/c-parser.c >> > @@ -5141,9 +5141,8 @@ c_parser_statement_after_labels (c_parser *parser, vec<tree> *chain) >> > (recursively) all of the component statements should already have >> > line numbers assigned. ??? Can we discard no-op statements >> > earlier? */ >> > - if (CAN_HAVE_LOCATION_P (stmt) >> > - && EXPR_LOCATION (stmt) == UNKNOWN_LOCATION) >> > - SET_EXPR_LOCATION (stmt, loc); >> > + if (EXPR_LOCATION (stmt) == UNKNOWN_LOCATION) >> > + protected_set_expr_location (stmt, loc); >> >> This one doesn't look like an improvement though as EXPR_LOCATION tests >> CAN_HAVE_LOCATION_P and returns UNKNOWN_LOCATION if not. > > Yeah, but protected_set_expr_location tests CAN_HAVE_LOCATION_P so we > wouldn't set the location anyway. > > But I can surely revert that bit if you prefer. Well, it's not my call but the original code is clearer if one looks up EXPR_LOCATION. Richard. > Marek
diff --git gcc/c/c-parser.c gcc/c/c-parser.c index 0df7d7b..2d24c21 100644 --- gcc/c/c-parser.c +++ gcc/c/c-parser.c @@ -5141,9 +5141,8 @@ c_parser_statement_after_labels (c_parser *parser, vec<tree> *chain) (recursively) all of the component statements should already have line numbers assigned. ??? Can we discard no-op statements earlier? */ - if (CAN_HAVE_LOCATION_P (stmt) - && EXPR_LOCATION (stmt) == UNKNOWN_LOCATION) - SET_EXPR_LOCATION (stmt, loc); + if (EXPR_LOCATION (stmt) == UNKNOWN_LOCATION) + protected_set_expr_location (stmt, loc); parser->in_if_block = in_if_block; } @@ -10681,8 +10680,7 @@ c_parser_omp_clause_num_gangs (c_parser *parser, tree list) /* Attempt to statically determine when the number isn't positive. */ c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, build_int_cst (TREE_TYPE (t), 0)); - if (CAN_HAVE_LOCATION_P (c)) - SET_EXPR_LOCATION (c, expr_loc); + protected_set_expr_location (c, expr_loc); if (c == boolean_true_node) { warning_at (expr_loc, 0, @@ -10726,8 +10724,7 @@ c_parser_omp_clause_num_threads (c_parser *parser, tree list) /* Attempt to statically determine when the number isn't positive. */ c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, build_int_cst (TREE_TYPE (t), 0)); - if (CAN_HAVE_LOCATION_P (c)) - SET_EXPR_LOCATION (c, expr_loc); + protected_set_expr_location (c, expr_loc); if (c == boolean_true_node) { warning_at (expr_loc, 0, @@ -10771,8 +10768,7 @@ c_parser_omp_clause_num_workers (c_parser *parser, tree list) /* Attempt to statically determine when the number isn't positive. */ c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, build_int_cst (TREE_TYPE (t), 0)); - if (CAN_HAVE_LOCATION_P (c)) - SET_EXPR_LOCATION (c, expr_loc); + protected_set_expr_location (c, expr_loc); if (c == boolean_true_node) { warning_at (expr_loc, 0, @@ -11116,8 +11112,7 @@ c_parser_omp_clause_vector_length (c_parser *parser, tree list) /* Attempt to statically determine when the number isn't positive. */ c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, build_int_cst (TREE_TYPE (t), 0)); - if (CAN_HAVE_LOCATION_P (c)) - SET_EXPR_LOCATION (c, expr_loc); + protected_set_expr_location (c, expr_loc); if (c == boolean_true_node) { warning_at (expr_loc, 0, @@ -11193,8 +11188,7 @@ c_parser_omp_clause_num_teams (c_parser *parser, tree list) /* Attempt to statically determine when the number isn't positive. */ c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, build_int_cst (TREE_TYPE (t), 0)); - if (CAN_HAVE_LOCATION_P (c)) - SET_EXPR_LOCATION (c, expr_loc); + protected_set_expr_location (c, expr_loc); if (c == boolean_true_node) { warning_at (expr_loc, 0, "%<num_teams%> value must be positive"); @@ -11237,8 +11231,7 @@ c_parser_omp_clause_thread_limit (c_parser *parser, tree list) /* Attempt to statically determine when the number isn't positive. */ c = fold_build2_loc (expr_loc, LE_EXPR, boolean_type_node, t, build_int_cst (TREE_TYPE (t), 0)); - if (CAN_HAVE_LOCATION_P (c)) - SET_EXPR_LOCATION (c, expr_loc); + protected_set_expr_location (c, expr_loc); if (c == boolean_true_node) { warning_at (expr_loc, 0, "%<thread_limit%> value must be positive"); diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index 10cad36..df3245a4 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -5216,8 +5216,7 @@ build_c_cast (location_t loc, tree type, tree expr) || TREE_CODE (expr) == COMPLEX_CST))) value = build1 (NOP_EXPR, type, value); - if (CAN_HAVE_LOCATION_P (value)) - SET_EXPR_LOCATION (value, loc); + protected_set_expr_location (value, loc); return value; } @@ -5255,8 +5254,8 @@ c_cast_expr (location_t loc, struct c_type_name *type_name, tree expr) SET_EXPR_LOCATION (ret, loc); } - if (CAN_HAVE_LOCATION_P (ret) && !EXPR_HAS_LOCATION (ret)) - SET_EXPR_LOCATION (ret, loc); + if (!EXPR_HAS_LOCATION (ret)) + protected_set_expr_location (ret, loc); /* C++ does not permits types to be defined in a cast, but it allows references to incomplete types. */