[C PATCH] Clamp down "incomplete type" error (PR c/63543)

[C PATCH] Clamp down "incomplete type" error (PR c/63543)

[Marek Polacek]

We've got a complaint that the "dereferencing pointer to incomplete
type" error is printed for all occurrences of the incomplete type,
which is too verbose.  Also it'd be nicer to print the type as well.
This patch fixes this; if we find an incomplete type, mark it with error
node, then we don't print the error message more than once.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2014-10-15  Marek Polacek  <polacek@redhat.com>

	PR c/63543
	* c-typeck.c (build_indirect_ref): Print the "dereferencing..."
	error only once for each type.  Print also the type.

	* gcc.dg/pr63543.c: New test.

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 5c0697a..f00069c 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -2378,7 +2378,10 @@ build_indirect_ref (location_t loc, tree ptr, ref_operator errstring)
 
 	  if (!COMPLETE_OR_VOID_TYPE_P (t) && TREE_CODE (t) != ARRAY_TYPE)
 	    {
-	      error_at (loc, "dereferencing pointer to incomplete type");
+	      /* Print the error only once for each type.  */
+	      TREE_TYPE (ptr) = error_mark_node;
+	      error_at (loc, "dereferencing pointer to incomplete type %qT",
+			t);
 	      return error_mark_node;
 	    }
 	  if (VOID_TYPE_P (t) && c_inhibit_evaluation_warnings == 0)
diff --git gcc/testsuite/gcc.dg/pr63543.c gcc/testsuite/gcc.dg/pr63543.c
index e69de29..215b62e 100644
--- gcc/testsuite/gcc.dg/pr63543.c
+++ gcc/testsuite/gcc.dg/pr63543.c
@@ -0,0 +1,21 @@
+/* PR c/63543 */
+/* { dg-do compile } */
+
+struct S;
+union U;
+
+int
+f1 (struct S *s)
+{
+  return s->a /* { dg-error "dereferencing pointer to incomplete type .struct S." } */
+	 + s->b
+	 + s->c;
+}
+
+int
+f2 (union U *u)
+{
+  return u->a /* { dg-error "dereferencing pointer to incomplete type .union U." } */
+	 + u->a
+	 + u->a;
+}

	Marek

Re: [C PATCH] Clamp down "incomplete type" error (PR c/63543)

[Jeff Law]

On 10/15/14 11:22, Marek Polacek wrote:
> We've got a complaint that the "dereferencing pointer to incomplete
> type" error is printed for all occurrences of the incomplete type,
> which is too verbose.  Also it'd be nicer to print the type as well.
> This patch fixes this; if we find an incomplete type, mark it with error
> node, then we don't print the error message more than once.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2014-10-15  Marek Polacek  <polacek@redhat.com>
>
> 	PR c/63543
> 	* c-typeck.c (build_indirect_ref): Print the "dereferencing..."
> 	error only once for each type.  Print also the type.
>
> 	* gcc.dg/pr63543.c: New test.
OK.
Jeff

Re: [C PATCH] Clamp down "incomplete type" error (PR c/63543)

[Joseph S. Myers]

On Wed, 15 Oct 2014, Marek Polacek wrote:

> We've got a complaint that the "dereferencing pointer to incomplete
> type" error is printed for all occurrences of the incomplete type,
> which is too verbose.  Also it'd be nicer to print the type as well.
> This patch fixes this; if we find an incomplete type, mark it with error
> node, then we don't print the error message more than once.

I don't like this approach of modifying the type; type nodes are shared 
objects and this could affect all sorts of other logic subsequently 
working with the type.  I think there should be some sort of annotation of 
the type (either in the type itself, or on the side) that *only* means an 
error has been given for the type being incomplete, rather than inserting 
error_mark_node into the type.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [C PATCH] Clamp down "incomplete type" error (PR c/63543)

[Jeff Law]

On 10/15/14 15:46, Joseph S. Myers wrote:
> On Wed, 15 Oct 2014, Marek Polacek wrote:
>
>> We've got a complaint that the "dereferencing pointer to incomplete
>> type" error is printed for all occurrences of the incomplete type,
>> which is too verbose.  Also it'd be nicer to print the type as well.
>> This patch fixes this; if we find an incomplete type, mark it with error
>> node, then we don't print the error message more than once.
>
> I don't like this approach of modifying the type; type nodes are shared
> objects and this could affect all sorts of other logic subsequently
> working with the type.  I think there should be some sort of annotation of
> the type (either in the type itself, or on the side) that *only* means an
> error has been given for the type being incomplete, rather than inserting
> error_mark_node into the type.
Isn't slamming error_mark_node well established at this point?  I fact I 
recall seeing it documented to be used in this kind of way to prevent 
future errors.

