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

login
register
mail settings
Submitter Marek Polacek
Date April 25, 2014, 3:44 p.m.
Message ID <20140425154452.GS2103@redhat.com>
Download mbox | patch
Permalink /patch/342909/
State New
Headers show

Comments

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
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

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" } */
+}