This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] C++: Fix-it hints for '.' vs '->' (PR c++/84898)
- From: Jason Merrill <jason at redhat dot com>
- To: David Malcolm <dmalcolm at redhat dot com>
- Cc: gcc-patches List <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 30 Aug 2018 15:18:41 -0400
- Subject: Re: [PATCH] C++: Fix-it hints for '.' vs '->' (PR c++/84898)
- References: <1535575020-41466-1-git-send-email-dmalcolm@redhat.com>
On Wed, Aug 29, 2018 at 4:37 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> 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);
Maybe give the new parameter a default argument of UNKNOWN_LOCATION
rather than add it to callers?
> extern tree build_x_arrow (location_t, tree,
> - tsubst_flags_t);
> + tsubst_flags_t, location_t);
...
> 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);
Do we really want to pass two different locations into these
functions? Currently what we're passing is just the location of the
first token of the expression argument, so it seems redundant; perhaps
we could stick with one location parameter and pass the operator
location instead.
Jason