jeff

Re: [C PATCH] Clamp down "incomplete type" error (PR c/63543)

[Jakub Jelinek]

On Wed, Oct 15, 2014 at 03:48:20PM -0600, Jeff Law wrote:
> On 10/15/14 15:46, Joseph S. Myers wrote:
> >On Wed, 15 Oct 2014, Marek Polacek wrote:
> >
> >>We've got a complaint that the "dereferencing pointer to incomplete
> >>type" error is printed for all occurrences of the incomplete type,
> >>which is too verbose.  Also it'd be nicer to print the type as well.
> >>This patch fixes this; if we find an incomplete type, mark it with error
> >>node, then we don't print the error message more than once.
> >
> >I don't like this approach of modifying the type; type nodes are shared
> >objects and this could affect all sorts of other logic subsequently
> >working with the type.  I think there should be some sort of annotation of
> >the type (either in the type itself, or on the side) that *only* means an
> >error has been given for the type being incomplete, rather than inserting
> >error_mark_node into the type.
> Isn't slamming error_mark_node well established at this point?  I fact I
> recall seeing it documented to be used in this kind of way to prevent future
> errors.

I think in this case it is way too risky to put error_mark_node there,
it will affect also all the places where the object with that type isn't
dereferenced.
If there are spare bits on the type, either using one for this error, or
to represent some error has been diagnosed for the particular type and
hash table lookup could be used to look up which exactly, or just hash table
without any bit...

	Jakub

Re: [C PATCH] Clamp down "incomplete type" error (PR c/63543)

[Joseph S. Myers]

On Wed, 15 Oct 2014, Jeff Law wrote:

> On 10/15/14 15:46, Joseph S. Myers wrote:
> > On Wed, 15 Oct 2014, Marek Polacek wrote:
> > 
> > > We've got a complaint that the "dereferencing pointer to incomplete
> > > type" error is printed for all occurrences of the incomplete type,
> > > which is too verbose.  Also it'd be nicer to print the type as well.
> > > This patch fixes this; if we find an incomplete type, mark it with error
> > > node, then we don't print the error message more than once.
> > 
> > I don't like this approach of modifying the type; type nodes are shared
> > objects and this could affect all sorts of other logic subsequently
> > working with the type.  I think there should be some sort of annotation of
> > the type (either in the type itself, or on the side) that *only* means an
> > error has been given for the type being incomplete, rather than inserting
> > error_mark_node into the type.
> Isn't slamming error_mark_node well established at this point?  I fact I
> recall seeing it documented to be used in this kind of way to prevent future
> errors.

Returning error_mark_node for the erroneous expression, yes - the 
pre-existing code already does that in this case.  The problem is that the 
insertion of error_mark_node into the type will lead to other uses of that 
type (including ones that have already been processed without errors) 
being affected, and the type itself isn't erroneous.  Indeed, the patch 
would create a "pointer-to-error_mark" type node, which is not something 
code in GCC would ever normally expect to handle 
(build_pointer_type_for_mode just returns error_mark_node if passed 
error_mark_node, so you can't get a POINTER_TYPE whose target is 
error_mark_node that way).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [C PATCH] Clamp down "incomplete type" error (PR c/63543)

[Marek Polacek]

On Wed, Oct 15, 2014 at 09:46:20PM +0000, Joseph S. Myers wrote:
> On Wed, 15 Oct 2014, Marek Polacek wrote:
> 
> > We've got a complaint that the "dereferencing pointer to incomplete
> > type" error is printed for all occurrences of the incomplete type,
> > which is too verbose.  Also it'd be nicer to print the type as well.
> > This patch fixes this; if we find an incomplete type, mark it with error
> > node, then we don't print the error message more than once.
> 
> I don't like this approach of modifying the type; type nodes are shared 
> objects and this could affect all sorts of other logic subsequently 
> working with the type.  I think there should be some sort of annotation of 
> the type (either in the type itself, or on the side) that *only* means an 
> error has been given for the type being incomplete, rather than inserting 
> error_mark_node into the type.

