Patchwork PR c/21659: missing "weak declaration must precede definition" error

login
register
mail settings
Submitter Jan Hubicka
Date Jan. 22, 2011, 5:54 p.m.
Message ID <20110122175448.GC8959@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/80011/
State New
Headers show

Comments

Jan Hubicka - Jan. 22, 2011, 5:54 p.m.
Hi, this PR is about fact that we no longer output
"weak declaration must precede definition" errors. These errors
makes no sense in unit-at-a-time compilation model and thus they
didn't exist since we dropped non-unit-at-a-time.

This patch updates docs and turns the error messages into sanity check.
We might document the behaviour change, but I don't think we need
to preserve this limitation of earlier compilers.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	PR c/21659
	* doc/extend.texi (weak pragma): Drop claim that it must
	appear before definition.
	* varasm.c (merge_weak, declare_weak): Only sanity check
	that DECL is not output at a time it is declared weak.
Richard Guenther - Jan. 24, 2011, 12:41 p.m.
On Sat, Jan 22, 2011 at 6:54 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi, this PR is about fact that we no longer output
> "weak declaration must precede definition" errors. These errors
> makes no sense in unit-at-a-time compilation model and thus they
> didn't exist since we dropped non-unit-at-a-time.
>
> This patch updates docs and turns the error messages into sanity check.
> We might document the behaviour change, but I don't think we need
> to preserve this limitation of earlier compilers.
>
> Bootstrapped/regtested x86_64-linux, OK?

Ok.

Thanks,
Richard.

> Honza
>
>        PR c/21659
>        * doc/extend.texi (weak pragma): Drop claim that it must
>        appear before definition.
>        * varasm.c (merge_weak, declare_weak): Only sanity check
>        that DECL is not output at a time it is declared weak.
>
> Index: doc/extend.texi
> ===================================================================
> --- doc/extend.texi     (revision 169127)
> +++ doc/extend.texi     (working copy)
> @@ -13024,8 +13024,7 @@ aliases.
>  @cindex pragma, weak
>  This pragma declares @var{symbol} to be weak, as if the declaration
>  had the attribute of the same name.  The pragma may appear before
> -or after the declaration of @var{symbol}, but must appear before
> -either its first use or its definition.  It is not an error for
> +or after the declaration of @var{symbol}.  It is not an error for
>  @var{symbol} to never be defined at all.
>
>  @item #pragma weak @var{symbol1} = @var{symbol2}
> Index: varasm.c
> ===================================================================
> --- varasm.c    (revision 169127)
> +++ varasm.c    (working copy)
> @@ -5139,20 +5139,16 @@ merge_weak (tree newdecl, tree olddecl)
>       /* NEWDECL is weak, but OLDDECL is not.  */
>
>       /* If we already output the OLDDECL, we're in trouble; we can't
> -        go back and make it weak.  This error cannot be caught in
> -        declare_weak because the NEWDECL and OLDDECL was not yet
> -        been merged; therefore, TREE_ASM_WRITTEN was not set.  */
> -      if (TREE_ASM_WRITTEN (olddecl))
> -       error ("weak declaration of %q+D must precede definition",
> -              newdecl);
> +        go back and make it weak.  This should never happen in
> +        unit-at-a-time compilation.  */
> +      gcc_assert (!TREE_ASM_WRITTEN (olddecl));
>
>       /* If we've already generated rtl referencing OLDDECL, we may
>         have done so in a way that will not function properly with
> -        a weak symbol.  */
> -      else if (TREE_USED (olddecl)
> -              && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (olddecl)))
> -       warning (0, "weak declaration of %q+D after first use results "
> -                 "in unspecified behavior", newdecl);
> +        a weak symbol.  Again in unit-at-a-time this should be
> +        impossible.  */
> +      gcc_assert (!TREE_USED (olddecl)
> +                 || !TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (olddecl)));
>
>       if (TARGET_SUPPORTS_WEAK)
>        {
> @@ -5184,10 +5180,9 @@ merge_weak (tree newdecl, tree olddecl)
>  void
>  declare_weak (tree decl)
>  {
> +  gcc_assert (TREE_CODE (decl) != FUNCTION_DECL || !TREE_ASM_WRITTEN (decl));
>   if (! TREE_PUBLIC (decl))
>     error ("weak declaration of %q+D must be public", decl);
> -  else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_ASM_WRITTEN (decl))
> -    error ("weak declaration of %q+D must precede definition", decl);
>   else if (!TARGET_SUPPORTS_WEAK)
>     warning (0, "weak declaration of %q+D not supported", decl);
>
>

Patch

Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 169127)
+++ doc/extend.texi	(working copy)
@@ -13024,8 +13024,7 @@  aliases.
 @cindex pragma, weak
 This pragma declares @var{symbol} to be weak, as if the declaration
 had the attribute of the same name.  The pragma may appear before
-or after the declaration of @var{symbol}, but must appear before
-either its first use or its definition.  It is not an error for
+or after the declaration of @var{symbol}.  It is not an error for
 @var{symbol} to never be defined at all.
 
 @item #pragma weak @var{symbol1} = @var{symbol2}
Index: varasm.c
===================================================================
--- varasm.c	(revision 169127)
+++ varasm.c	(working copy)
@@ -5139,20 +5139,16 @@  merge_weak (tree newdecl, tree olddecl)
       /* NEWDECL is weak, but OLDDECL is not.  */
 
       /* If we already output the OLDDECL, we're in trouble; we can't
-	 go back and make it weak.  This error cannot be caught in
-	 declare_weak because the NEWDECL and OLDDECL was not yet
-	 been merged; therefore, TREE_ASM_WRITTEN was not set.  */
-      if (TREE_ASM_WRITTEN (olddecl))
-	error ("weak declaration of %q+D must precede definition",
-	       newdecl);
+	 go back and make it weak.  This should never happen in
+	 unit-at-a-time compilation.  */
+      gcc_assert (!TREE_ASM_WRITTEN (olddecl));
 
       /* If we've already generated rtl referencing OLDDECL, we may
 	 have done so in a way that will not function properly with
-	 a weak symbol.  */
-      else if (TREE_USED (olddecl)
-	       && TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (olddecl)))
-	warning (0, "weak declaration of %q+D after first use results "
-                 "in unspecified behavior", newdecl);
+	 a weak symbol.  Again in unit-at-a-time this should be
+	 impossible.  */
+      gcc_assert (!TREE_USED (olddecl)
+	          || !TREE_SYMBOL_REFERENCED (DECL_ASSEMBLER_NAME (olddecl)));
 
       if (TARGET_SUPPORTS_WEAK)
 	{
@@ -5184,10 +5180,9 @@  merge_weak (tree newdecl, tree olddecl)
 void
 declare_weak (tree decl)
 {
+  gcc_assert (TREE_CODE (decl) != FUNCTION_DECL || !TREE_ASM_WRITTEN (decl));
   if (! TREE_PUBLIC (decl))
     error ("weak declaration of %q+D must be public", decl);
-  else if (TREE_CODE (decl) == FUNCTION_DECL && TREE_ASM_WRITTEN (decl))
-    error ("weak declaration of %q+D must precede definition", decl);
   else if (!TARGET_SUPPORTS_WEAK)
     warning (0, "weak declaration of %q+D not supported", decl);