c: Improve build_component_ref diagnostics [PR91134]

On the following testcase (the first dg-error line) we emit a weird
diagnostics and even fixit on pointerpointer->member
where pointerpointer is pointer to pointer to struct and we say
'pointerpointer' is a pointer; did you mean to use '->'?
The first part is indeed true, but suggesting -> when the code already
does use -> is confusing.
The following patch adjusts callers so that they tell it if it is from
. parsing or from -> parsing and in the latter case suggests to dereference
the left operand instead by adding (* before it and ) after it (before ->).
Or would a suggestion to add [0] before -> be better?

2022-05-25  Jakub Jelinek  <jakub@redhat.com>

	PR c/91134
gcc/c/
	* c-tree.h (build_component_ref): Add ARROW_LOC location_t argument.
	* c-typeck.cc (build_component_ref): Likewise.  If DATUM is
	INDIRECT_REF and ARROW_LOC isn't UNKNOWN_LOCATION, print a different
	diagnostic and fixit hint if DATUM has pointer type.
	* c-parser.cc (c_parser_postfix_expression,
	c_parser_omp_variable_list): Adjust build_component_ref callers.
	* gimple-parser.cc (c_parser_gimple_postfix_expression_after_primary):
	Likewise.
gcc/objc/
	* objc-act.cc (objc_build_component_ref): Adjust build_component_ref
	caller.
gcc/testsuite/
	* gcc.dg/pr91134.c: New test.

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 492d995a281a358d0a6d1a1d73487e2dbd385a97..aa24b431769f70ac2845369332198ec095778dc9 100644 (file)
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -9235,8 +9235,9 @@ c_parser_postfix_expression (c_parser *parser)
            if (c_parser_next_token_is (parser, CPP_NAME))
              {
                c_token *comp_tok = c_parser_peek_token (parser);
-               offsetof_ref = build_component_ref
-                 (loc, offsetof_ref, comp_tok->value, comp_tok->location);
+               offsetof_ref
+                 = build_component_ref (loc, offsetof_ref, comp_tok->value,
+                                        comp_tok->location, UNKNOWN_LOCATION);
                c_parser_consume_token (parser);
                while (c_parser_next_token_is (parser, CPP_DOT)
                       || c_parser_next_token_is (parser,
@@ -9263,9 +9264,11 @@ c_parser_postfix_expression (c_parser *parser)
                            break;
                          }
                        c_token *comp_tok = c_parser_peek_token (parser);
-                       offsetof_ref = build_component_ref
-                         (loc, offsetof_ref, comp_tok->value,
-                          comp_tok->location);
+                       offsetof_ref
+                         = build_component_ref (loc, offsetof_ref,
+                                                comp_tok->value,
+                                                comp_tok->location,
+                                                UNKNOWN_LOCATION);
                        c_parser_consume_token (parser);
                      }
                    else
@@ -10612,7 +10615,7 @@ c_parser_postfix_expression_after_primary (c_parser *parser,
          finish = c_parser_peek_token (parser)->get_finish ();
          c_parser_consume_token (parser);
          expr.value = build_component_ref (op_loc, expr.value, ident,
-                                           comp_loc);
+                                           comp_loc, UNKNOWN_LOCATION);
          set_c_expr_source_range (&expr, start, finish);
          expr.original_code = ERROR_MARK;
          if (TREE_CODE (expr.value) != COMPONENT_REF)
@@ -10652,7 +10655,8 @@ c_parser_postfix_expression_after_primary (c_parser *parser,
                                            build_indirect_ref (op_loc,
                                                                expr.value,
                                                                RO_ARROW),
-                                           ident, comp_loc);
+                                           ident, comp_loc,
+                                           expr.get_location ());
          set_c_expr_source_range (&expr, start, finish);
          expr.original_code = ERROR_MARK;
          if (TREE_CODE (expr.value) != COMPONENT_REF)