I'm sorry.  Here is another version, which uses a lang flag to distinguish
whether a "dereferencing incomplete..." error message has been given.

One could argue that I should've named the lang flag e.g.
C_TYPE_INCOMPLETE_ERROR_REPORTED, but I hope it can be reused in the
future for some similar purpose.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2014-10-16  Marek Polacek  <polacek@redhat.com>

	PR c/63543
	* c-tree.h (C_TYPE_ERROR_REPORTED): Define.
	* c-typeck.c (build_indirect_ref): Don't print the "dereferencing..."
	error multiple times.  Print the type.

	* gcc.dg/pr63543.c: New test.
	* gcc.dg/array-8.c: Remove dg-error.
	* gcc.dg/pr48552-1.c: Remove and adjust dg-error.
	* gcc.dg/pr48552-2.c: Likewise.

diff --git gcc/c/c-tree.h gcc/c/c-tree.h
index e6aca01..5ff9d9c 100644
--- gcc/c/c-tree.h
+++ gcc/c/c-tree.h
@@ -56,6 +56,9 @@ along with GCC; see the file COPYING3.  If not see
    This is used for -Wc++-compat. */
 #define C_TYPE_DEFINED_IN_STRUCT(TYPE) TYPE_LANG_FLAG_2 (TYPE)
 
+/* Record whether an "incomplete type" error was given for the type.  */
+#define C_TYPE_ERROR_REPORTED(TYPE) TYPE_LANG_FLAG_3 (TYPE)
+
 /* Record whether a typedef for type `int' was actually `signed int'.  */
 #define C_TYPEDEF_EXPLICITLY_SIGNED(EXP) DECL_LANG_FLAG_1 (EXP)
 
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 5c0697a..0f363f5 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -2378,7 +2378,12 @@ build_indirect_ref (location_t loc, tree ptr, ref_operator errstring)
 
 	  if (!COMPLETE_OR_VOID_TYPE_P (t) && TREE_CODE (t) != ARRAY_TYPE)
 	    {
-	      error_at (loc, "dereferencing pointer to incomplete type");
+	      if (!C_TYPE_ERROR_REPORTED (TREE_TYPE (ptr)))
+		{
+		  error_at (loc, "dereferencing pointer to incomplete type "
+			    "%qT", t);
+		  C_TYPE_ERROR_REPORTED (TREE_TYPE (ptr)) = 1;
+		}
 	      return error_mark_node;
 	    }
 	  if (VOID_TYPE_P (t) && c_inhibit_evaluation_warnings == 0)
diff --git gcc/testsuite/gcc.dg/array-8.c gcc/testsuite/gcc.dg/array-8.c
index d469a80..2872985 100644
--- gcc/testsuite/gcc.dg/array-8.c
+++ gcc/testsuite/gcc.dg/array-8.c
@@ -45,5 +45,4 @@ g (void)
   sip[0]; /* { dg-error "invalid use of undefined type 'struct si'" } */
   /* { dg-error "dereferencing pointer to incomplete type" "incomplete" { target *-*-* } 45 } */
   0[sip]; /* { dg-error "invalid use of undefined type 'struct si'" } */
-  /* { dg-error "dereferencing pointer to incomplete type" "incomplete" { target *-*-* } 47 } */
 }
