Re: [C++ PATCH] Diagnose static data members in anon ns which are used, but weren't defined (PR c++/34094)

[Jakub Jelinek <jakub at redhat dot com>]

On Tue, Nov 20, 2007 at 04:11:15PM -0500, Jason Merrill wrote:
> Jakub Jelinek wrote:
> >Before the visibility changes for anon namespaces if a static data member
> >wasn't defined (just declared), but used, there would be a link time
> >failure.  But as constrain_visibility (decl, VISIBILITY_ANON) makes
> >now the static data members !TREE_PUBLIC and DECL_NOR_REALLY_EXTERN,
> >even when they are not defined g++ emits them in .bss.
> 
> That seems like the bug.  Why are we emitting a definition of something 
> which hasn't been defined?

Because even just the declared VAR_DECL constrained to VISIBILITY_ANON
is very similar to the middle-end to how a real definition looks like.

just declaration:

 <var_decl 0x2aaaaea21f00 i
    type <integer_type 0x2aaaae93a540 int public SI ...>
    used static external nonlocal decl_3 decl_5 decl_6 SI file m.C line 8 col 16 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4>
    align 32 context <record_type 0x2aaaaea75480 A>
    chain <var_decl 0x2aaaaea80000 j>>

vs. definition (which could even have no DECL_INITIAL):

 <var_decl 0x2aaaaea80000 j
    type <integer_type 0x2aaaae93a540 int public SI ....>
    used static tree_1 tree_2 tree_3 decl_5 SI file m.C line 15 col 10 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4>
    align 32 context <record_type 0x2aaaaea75480 A> initial <integer_cst 0x2aaaae962570 4>
    chain <var_decl 0x2aaaaea800a0 k>>

The important difference is DECL_EXTERNAL, but as it is
DECL_NOT_REALLY_EXTERN, cp_wrap_global_declarations wants to clear that.
DECL_NONLOCAL is a FE private flag in this case.

Instead of issuing the error we could just not set DECL_EXTERNAL for it:

--- gcc/cp/decl2.c.jj        2007-11-06 09:15:15.000000000 +0100
+++ gcc/cp/decl2.c     2007-11-20 22:29:01.000000000 +0100
@@ -3363,8 +3363,15 @@ cp_write_global_declarations (void)
            continue;
          import_export_decl (decl);
          /* If this static data member is needed, provide it to the
-            back end.  */
-         if (DECL_NOT_REALLY_EXTERN (decl) && decl_needed_p (decl))
+            back end.  Don't do that for static data members in
+            anonymous namespace which were just declared, but never
+            defined.  */
+         if (DECL_NOT_REALLY_EXTERN (decl) && decl_needed_p (decl)
+             && !(TREE_CODE (decl) == VAR_DECL
+                   && DECL_NONLOCAL (decl)
+                   && DECL_CLASS_SCOPE_P (decl)
+                   && DECL_INITIAL (decl) == NULL_TREE
+                   && type_visibility (DECL_CONTEXT (decl)) == VISIBILITY_ANON))
            DECL_EXTERNAL (decl) = 0;
        }
       if (VEC_length (tree, pending_statics) != 0

fixes this too (ld will complain).  But it is hard to have a g++.dg testcase
for that, plus I thought when the compiler can detect the bug, it is better
to report it than to leave it for ld.

> >I'm not sure whether the && DECL_INITIAL (decl) == NULL_TREE should
> >be there, given that [class.static.data]/4 says that the definition should
> >be present even for const qualified static data members with initializers
> >in the class, but removing that line breaks a bunch of testcases in the
> >testsuite.
> 
> The problem is that decl_needed_p isn't a strong enough test; I think we 
> set TREE_USED even if we just pull out the constant value of a static 
> data member.  We only need a definition for it if we try to use its address.

Ok.

	Jakub