diff mbox

[C] Improve warn msg (PR c/43395)

Message ID 20140429211714.GC11802@redhat.com
State New
Headers show

Commit Message

Marek Polacek April 29, 2014, 9:17 p.m. UTC
It's correct to warn about returning an address of a local label,
but it's clumsy to say it's "local variable.  We can easily distinguish
between a label and a variable.

Regtested/bootstrapped on x86_64-linux, ok for trunk?

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

	PR c/43395
c/
	* c-typeck.c (c_finish_return): Distinguish between label and variable
	when warning about returning local address.
cp/
	* typeck.c (maybe_warn_about_returning_address_of_local): Distinguish
	between label and variable when warning about returning local address.
testsuite/
	* c-c++-common/pr43395.c: New test.


	Marek

Comments

Jeff Law April 30, 2014, 9:14 p.m. UTC | #1
On 04/29/14 15:17, Marek Polacek wrote:
> It's correct to warn about returning an address of a local label,
> but it's clumsy to say it's "local variable.  We can easily distinguish
> between a label and a variable.
>
> Regtested/bootstrapped on x86_64-linux, ok for trunk?
>
> 2014-04-29  Marek Polacek  <polacek@redhat.com>
>
> 	PR c/43395
> c/
> 	* c-typeck.c (c_finish_return): Distinguish between label and variable
> 	when warning about returning local address.
> cp/
> 	* typeck.c (maybe_warn_about_returning_address_of_local): Distinguish
> 	between label and variable when warning about returning local address.
> testsuite/
> 	* c-c++-common/pr43395.c: New test.
OK.  Thanks.

Jeff
Joseph Myers May 2, 2014, 5:01 p.m. UTC | #2
On Tue, 29 Apr 2014, Marek Polacek wrote:

> It's correct to warn about returning an address of a local label,
> but it's clumsy to say it's "local variable.  We can easily distinguish
> between a label and a variable.
> 
> Regtested/bootstrapped on x86_64-linux, ok for trunk?

You always need to have complete sentences in diagnostics for the sake of 
translation.  Thus, you need two separate warning_at calls, one with each 
version of the message.  (Using ? : for the whole format string isn't 
sufficient; I think xgettext only extracts one of the two alternatives for 
translation if you do that.)
diff mbox

Patch

diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 62c72df..6c34203 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -9264,7 +9264,8 @@  c_finish_return (location_t loc, tree retval, tree origtype)
 		  && DECL_CONTEXT (inner) == current_function_decl)
 		warning_at (loc,
 			    OPT_Wreturn_local_addr, "function returns address "
-			    "of local variable");
+			    "of %s", TREE_CODE (inner) == LABEL_DECL
+				     ? "label" : "local variable");
 	      break;
 
 	    default:
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 9a80727..5eff7b9 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -8315,8 +8315,9 @@  maybe_warn_about_returning_address_of_local (tree retval)
 	warning (OPT_Wreturn_local_addr, "reference to local variable %q+D returned",
 		 whats_returned);
       else
-	warning (OPT_Wreturn_local_addr, "address of local variable %q+D returned",
-		 whats_returned);
+	warning (OPT_Wreturn_local_addr, "address of %s %q+D returned",
+		 TREE_CODE (whats_returned) == LABEL_DECL
+		 ? "label" : "local variable", whats_returned);
       return;
     }
 }
diff --git gcc/testsuite/c-c++-common/pr43395.c gcc/testsuite/c-c++-common/pr43395.c
index e69de29..92f048d 100644
--- gcc/testsuite/c-c++-common/pr43395.c
+++ gcc/testsuite/c-c++-common/pr43395.c
@@ -0,0 +1,30 @@ 
+/* PR c/43395 */
+/* { dg-do compile } */
+
+void *
+foo (void)
+{
+lab:
+  return &&lab;
+/* { dg-warning "function returns address of label" "" { target c } 8 } */
+/* { dg-warning "address of label" "" { target c++ } 7 } */
+}
+
+void *
+bar (void)
+{
+  __label__ lab;
+lab:
+  return &&lab;
+/* { dg-warning "function returns address of label" "" { target c } 18 } */
+/* { dg-warning "address of label" "" { target c++ } 17 } */
+}
+
+void *
+baz (void)
+{
+  int i;
+  return &i;
+/* { dg-warning "function returns address of local variable" "" { target c } 27 } */
+/* { dg-warning "address of local variable" "" { target c++ } 26 } */
+}