diff mbox

C: fix reported range of invalid unary dereference.

Message ID 1450805189-14333-1-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Dec. 22, 2015, 5:26 p.m. UTC
Currently, trunk emits this for a bad unary * in C:

bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have ‘int’)
   return *some_f.x;
          ^

The following patch fixes the reported location to highlight the
expression that was attempted to be dereferenced:

bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have ‘int’)
   return *some_f.x;
          ^~~~~~~~~

Based on another example from:
  http://clang.llvm.org/diagnostics.html
albeit within the "Precision in Wording" example; I didn't change the
wording.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
OK for trunk in stage 3?

gcc/c/ChangeLog:
	* c-parser.c (c_parser_unary_expression): For dereferences, build
	a combined location before calling build_indirect_ref, so that
	error reports cover the full range, manually updating the c_expr
	src_range.

gcc/testsuite/ChangeLog:
	* gcc.dg/bad-dereference.c: New test case.
---
 gcc/c/c-parser.c                       |  8 ++++++--
 gcc/testsuite/gcc.dg/bad-dereference.c | 24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/bad-dereference.c

Comments

Trevor Saunders Dec. 22, 2015, 5:32 p.m. UTC | #1
On Tue, Dec 22, 2015 at 12:26:29PM -0500, David Malcolm wrote:
> Currently, trunk emits this for a bad unary * in C:
> 
> bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have ‘int’)
>    return *some_f.x;
>           ^
> 
> The following patch fixes the reported location to highlight the
> expression that was attempted to be dereferenced:
> 
> bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have ‘int’)
>    return *some_f.x;
>           ^~~~~~~~~
> 
> Based on another example from:
>   http://clang.llvm.org/diagnostics.html
> albeit within the "Precision in Wording" example; I didn't change the
> wording.

fwiw gcc's wording seems better to me.

> +++ b/gcc/c/c-parser.c
> @@ -6691,8 +6691,12 @@ c_parser_unary_expression (c_parser *parser)
>        op = c_parser_cast_expression (parser, NULL);
>        finish = op.get_finish ();
>        op = convert_lvalue_to_rvalue (exp_loc, op, true, true);
> -      ret.value = build_indirect_ref (op_loc, op.value, RO_UNARY_STAR);
> -      set_c_expr_source_range (&ret, op_loc, finish);
> +      {

I'd find these braces less confusing if they were for the entire case
not just part of it.

Trev
Jeff Law Dec. 23, 2015, 6:43 a.m. UTC | #2
On 12/22/2015 10:26 AM, David Malcolm wrote:
> Currently, trunk emits this for a bad unary * in C:
>
> bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have ‘int’)
>     return *some_f.x;
>            ^
>
> The following patch fixes the reported location to highlight the
> expression that was attempted to be dereferenced:
>
> bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have ‘int’)
>     return *some_f.x;
>            ^~~~~~~~~
>
> Based on another example from:
>    http://clang.llvm.org/diagnostics.html
> albeit within the "Precision in Wording" example; I didn't change the
> wording.
>
> Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
> OK for trunk in stage 3?
>
> gcc/c/ChangeLog:
> 	* c-parser.c (c_parser_unary_expression): For dereferences, build
> 	a combined location before calling build_indirect_ref, so that
> 	error reports cover the full range, manually updating the c_expr
> 	src_range.
>
> gcc/testsuite/ChangeLog:
> 	* gcc.dg/bad-dereference.c: New test case.
I agree with Trevor here, move those curlys out to enclose the entire 
switch case.  It'll mean reformatting, but, meh...

Approved with that change.

jeff
diff mbox

Patch

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 774354a..a22c3d2 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -6691,8 +6691,12 @@  c_parser_unary_expression (c_parser *parser)
       op = c_parser_cast_expression (parser, NULL);
       finish = op.get_finish ();
       op = convert_lvalue_to_rvalue (exp_loc, op, true, true);
-      ret.value = build_indirect_ref (op_loc, op.value, RO_UNARY_STAR);
-      set_c_expr_source_range (&ret, op_loc, finish);
+      {
+	location_t combined_loc = make_location (op_loc, op_loc, finish);
+	ret.value = build_indirect_ref (combined_loc, op.value, RO_UNARY_STAR);
+	ret.src_range.m_start = op_loc;
+	ret.src_range.m_finish = finish;
+      }
       return ret;
     case CPP_PLUS:
       if (!c_dialect_objc () && !in_system_header_at (input_location))
diff --git a/gcc/testsuite/gcc.dg/bad-dereference.c b/gcc/testsuite/gcc.dg/bad-dereference.c
new file mode 100644
index 0000000..5f8188d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/bad-dereference.c
@@ -0,0 +1,24 @@ 
+/* { dg-options "-fdiagnostics-show-caret" } */
+
+struct foo
+{
+  int x;
+};
+
+int test_1 (struct foo some_f)
+{
+  return *some_f.x; /* { dg-error "invalid type argument of unary ... .have .int.." } */
+/* { dg-begin-multiline-output "" }
+   return *some_f.x;
+          ^~~~~~~~~
+   { dg-end-multiline-output "" } */
+}
+
+int test_2 (struct foo some_f)
+{
+  return *some_f; /* { dg-error "invalid type argument of unary ... .have .struct foo.." } */
+/* { dg-begin-multiline-output "" }
+   return *some_f;
+          ^~~~~~~
+   { dg-end-multiline-output "" } */
+}