diff --git gcc/testsuite/gcc.dg/pr48552-1.c gcc/testsuite/gcc.dg/pr48552-1.c
index 877c4c2..4b833fb 100644
--- gcc/testsuite/gcc.dg/pr48552-1.c
+++ gcc/testsuite/gcc.dg/pr48552-1.c
@@ -49,5 +49,5 @@ f7 (struct S *x)
 void
 f8 (struct S *x)
 {
-  __asm volatile ("" : "=r" (*x));	/* { dg-error "dereferencing pointer to incomplete type" "incomplete" } */
-}					/* { dg-error "invalid lvalue in asm output 0" "invalid lvalue" { target *-*-* } 52 } */
+  __asm volatile ("" : "=r" (*x));	/* { dg-error "invalid lvalue in asm output 0" } */
+}
diff --git gcc/testsuite/gcc.dg/pr48552-2.c gcc/testsuite/gcc.dg/pr48552-2.c
index a796983..954c411 100644
--- gcc/testsuite/gcc.dg/pr48552-2.c
+++ gcc/testsuite/gcc.dg/pr48552-2.c
@@ -49,5 +49,5 @@ f7 (struct S *x)
 void
 f8 (struct S *x)
 {
-  __asm ("" : "=r" (*x));	/* { dg-error "dereferencing pointer to incomplete type" "incomplete" } */
-}				/* { dg-error "invalid lvalue in asm output 0" "invalid lvalue" { target *-*-* } 52 } */
+  __asm ("" : "=r" (*x));	/* { dg-error "invalid lvalue in asm output 0" } */
+}
diff --git gcc/testsuite/gcc.dg/pr63543.c gcc/testsuite/gcc.dg/pr63543.c
index e69de29..215b62e 100644
--- gcc/testsuite/gcc.dg/pr63543.c
+++ gcc/testsuite/gcc.dg/pr63543.c
@@ -0,0 +1,21 @@
+/* PR c/63543 */
+/* { dg-do compile } */
+
+struct S;
+union U;
+
+int
+f1 (struct S *s)
+{
+  return s->a /* { dg-error "dereferencing pointer to incomplete type .struct S." } */
+	 + s->b
+	 + s->c;
+}
+
+int
+f2 (union U *u)
+{
+  return u->a /* { dg-error "dereferencing pointer to incomplete type .union U." } */
+	 + u->a
+	 + u->a;
+}

	Marek

Re: [C PATCH] Clamp down "incomplete type" error (PR c/63543)

[Marek Polacek]

On Wed, Oct 15, 2014 at 11:52:59PM +0200, Jakub Jelinek wrote:
> On Wed, Oct 15, 2014 at 03:48:20PM -0600, Jeff Law wrote:
> > Isn't slamming error_mark_node well established at this point?  I fact I
> > recall seeing it documented to be used in this kind of way to prevent future
> > errors.
> 
> I think in this case it is way too risky to put error_mark_node there,
> it will affect also all the places where the object with that type isn't
> dereferenced.
> If there are spare bits on the type, either using one for this error, or
> to represent some error has been diagnosed for the particular type and
> hash table lookup could be used to look up which exactly, or just hash table
> without any bit...

I thought sticking error_mark_node into the type would be feasible
here, because we do it elsewhere too, and the program is invalid
anyway.

Well - maybe the new patch is better.

Thanks,

	Marek

Re: [C PATCH] Clamp down "incomplete type" error (PR c/63543)

[Jeff Law]

On 10/16/14 11:20, Marek Polacek wrote:
> On Wed, Oct 15, 2014 at 09:46:20PM +0000, Joseph S. Myers wrote:
>> On Wed, 15 Oct 2014, Marek Polacek wrote:
>>
>>> We've got a complaint that the "dereferencing pointer to incomplete
>>> type" error is printed for all occurrences of the incomplete type,
>>> which is too verbose.  Also it'd be nicer to print the type as well.
>>> This patch fixes this; if we find an incomplete type, mark it with error
>>> node, then we don't print the error message more than once.
>>
>> I don't like this approach of modifying the type; type nodes are shared
>> objects and this could affect all sorts of other logic subsequently
>> working with the type.  I think there should be some sort of annotation of
>> the type (either in the type itself, or on the side) that *only* means an
>> error has been given for the type being incomplete, rather than inserting
>> error_mark_node into the type.
>
> I'm sorry.  Here is another version, which uses a lang flag to distinguish
> whether a "dereferencing incomplete..." error message has been given.
>
> One could argue that I should've named the lang flag e.g.
> C_TYPE_INCOMPLETE_ERROR_REPORTED, but I hope it can be reused in the
> future for some similar purpose.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2014-10-16  Marek Polacek  <polacek@redhat.com>
>
> 	PR c/63543
> 	* c-tree.h (C_TYPE_ERROR_REPORTED): Define.
> 	* c-typeck.c (build_indirect_ref): Don't print the "dereferencing..."
> 	error multiple times.  Print the type.
>
> 	* gcc.dg/pr63543.c: New test.
> 	* gcc.dg/array-8.c: Remove dg-error.
> 	* gcc.dg/pr48552-1.c: Remove and adjust dg-error.
> 	* gcc.dg/pr48552-2.c: Likewise.
OK.

It's probably safe to assume we'll find more uses for that flag.

jeff