This patch implements fix-it hints for "." vs "->" mismatches in the C++ frontend, for the places where the relevant location information is readily available.
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu; adds 39 PASS results to g++.sum. OK for trunk? gcc/cp/ChangeLog: PR c++/84898 * cp-tree.h (finish_class_member_access_expr): Add location_t param. (build_x_arrow): Likewise. * parser.c (cp_parser_postfix_expression): Retain the location of the "." or "->" token and pass it to cp_parser_postfix_dot_deref_expression. (cp_parser_postfix_dot_deref_expression): Add "loc_dot_or_deref" param. Pass it on to calls to build_x_arrow and finish_class_member_access_expr. (cp_parser_builtin_offsetof): Update for new param. (cp_parser_range_for_member_function): Likewise. (cp_parser_omp_var_list_no_open): Likewise. * pt.c (tsubst_copy_and_build): Likewise. * semantics.c (finish_id_expression): Likewise. * typeck.c (finish_class_member_access_expr): Add new param "loc_dot_or_deref". When not UNKNOWN_LOCATION, use it to provide a "->" fix-it hint for the pointer type error, and for the error's location, rather than input_location, thus moving it from the field name to the "." token. * typeck2.c: Include "gcc-rich-location.h". (build_x_arrow): Add param "loc_deref". Use it to provide a fix-it hint. gcc/objc/ChangeLog: PR c++/84898 * objc-act.c (objc_build_component_ref): Update for new param. gcc/testsuite/ChangeLog: PR c++/84898 * g++.dg/diagnostic/fixits.C: New test. * g++.dg/parse/error20.C: Update expected column number to be on the erroneous ".", rather than the fieldname. libcc1/ChangeLog: PR c++/84898 * libcp1plugin.cc (plugin_build_binary_expr): Update for new parameters of build_x_arrow and finish_class_member_access_expr. --- gcc/cp/cp-tree.h | 4 +- gcc/cp/parser.c | 39 +++++++++------ gcc/cp/pt.c | 4 +- gcc/cp/semantics.c | 3 +- gcc/cp/typeck.c | 20 ++++++-- gcc/cp/typeck2.c | 13 ++++- gcc/objc/objc-act.c | 3 +- gcc/testsuite/g++.dg/diagnostic/fixits.C | 83 ++++++++++++++++++++++++++++++++ gcc/testsuite/g++.dg/parse/error20.C | 2 +- libcc1/libcp1plugin.cc | 5 +- 10 files changed, 147 insertions(+), 29 deletions(-) create mode 100644 gcc/testsuite/g++.dg/diagnostic/fixits.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 055f2bc..56e99b2 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7244,7 +7244,7 @@ extern tree decay_conversion (tree, extern tree build_class_member_access_expr (cp_expr, tree, tree, bool, tsubst_flags_t); extern tree finish_class_member_access_expr (cp_expr, tree, bool, - tsubst_flags_t); + tsubst_flags_t, location_t); extern tree build_x_indirect_ref (location_t, tree, ref_operator, tsubst_flags_t); extern tree cp_build_indirect_ref (tree, ref_operator, @@ -7404,7 +7404,7 @@ extern tree digest_init_flags (tree, tree, int, tsubst_flags_t); extern tree digest_nsdmi_init (tree, tree, tsubst_flags_t); extern tree build_scoped_ref (tree, tree, tree *); extern tree build_x_arrow (location_t, tree, - tsubst_flags_t); + tsubst_flags_t, location_t); extern tree build_m_component_ref (tree, tree, tsubst_flags_t); extern tree build_functional_cast (tree, tree, tsubst_flags_t); extern tree add_exception_specifier (tree, tree, int); diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index d8d4301..e869c8e 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -2045,7 +2045,8 @@ static cp_expr cp_parser_postfix_expression static tree cp_parser_postfix_open_square_expression (cp_parser *, tree, bool, bool); static tree cp_parser_postfix_dot_deref_expression - (cp_parser *, enum cpp_ttype, cp_expr, bool, cp_id_kind *, location_t); + (cp_parser *, enum cpp_ttype, cp_expr, bool, cp_id_kind *, location_t, + location_t); static vec<tree, va_gc> *cp_parser_parenthesized_expression_list (cp_parser *, int, bool, bool, bool *, location_t * = NULL, bool = false); @@ -7281,16 +7282,20 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p, postfix-expression . pseudo-destructor-name postfix-expression -> template [opt] id-expression postfix-expression -> pseudo-destructor-name */ + { + location_t loc_dot_or_deref = token->location; - /* Consume the `.' or `->' operator. */ - cp_lexer_consume_token (parser->lexer); + /* Consume the `.' or `->' operator. */ + cp_lexer_consume_token (parser->lexer); - postfix_expression - = cp_parser_postfix_dot_deref_expression (parser, token->type, - postfix_expression, - false, &idk, loc); + postfix_expression + = cp_parser_postfix_dot_deref_expression (parser, token->type, + postfix_expression, + false, &idk, loc, + loc_dot_or_deref); - is_member_access = true; + is_member_access = true; + } break; case CPP_PLUS_PLUS: @@ -7478,7 +7483,8 @@ cp_parser_postfix_dot_deref_expression (cp_parser *parser, enum cpp_ttype token_type, cp_expr postfix_expression, bool for_offsetof, cp_id_kind *idk, - location_t location) + location_t location, + location_t loc_dot_or_deref) { tree name; bool dependent_p; @@ -7489,7 +7495,8 @@ cp_parser_postfix_dot_deref_expression (cp_parser *parser, /* If this is a `->' operator, dereference the pointer. */ if (token_type == CPP_DEREF) postfix_expression = build_x_arrow (location, postfix_expression, - tf_warning_or_error); + tf_warning_or_error, + loc_dot_or_deref); /* Check to see whether or not the expression is type-dependent and not the current instantiation. */ dependent_p = type_dependent_object_expression_p (postfix_expression); @@ -7641,7 +7648,8 @@ cp_parser_postfix_dot_deref_expression (cp_parser *parser, postfix_expression = finish_class_member_access_expr (postfix_expression, name, template_p, - tf_warning_or_error); + tf_warning_or_error, + loc_dot_or_deref); /* Build a location e.g.: ptr->access_expr ~~~^~~~~~~~~~~~~ @@ -9864,7 +9872,8 @@ cp_parser_builtin_offsetof (cp_parser *parser) /* Parse the offsetof-member-designator. We begin as if we saw "expr->". */ expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DEREF, object_ptr, - true, &dummy, token->location); + true, &dummy, token->location, + UNKNOWN_LOCATION); while (true) { token = cp_lexer_peek_token (parser->lexer); @@ -9887,6 +9896,7 @@ cp_parser_builtin_offsetof (cp_parser *parser) cp_lexer_consume_token (parser->lexer); expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DOT, expr, true, &dummy, + token->location, token->location); break; @@ -12239,7 +12249,8 @@ cp_parser_range_for_member_function (tree range, tree identifier) vec<tree, va_gc> *vec; member = finish_class_member_access_expr (range, identifier, - false, tf_warning_or_error); + false, tf_warning_or_error, + UNKNOWN_LOCATION); if (member == error_mark_node) return error_mark_node; @@ -31676,7 +31687,7 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind, decl = cp_parser_postfix_dot_deref_expression (parser, CPP_DOT, decl, false, - &idk, loc); + &idk, loc, loc); } /* FALLTHROUGH. */ case OMP_CLAUSE_DEPEND: diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index a7266e3..a3d17a1 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -18164,7 +18164,7 @@ tsubst_copy_and_build (tree t, if (DECL_P (op1) && !mark_used (op1, complain) && !(complain & tf_error)) RETURN (error_mark_node); - RETURN (build_x_arrow (input_location, op1, complain)); + RETURN (build_x_arrow (input_location, op1, complain, UNKNOWN_LOCATION)); case NEW_EXPR: { @@ -18781,7 +18781,7 @@ tsubst_copy_and_build (tree t, r = finish_class_member_access_expr (object, member, /*template_p=*/false, - complain); + complain, UNKNOWN_LOCATION); if (TREE_CODE (r) == COMPONENT_REF) REF_PARENTHESIZED_P (r) = REF_PARENTHESIZED_P (t); RETURN (r); diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index bfdca50..7edcfde 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -3812,7 +3812,8 @@ finish_id_expression (tree id_expression, decl = maybe_dummy_object (DECL_CONTEXT (first_fn), 0); return finish_class_member_access_expr (decl, id_expression, /*template_p=*/false, - tf_warning_or_error); + tf_warning_or_error, + UNKNOWN_LOCATION); } decl = baselink_for_fns (decl); diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 122d9dc..f367e53 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -2756,7 +2756,8 @@ access_failure_info::maybe_suggest_accessor (bool const_p) const tree finish_class_member_access_expr (cp_expr object, tree name, bool template_p, - tsubst_flags_t complain) + tsubst_flags_t complain, + location_t loc_dot_or_deref) { tree expr; tree object_type; @@ -2813,9 +2814,20 @@ finish_class_member_access_expr (cp_expr object, tree name, bool template_p, { if (INDIRECT_TYPE_P (object_type) && CLASS_TYPE_P (TREE_TYPE (object_type))) - error ("request for member %qD in %qE, which is of pointer " - "type %qT (maybe you meant to use %<->%> ?)", - name, object.get_value (), object_type); + { + location_t loc; + if (loc_dot_or_deref != UNKNOWN_LOCATION) + loc = loc_dot_or_deref; + else + loc = input_location; + gcc_rich_location richloc (loc); + if (loc_dot_or_deref != UNKNOWN_LOCATION) + richloc.add_fixit_replace ("->"); + error_at (&richloc, + "request for member %qD in %qE, which is of pointer " + "type %qT (maybe you meant to use %<->%> ?)", + name, object.get_value (), object_type); + } else error ("request for member %qD in %qE, which is of non-class " "type %qT", name, object.get_value (), object_type); diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c index 1e899ab..3611a27 100644 --- a/gcc/cp/typeck2.c +++ b/gcc/cp/typeck2.c @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. If not see #include "stor-layout.h" #include "varasm.h" #include "intl.h" +#include "gcc-rich-location.h" static tree process_init_constructor (tree type, tree init, int nested, @@ -1839,7 +1840,8 @@ build_scoped_ref (tree datum, tree basetype, tree* binfo_p) delegation is detected. */ tree -build_x_arrow (location_t loc, tree expr, tsubst_flags_t complain) +build_x_arrow (location_t loc, tree expr, tsubst_flags_t complain, + location_t loc_deref) { tree orig_expr = expr; tree type = TREE_TYPE (expr); @@ -1897,7 +1899,14 @@ build_x_arrow (location_t loc, tree expr, tsubst_flags_t complain) if (last_rval == NULL_TREE) { if (complain & tf_error) - error ("base operand of %<->%> has non-pointer type %qT", type); + { + gcc_rich_location richloc (loc); + if (loc_deref != UNKNOWN_LOCATION) + richloc.add_fixit_replace (loc_deref, "."); + error_at (&richloc, + "base operand of %<->%> has non-pointer type %qT", + type); + } return error_mark_node; } diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c index d086930..aec87fe 100644 --- a/gcc/objc/objc-act.c +++ b/gcc/objc/objc-act.c @@ -2652,7 +2652,8 @@ objc_build_component_ref (tree datum, tree component) a worthy substitute. */ #ifdef OBJCPLUS return finish_class_member_access_expr (datum, component, false, - tf_warning_or_error); + tf_warning_or_error, + UNKNOWN_LOCATION); #else return build_component_ref (input_location, datum, component, UNKNOWN_LOCATION); diff --git a/gcc/testsuite/g++.dg/diagnostic/fixits.C b/gcc/testsuite/g++.dg/diagnostic/fixits.C new file mode 100644 index 0000000..3e01d56 --- /dev/null +++ b/gcc/testsuite/g++.dg/diagnostic/fixits.C @@ -0,0 +1,83 @@ +/* { dg-do compile } */ +/* { dg-options "-fdiagnostics-show-caret" } */ + +struct foo { int field; }; +union u { int field; }; + +/* Verify that we issue a hint for "." used with a ptr to a struct. */ + +int test_1 (struct foo *ptr) +{ + return ptr.field; /* { dg-error "maybe you meant to use '->'" } */ +/* { dg-begin-multiline-output "" } + return ptr.field; + ^ + -> + { dg-end-multiline-output "" } */ +} + +/* Likewise for a ptr to a union. */ + +int test_2 (union u *ptr) +{ + return ptr.field; /* { dg-error "maybe you meant to use '->'" } */ +/* { dg-begin-multiline-output "" } + return ptr.field; + ^ + -> + { dg-end-multiline-output "" } */ +} + +/* Verify that we don't issue a hint for a ptr to something that isn't a + struct or union. */ + +int test_3 (void **ptr) +{ + return ptr.field; /* { dg-error "which is of non-class type" } */ +/* { dg-begin-multiline-output "" } + return ptr.field; + ^~~~~ + { dg-end-multiline-output "" } */ +} + +int test_4 () +{ + struct foo val; + return val->field; /* { dg-error "has non-pointer type" } */ +/* { dg-begin-multiline-output "" } + return val->field; + ^~~ + -- + . + { dg-end-multiline-output "" } */ +} + +/* Likewise for a ptr to a union. */ + +int test_5 () +{ + union u val; + + return val->field; /* { dg-error "has non-pointer type" } */ +/* { dg-begin-multiline-output "" } + return val->field; + ^~~ + -- + . + { dg-end-multiline-output "" } */ +} + +struct nested +{ + struct foo *indirect; +}; + +int test_6 () +{ + return __builtin_offsetof (nested, indirect.field); /* { dg-error "maybe you meant to use '->'" } */ +/* { dg-begin-multiline-output "" } + return __builtin_offsetof (nested, indirect.field); + ^ + -> + { dg-end-multiline-output "" } */ +} diff --git a/gcc/testsuite/g++.dg/parse/error20.C b/gcc/testsuite/g++.dg/parse/error20.C index 6119df9..18e981aa 100644 --- a/gcc/testsuite/g++.dg/parse/error20.C +++ b/gcc/testsuite/g++.dg/parse/error20.C @@ -12,7 +12,7 @@ struct C { }; int main() { C c; - A(c.p.i); // { dg-error "9:request for member 'i' in 'c.C::p', which is of pointer type 'B" } + A(c.p.i); // { dg-error "8:request for member 'i' in 'c.C::p', which is of pointer type 'B" } return 0; } diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc index 1034147..be5e4a2 100644 --- a/libcc1/libcp1plugin.cc +++ b/libcc1/libcp1plugin.cc @@ -2940,12 +2940,13 @@ plugin_build_binary_expr (cc1_plugin::connection *self, switch (opcode) { case INDIRECT_REF: // This is actually a "->". - op0 = build_x_arrow (/*loc=*/0, op0, tf_error); + op0 = build_x_arrow (/*loc=*/0, op0, tf_error, UNKNOWN_LOCATION); /* Fall through. */ case COMPONENT_REF: result = finish_class_member_access_expr (op0, op1, /*template_p=*/false, - tf_error); + tf_error, + UNKNOWN_LOCATION); break; default: -- 1.8.5.3