Re: [patch] Hide the caret for -Wstack-usage
> I personally don't like much this pattern of saving a variable, doing
> something and then restoring it.
Right, it's kludgy, so I have only changed the location in the end.
Tested on x86_64-suse-linux, applied on all active branches as obvious.
2014-07-19 Eric Botcazou
* toplev.c (output_stack_usage): Adjust the location of the warning.
2014-07-19 Eric Botcazou
* gcc.dg/stack-usage-2.c: Adjust.
--
Eric BotcazouIndex: toplev.c
===
--- toplev.c (revision 212833)
+++ toplev.c (working copy)
@@ -1052,16 +1052,19 @@ output_stack_usage (void)
if (warn_stack_usage >= 0)
{
+ const location_t loc = DECL_SOURCE_LOCATION (current_function_decl);
+
if (stack_usage_kind == DYNAMIC)
- warning (OPT_Wstack_usage_, "stack usage might be unbounded");
+ warning_at (loc, OPT_Wstack_usage_, "stack usage might be unbounded");
else if (stack_usage > warn_stack_usage)
{
if (stack_usage_kind == DYNAMIC_BOUNDED)
- warning (OPT_Wstack_usage_, "stack usage might be %wd bytes",
- stack_usage);
+ warning_at (loc,
+ OPT_Wstack_usage_, "stack usage might be %wd bytes",
+ stack_usage);
else
- warning (OPT_Wstack_usage_, "stack usage is %wd bytes",
- stack_usage);
+ warning_at (loc, OPT_Wstack_usage_, "stack usage is %wd bytes",
+ stack_usage);
}
}
}
Index: testsuite/gcc.dg/stack-usage-2.c
===
--- testsuite/gcc.dg/stack-usage-2.c (revision 212833)
+++ testsuite/gcc.dg/stack-usage-2.c (working copy)
@@ -1,33 +1,32 @@
/* { dg-do compile } */
/* { dg-options "-Wstack-usage=512" } */
-int foo1 (void)
+int foo1 (void) /* { dg-bogus "stack usage" } */
{
char arr[16];
arr[0] = 1;
return 0;
-} /* { dg-bogus "stack usage" } */
+}
-int foo2 (void)
+int foo2 (void) /* { dg-warning "stack usage is \[0-9\]* bytes" } */
{
char arr[1024];
arr[0] = 1;
return 0;
-} /* { dg-warning "stack usage is \[0-9\]* bytes" } */
+}
-int foo3 (void)
+int foo3 (void) /* { dg-warning "stack usage might be \[0-9\]* bytes" } */
{
char arr[1024] __attribute__((aligned (512)));
arr[0] = 1;
/* Force dynamic realignment of argument pointer. */
__builtin_apply ((void (*)()) foo2, 0, 0);
return 0;
+}
-} /* { dg-warning "stack usage might be \[0-9\]* bytes" } */
-
-int foo4 (int n)
+int foo4 (int n) /* { dg-warning "stack usage might be unbounded" } */
{
char arr[n];
arr[0] = 1;
return 0;
-} /* { dg-warning "stack usage might be unbounded" } */
+}
Re: [patch] Hide the caret for -Wstack-usage
On 12 July 2014 03:39, Eric Botcazou wrote: >> Your patch hides the caret but the location still points to the >> closing brace (so if you use an IDE like emacs this is where you will >> go). >> >> Is there a location for the definition? Could we store/pass one to this >> point? >> >> Otherwise, would it be appropriate to point to the declaration of the >> function? After all, this is what -fstack-usage seems to do. > > Good idea, let's use a consistent location, revised patch attached, thanks. I personally don't like much this pattern of saving a variable, doing something and then restoring it. This old patch: https://gcc.gnu.org/ml/gcc-patches/2012-04/msg01836.html added control flags to diagnostics, like we have for the various dump_ functions. This seems cleaner to me, but it may be a matter of style. Otherwise, this looks like an improvement overall. Cheers, Manuel.
Re: [patch] Hide the caret for -Wstack-usage
> Your patch hides the caret but the location still points to the
> closing brace (so if you use an IDE like emacs this is where you will
> go).
>
> Is there a location for the definition? Could we store/pass one to this
> point?
>
> Otherwise, would it be appropriate to point to the declaration of the
> function? After all, this is what -fstack-usage seems to do.
Good idea, let's use a consistent location, revised patch attached, thanks.
2014-07-12 Eric Botcazou
* toplev.c (output_stack_usage): Adjust the location and temporarily
hide the caret.
2014-07-12 Eric Botcazou
* gcc.dg/stack-usage-2.c: Adjust.
--
Eric BotcazouIndex: toplev.c
===
--- toplev.c (revision 212477)
+++ toplev.c (working copy)
@@ -1052,17 +1052,24 @@ output_stack_usage (void)
if (warn_stack_usage >= 0)
{
+ const location_t loc = DECL_SOURCE_LOCATION (current_function_decl);
+ /* Avoid displaying the caret which is useless here. */
+ const bool saved_show_caret = global_dc->show_caret;
+ global_dc->show_caret = false;
+
if (stack_usage_kind == DYNAMIC)
- warning (OPT_Wstack_usage_, "stack usage might be unbounded");
+ warning_at (loc, OPT_Wstack_usage_, "stack usage might be unbounded");
else if (stack_usage > warn_stack_usage)
{
if (stack_usage_kind == DYNAMIC_BOUNDED)
- warning (OPT_Wstack_usage_, "stack usage might be %wd bytes",
- stack_usage);
+ warning_at (loc, OPT_Wstack_usage_, "stack usage might be %wd bytes",
+ stack_usage);
else
- warning (OPT_Wstack_usage_, "stack usage is %wd bytes",
- stack_usage);
+ warning_at (loc, OPT_Wstack_usage_, "stack usage is %wd bytes",
+ stack_usage);
}
+
+ global_dc->show_caret = saved_show_caret;
}
}
Index: testsuite/gcc.dg/stack-usage-2.c
===
--- testsuite/gcc.dg/stack-usage-2.c (revision 212477)
+++ testsuite/gcc.dg/stack-usage-2.c (working copy)
@@ -1,33 +1,32 @@
/* { dg-do compile } */
/* { dg-options "-Wstack-usage=512" } */
-int foo1 (void)
+int foo1 (void) /* { dg-bogus "stack usage" } */
{
char arr[16];
arr[0] = 1;
return 0;
-} /* { dg-bogus "stack usage" } */
+}
-int foo2 (void)
+int foo2 (void) /* { dg-warning "stack usage is \[0-9\]* bytes" } */
{
char arr[1024];
arr[0] = 1;
return 0;
-} /* { dg-warning "stack usage is \[0-9\]* bytes" } */
+}
-int foo3 (void)
+int foo3 (void) /* { dg-warning "stack usage might be \[0-9\]* bytes" } */
{
char arr[1024] __attribute__((aligned (512)));
arr[0] = 1;
/* Force dynamic realignment of argument pointer. */
__builtin_apply ((void (*)()) foo2, 0, 0);
return 0;
+}
-} /* { dg-warning "stack usage might be \[0-9\]* bytes" } */
-
-int foo4 (int n)
+int foo4 (int n) /* { dg-warning "stack usage might be unbounded" } */
{
char arr[n];
arr[0] = 1;
return 0;
-} /* { dg-warning "stack usage might be unbounded" } */
+}
RE: [patch] Hide the caret for -Wstack-usage
> since displaying the caret was made the default, the -Wstack-usage warning has > a strange-looking final part: > > stack-usage-2.c: In function 'foo2': > stack-usage-2.c:16:1: warning: stack usage is 920 bytes [-Wstack-usage=] > } > ^ Your patch hides the caret but the location still points to the closing brace (so if you use an IDE like emacs this is where you will go). Is there a location for the definition? Could we store/pass one to this point? Otherwise, would it be appropriate to point to the declaration of the function? After all, this is what -fstack-usage seems to do. Then you could use warning_at(loc, ...). Cheers, Manuel.
