[C] Better column info for initializers (PR c/60139)

Submitted by Marek Polacek on April 25, 2014, 3:44 p.m.

Details

Message ID 20140425154452.GS2103@redhat.com
State New
Headers show

Commit Message

Marek Polacek April 25, 2014, 3:44 p.m.
Another minor fix: use loc instead of input_location.  Also add
missing OPT_Wpedantic. 

After this is in, my plan is to make pedwarn_init and error_init
static (I already have a patch for that) and then add location
argument to error_init and pass proper location to it, and to
pedwarn_init as well -- that should greatly improve initializer
warnings/errors.

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

2014-04-25  Marek Polacek  <polacek@redhat.com>

	PR c/60139
	* c-typeck.c (output_init_element): Pass OPT_Wpedantic to pedwarn
	and pedwarn_init.  Use loc insted of input_location.

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


	Marek

Comments

Richard Henderson April 29, 2014, 7 p.m.
On 04/25/2014 08:44 AM, Marek Polacek wrote:
> 2014-04-25  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c/60139
> 	* c-typeck.c (output_init_element): Pass OPT_Wpedantic to pedwarn
> 	and pedwarn_init.  Use loc insted of input_location.
> 
> 	* gcc.dg/pr60139.c: New test.

Ok.


r~
Joseph S. Myers May 1, 2014, 11:55 p.m.
On Fri, 25 Apr 2014, Marek Polacek wrote:

> Another minor fix: use loc instead of input_location.  Also add
> missing OPT_Wpedantic. 

Why do you say it's missing?  A default pedwarn generally means "this is 
dubious code, not allowed by ISO C, but we don't want to make it a hard 
error" (and the trend is more to making such things hard errors, rather 
than limiting them to -pedantic).

For the *first* such change ("not computable at load time"), I think it's 
already conditional on (pedantic && !flag_isoc99), based on the setting of 
require_constant_elements, and so using OPT_Wpedantic is appropriate.  But 
I think the second represents an undesirable change to how the compiler 
behaves - code such as

int f();
int a = 1 ? 1 : f();

*should* be diagnosed by default rather than needing -pedantic.  If you 
think this case also only affects for form of diagnostics that were 
already only given if pedantic, please explain further why this is the 
case.
Marek Polacek May 5, 2014, 12:47 p.m.
On Thu, May 01, 2014 at 11:55:38PM +0000, Joseph S. Myers wrote:
> On Fri, 25 Apr 2014, Marek Polacek wrote:
> 
> > Another minor fix: use loc instead of input_location.  Also add
> > missing OPT_Wpedantic. 
> 
> Why do you say it's missing?  A default pedwarn generally means "this is 
> dubious code, not allowed by ISO C, but we don't want to make it a hard 
> error" (and the trend is more to making such things hard errors, rather 
> than limiting them to -pedantic).
> 
> For the *first* such change ("not computable at load time"), I think it's 
> already conditional on (pedantic && !flag_isoc99), based on the setting of 
> require_constant_elements, and so using OPT_Wpedantic is appropriate.  But 
> I think the second represents an undesirable change to how the compiler 
> behaves - code such as
> 
> int f();
> int a = 1 ? 1 : f();
> 
> *should* be diagnosed by default rather than needing -pedantic.  If you 

Yeah, this case is even now diagnosted by default (for this we warn in
digest_init).

> think this case also only affects for form of diagnostics that were 
> already only given if pedantic, please explain further why this is the 
> case.

Right.  I added OPT_Wpedantic to second pedwarn_init ("initializer
element is not a constant expression"), because this warning was
output only when using -pedantic and C89, but it didn't say "[-Wpedantic]",
thus it looked like the warning is enabled by default.  The reason
is that this pedwarn is guarded by require_constant_elements, which
is, as you said, dependent on (pedantic && !flag_isoc99) in
start_init.  Just for the record, this pedwarn warns e.g. on

double sin (double);
void
fn (int *p)
{
  double d[] = { sin (1.0) };
}

So the patch doesn't suppress any warnings, it only adds
"[-Wpedantic]" where it should be (there might be other cases where 
"[-Wpedantic]" is missing).

	Marek

Patch hide | download patch | download mbox

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 62c72df..a82801f 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -8255,12 +8255,12 @@  output_init_element (location_t loc, tree value, tree origtype,
 	  value = error_mark_node;
 	}
       else if (require_constant_elements)
-	pedwarn (input_location, 0,
+	pedwarn (loc, OPT_Wpedantic,
 		 "initializer element is not computable at load time");
     }
   else if (!maybe_const
 	   && (require_constant_value || require_constant_elements))
-    pedwarn_init (input_location, 0,
+    pedwarn_init (loc, OPT_Wpedantic,
 		  "initializer element is not a constant expression");
 
   /* Issue -Wc++-compat warnings about initializing a bitfield with
diff --git gcc/testsuite/gcc.dg/pr60139.c gcc/testsuite/gcc.dg/pr60139.c
index e69de29..a63d8b5 100644
--- gcc/testsuite/gcc.dg/pr60139.c
+++ gcc/testsuite/gcc.dg/pr60139.c
@@ -0,0 +1,14 @@ 
+/* PR c/60139 */
+/* { dg-do compile } */
+/* { dg-options "-Wpedantic" } */
+/* { dg-prune-output ".*near initialization for.*" } */
+
+double sin (double);
+void
+fn (int *p)
+{
+  int **a[] = { &p, /* { dg-warning "17:initializer element is not computable at load time" } */
+               (void *) 0, &p }; /* { dg-warning "28:initializer element is not computable at load time" } */
+  double d[] = { sin (1.0), /* { dg-warning "18:initializer element is not a constant expression" } */
+                 8.8, sin (1.0), 2.6 }; /* { dg-warning "23:initializer element is not a constant expression" } */
+}