@@ -13171,6 +13175,7 @@ c_parser_omp_variable_list (c_parser *parser,
                         && c_parser_next_token_is (parser, CPP_DEREF)))
                {
                  location_t op_loc = c_parser_peek_token (parser)->location;
+                 location_t arrow_loc = UNKNOWN_LOCATION;
                  if (c_parser_next_token_is (parser, CPP_DEREF))
                    {
                      c_expr t_expr;
@@ -13181,6 +13186,7 @@ c_parser_omp_variable_list (c_parser *parser,
                      t_expr = convert_lvalue_to_rvalue (op_loc, t_expr,
                                                         true, false);
                      t = build_indirect_ref (op_loc, t_expr.value, RO_ARROW);
+                     arrow_loc = t_expr.get_location ();
                    }
                  c_parser_consume_token (parser);
                  if (!c_parser_next_token_is (parser, CPP_NAME))
@@ -13194,7 +13200,8 @@ c_parser_omp_variable_list (c_parser *parser,
                  tree ident = comp_tok->value;
                  location_t comp_loc = comp_tok->location;
                  c_parser_consume_token (parser);
-                 t = build_component_ref (op_loc, t, ident, comp_loc);
+                 t = build_component_ref (op_loc, t, ident, comp_loc,
+                                          arrow_loc);
                }
              /* FALLTHROUGH  */
            case OMP_CLAUSE_AFFINITY:

diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 2bcb9662620a27c793870bbac3c7cfbc7c7bd850..3b322ad741a9c211eda3280735ef03eaf435eda2 100644 (file)
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -699,7 +699,8 @@ extern struct c_expr convert_lvalue_to_rvalue (location_t, struct c_expr,
 extern tree decl_constant_value_1 (tree, bool);
 extern void mark_exp_read (tree);
 extern tree composite_type (tree, tree);
-extern tree build_component_ref (location_t, tree, tree, location_t);
+extern tree build_component_ref (location_t, tree, tree, location_t,
+                                location_t);
 extern tree build_array_ref (location_t, tree, tree);
 extern tree build_external_ref (location_t, tree, bool, tree *);
 extern void pop_maybe_used (bool);

diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 4f3611f1b89583b04fc886ee6b36346f0b379a0b..6b05eddfdf45c615d3de48fa73fb41ded28f1486 100644 (file)
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -2457,11 +2457,12 @@ should_suggest_deref_p (tree datum_type)
 /* Make an expression to refer to the COMPONENT field of structure or
    union value DATUM.  COMPONENT is an IDENTIFIER_NODE.  LOC is the
    location of the COMPONENT_REF.  COMPONENT_LOC is the location
-   of COMPONENT.  */
+   of COMPONENT.  ARROW_LOC is the location of the first -> operand if
+   it is from -> operator.  */
 
 tree
 build_component_ref (location_t loc, tree datum, tree component,
-                    location_t component_loc)
+                    location_t component_loc, location_t arrow_loc)
 {
   tree type = TREE_TYPE (datum);
   enum tree_code code = TREE_CODE (type);
@@ -2577,11 +2578,23 @@ build_component_ref (location_t loc, tree datum, tree component,
       /* Special-case the error message for "ptr.field" for the case
         where the user has confused "." vs "->".  */
       rich_location richloc (line_table, loc);
-      /* "loc" should be the "." token.  */
-      richloc.add_fixit_replace ("->");
-      error_at (&richloc,
-               "%qE is a pointer; did you mean to use %<->%>?",
-               datum);
+      if (TREE_CODE (datum) == INDIRECT_REF && arrow_loc != UNKNOWN_LOCATION)
+       {
+         richloc.add_fixit_insert_before (arrow_loc, "(*");
+         richloc.add_fixit_insert_after (arrow_loc, ")");
+         error_at (&richloc,
+                   "%qE is a pointer to pointer; did you mean to dereference "
+                   "it before applying %<->%> to it?",
+                   TREE_OPERAND (datum, 0));
+       }
+      else
+       {
+         /* "loc" should be the "." token.  */
+         richloc.add_fixit_replace ("->");
+         error_at (&richloc,
+                   "%qE is a pointer; did you mean to use %<->%>?",
+                   datum);
+       }
       return error_mark_node;
     }
   else if (code != ERROR_MARK)

diff --git a/gcc/c/gimple-parser.cc b/gcc/c/gimple-parser.cc
index d1afd42556ce17f45c25c1dd235d296315bff024..b909eac498d60b313dd1e628ee532d0f3771f768 100644 (file)
--- a/gcc/c/gimple-parser.cc
+++ b/gcc/c/gimple-parser.cc
@@ -1800,7 +1800,7 @@ c_parser_gimple_postfix_expression_after_primary (gimple_parser &parser,
            finish = c_parser_peek_token (parser)->get_finish ();
            c_parser_consume_token (parser);
            expr.value = build_component_ref (op_loc, expr.value, ident,
-                                             comp_loc);
+                                             comp_loc, UNKNOWN_LOCATION);
            set_c_expr_source_range (&expr, start, finish);
            expr.original_code = ERROR_MARK;
            if (TREE_CODE (expr.value) != COMPONENT_REF)
@@ -1848,7 +1848,8 @@ c_parser_gimple_postfix_expression_after_primary (gimple_parser &parser,
            expr.value = build_component_ref (op_loc,
                                              build_simple_mem_ref_loc
                                                (op_loc, expr.value),
-                                             ident, comp_loc);
+                                             ident, comp_loc,
+                                             expr.get_location ());
            set_c_expr_source_range (&expr, start, finish);
            expr.original_code = ERROR_MARK;
            if (TREE_CODE (expr.value) != COMPONENT_REF)

diff --git a/gcc/objc/objc-act.cc b/gcc/objc/objc-act.cc
index 252274c8f5d458e60a1cb4f51db9e7c77f36d600..10591bb75f1ea6d7b9f4199f7ac25b3562eae7c9 100644 (file)
--- a/gcc/objc/objc-act.cc
+++ b/gcc/objc/objc-act.cc
@@ -2812,7 +2812,7 @@ objc_build_component_ref (tree datum, tree component)
                                           tf_warning_or_error);
 #else
   return build_component_ref (input_location, datum, component,
-                             UNKNOWN_LOCATION);
+                             UNKNOWN_LOCATION, UNKNOWN_LOCATION);
 #endif
 }
 

