* [PATCH] C++: Fix-it hints for '.' vs '->' (PR c++/84898)
@ 2018-08-29 19:50 David Malcolm
2018-08-30 19:19 ` Jason Merrill
0 siblings, 1 reply; 2+ messages in thread
From: David Malcolm @ 2018-08-29 19:50 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] C++: Fix-it hints for '.' vs '->' (PR c++/84898)
2018-08-29 19:50 [PATCH] C++: Fix-it hints for '.' vs '->' (PR c++/84898) David Malcolm
@ 2018-08-30 19:19 ` Jason Merrill
0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2018-08-30 19:19 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches List
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-08-30 19:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-29 19:50 [PATCH] C++: Fix-it hints for '.' vs '->' (PR c++/84898) David Malcolm
2018-08-30 19:19 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).