diff --git a/gcc/testsuite/gcc.dg/pr91134.c b/gcc/testsuite/gcc.dg/pr91134.c
new file mode 100644 (file)
index 0000000..8844f42
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr91134.c
@@ -0,0 +1,32 @@
+/* PR c/91134 */
+/* { dg-options "-fdiagnostics-show-caret" } */
+
+struct X { int member; } x;
+struct Y { struct X **x; } y;
+
+int
+foo (void)
+{
+  struct X *pointer = &x;
+  struct Y *yp = &y;
+  struct X **pointerpointer = &pointer;
+  int i = *pointerpointer->member;     /* { dg-error "'pointerpointer' is a pointer to pointer; did you mean to dereference it before applying '->' to it\\\?" } */
+/* { dg-begin-multiline-output "" }
+   int i = *pointerpointer->member;
+                          ^~
+            (*            )
+   { dg-end-multiline-output "" } */
+  int j = pointer.member;              /* { dg-error "'pointer' is a pointer; did you mean to use '->'\\\?" } */
+/* { dg-begin-multiline-output "" }
+   int j = pointer.member;
+                  ^
+                  ->
+   { dg-end-multiline-output "" } */
+  int k = yp->x->member;               /* { dg-error "'yp->x' is a pointer to pointer; did you mean to dereference it before applying '->' to it\\\?" } */
+/* { dg-begin-multiline-output "" }
+   int k = yp->x->member;
+                ^~
+           (*   )
+   { dg-end-multiline-output "" } */
+  return i + j + k;
+}