[C++ PATCH] Fix -frepo (PR c++/34178)

[C++ PATCH] Fix -frepo (PR c++/34178)

[Jakub Jelinek]

Hi!

The testcase below doesn't link with -frepo, the static data member
isn't defined anywhere.
There are IMHO two separate issues - one is it doesn't make much sense
to special case any constant expr initialized static data members, only
const static data members with const expr initialization need to be
present.  The other is an interaction between repo_emit_p and
import_export_decl.  In some cases repo_emit_p decides to force emitting
something, possibly in all compilation units, by returning 2.  But
that 2 is the same value repo_emit_p returns when -fno-repo.  With
-fno-repo -fno-implicit-templates we wouldn't want to instantiate e.g.
the static data member in this case, but when repo_emit_p with -frepo
(which implies -fno-implicit-templates) decided it should be emitted,
not emitting it means it won't be in *.rpo either and therefore won't
be added even during linking.

Following was bootstrapped/regtested on x86_64-linux, ok for trunk?

2007-11-27  Jakub Jelinek  <jakub@redhat.com>

	PR c++/34178
	* repo.c (repo_emit_p): Return 2 for DECL_INTEGRAL_CONSTANT_VAR_P
	in class scope rather than DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P.
	* decl2.c (import_export_decl): Don't make VAR_DECLs import_p when
	flag_use_repository and repo_emit_p returned 2.

	* g++.dg/template/repo6.C: New test.

--- gcc/cp/repo.c.jj	2007-08-27 10:15:32.000000000 +0200
+++ gcc/cp/repo.c	2007-11-27 20:02:57.000000000 +0100
@@ -304,10 +304,10 @@ repo_emit_p (tree decl)
 	  && (!TYPE_LANG_SPECIFIC (type)
 	      || !CLASSTYPE_TEMPLATE_INSTANTIATION (type)))
 	return 2;
-      /* Static data members initialized by constant expressions must
+      /* Const static data members initialized by constant expressions must
 	 be processed where needed so that their definitions are
 	 available.  */
-      if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl)
+      if (DECL_INTEGRAL_CONSTANT_VAR_P (decl)
 	  && DECL_CLASS_SCOPE_P (decl))
 	return 2;
     }
--- gcc/cp/decl2.c.jj	2007-11-26 22:19:54.000000000 +0100
+++ gcc/cp/decl2.c	2007-11-27 20:21:57.000000000 +0100
@@ -2230,7 +2230,8 @@ import_export_decl (tree decl)
     {
       /* DECL is an implicit instantiation of a function or static
 	 data member.  */
-      if (flag_implicit_templates
+      if ((flag_implicit_templates
+	   && !flag_use_repository)
 	  || (flag_implicit_inline_templates
 	      && TREE_CODE (decl) == FUNCTION_DECL
 	      && DECL_DECLARED_INLINE_P (decl)))
--- gcc/testsuite/g++.dg/template/repo6.C.jj	2007-11-27 20:59:36.000000000 +0100
+++ gcc/testsuite/g++.dg/template/repo6.C	2007-11-27 20:59:29.000000000 +0100
@@ -0,0 +1,25 @@
+// PR c++/34178
+// { dg-options "-frepo" }
+// { dg-final { cleanup-repo-files } }
+// { dg-require-host-local "" }
+
+template<typename T>
+class A
+{
+private:
+  static const int x;
+  static int y;
+
+public:
+  int getX () { return x + y; }
+};
+
+template<typename T> const int A<T>::x = 0;
+template<typename T> int A<T>::y = 0;
+
+int
+main ()
+{
+  A<int> a;
+  return a.getX();
+}

	Jakub

[C++ PATCH] Fix -frepo (PR c++/34178, take 2)

[Jakub Jelinek]

On Tue, Nov 27, 2007 at 04:58:31PM -0500, Jakub Jelinek wrote:
> The testcase below doesn't link with -frepo, the static data member
> isn't defined anywhere.
> There are IMHO two separate issues - one is it doesn't make much sense
> to special case any constant expr initialized static data members, only
> const static data members with const expr initialization need to be
> present.  The other is an interaction between repo_emit_p and
> import_export_decl.  In some cases repo_emit_p decides to force emitting
> something, possibly in all compilation units, by returning 2.  But
> that 2 is the same value repo_emit_p returns when -fno-repo.  With
> -fno-repo -fno-implicit-templates we wouldn't want to instantiate e.g.
> the static data member in this case, but when repo_emit_p with -frepo
> (which implies -fno-implicit-templates) decided it should be emitted,
> not emitting it means it won't be in *.rpo either and therefore won't
> be added even during linking.

Turns out I was wrong, even const static data members initialized
outside of the class should be always emitted, as can be seen e.g. on
the new repo7.C testcase I have just posted.  But for non-const static
data members, even when initialized by constant expression we should handle
them normally (i.e. with -frepo just note them in *.rpo and allow them
to be compiled in when linker invokes us again).

Regtested on x86_64-linux, ok for trunk?

2007-12-05  Jakub Jelinek  <jakub@redhat.com>

	PR c++/34178
	* repo.c (repo_emit_p): Return 2 for
	DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P decls only if they are
	const.
	* decl2.c (import_export_decl): Don't make VAR_DECLs import_p when
	flag_use_repository and repo_emit_p returned 2.

	* g++.dg/template/repo6.C: New test.

--- gcc/cp/decl2.c.jj	2007-12-05 13:11:12.000000000 +0100
+++ gcc/cp/decl2.c	2007-12-05 13:37:34.000000000 +0100
@@ -2230,7 +2230,8 @@ import_export_decl (tree decl)
     {
       /* DECL is an implicit instantiation of a function or static
 	 data member.  */
-      if (flag_implicit_templates
+      if ((flag_implicit_templates
+	   && !flag_use_repository)
 	  || (flag_implicit_inline_templates
 	      && TREE_CODE (decl) == FUNCTION_DECL
 	      && DECL_DECLARED_INLINE_P (decl)))
--- gcc/cp/repo.c.jj	2007-12-04 20:33:17.000000000 +0100
+++ gcc/cp/repo.c	2007-12-05 14:40:36.000000000 +0100
@@ -304,11 +304,12 @@ repo_emit_p (tree decl)
 	  && (!TYPE_LANG_SPECIFIC (type)
 	      || !CLASSTYPE_TEMPLATE_INSTANTIATION (type)))
 	return 2;
-      /* Static data members initialized by constant expressions must
+      /* Const static data members initialized by constant expressions must
 	 be processed where needed so that their definitions are
 	 available.  */
       if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl)
-	  && DECL_CLASS_SCOPE_P (decl))
+	  && DECL_CLASS_SCOPE_P (decl)
+	  && CP_TYPE_CONST_P (TREE_TYPE (decl)))
 	return 2;
     }
   else if (!DECL_TEMPLATE_INSTANTIATION (decl))
--- gcc/testsuite/g++.dg/template/repo6.C.jj	2007-12-04 20:44:19.000000000 +0100
+++ gcc/testsuite/g++.dg/template/repo6.C	2007-12-04 20:44:19.000000000 +0100
@@ -0,0 +1,25 @@
+// PR c++/34178
+// { dg-options "-frepo" }
+// { dg-final { cleanup-repo-files } }
+// { dg-require-host-local "" }
+
+template<typename T>
+class A
+{
+private:
+  static const int x;
+  static int y;
+
+public:
+  int getX () { return x + y; }
+};
+
+template<typename T> const int A<T>::x = 0;
+template<typename T> int A<T>::y = 0;
+
+int
+main ()
+{
+  A<int> a;
+  return a.getX();
+}


	Jakub

Re: [C++ PATCH] Fix -frepo (PR c++/34178, take 2)

[Mark Mitchell]

Jakub Jelinek wrote:

> 2007-12-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/34178
> 	* repo.c (repo_emit_p): Return 2 for
> 	DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P decls only if they are
> 	const.

Why don't we want to use DECL_INTEGRAL_CONSTANT_VAR_P here?

> 	* decl2.c (import_export_decl): Don't make VAR_DECLs import_p when
> 	flag_use_repository and repo_emit_p returned 2.

> +      if ((flag_implicit_templates
> +	   && !flag_use_repository)

Shouldn't that condition involve emit_p, rather than just be
!flag_use_repository?

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [C++ PATCH] Fix -frepo (PR c++/34178, take 2)

[Jakub Jelinek]

On Wed, Dec 05, 2007 at 05:03:29PM -0800, Mark Mitchell wrote:
> Jakub Jelinek wrote:
> 
> > 2007-12-05  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR c++/34178
> > 	* repo.c (repo_emit_p): Return 2 for
> > 	DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P decls only if they are
> > 	const.
> 
> Why don't we want to use DECL_INTEGRAL_CONSTANT_VAR_P here?

That's what my first patch version did.  But that breaks e.g. the repo7.C
testcase in http://gcc.gnu.org/ml/gcc-patches/2007-12/msg00182.html
From the
#define DECL_INTEGRAL_CONSTANT_VAR_P(NODE)              \
  (TREE_CODE (NODE) == VAR_DECL                         \
   && CP_TYPE_CONST_NON_VOLATILE_P (TREE_TYPE (NODE))   \
   && INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (NODE)) \
   && DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (NODE))
conditions we already check
TREE_CODE (decl) == VAR_DECL
and
DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl)
in repo.c and with this patch also
CP_TYPE_CONST_P (TREE_TYPE (decl))
But in repo7.C the const static data member is aggregate, not integral or
enum.  finish_static_data_member has
  /* Force the compiler to know when an uninitialized static const
     member is being used.  */
  if (CP_TYPE_CONST_P (TREE_TYPE (decl)) && init == 0)
    TREE_USED (decl) = 1;
Later on duplicate_decls clears DECL_EXTERNAL on this decl (because a
definition was parsed) and after tsubst finish_static_data_member
sets TREE_USED again.  Next in instantiate_decl we call
14742     if (TREE_PUBLIC (d) && !DECL_REALLY_EXTERN (d) && !repo_emit_p (d))
on this, and with DECL_INTEGRAL_CONSTANT_VAR_P test in repo_emit_p that
returns 0, which means the rest of instantiate_decl is bypassed for this
decl.  Then this makes into cgraph, which emits it, as the decl
is !DECL_EXTERNAL, TREE_USED etc. (and even emits it without the
initialized as common symbol, as DECL_INITIAL is NULL).

> > 	* decl2.c (import_export_decl): Don't make VAR_DECLs import_p when
> > 	flag_use_repository and repo_emit_p returned 2.
> 
> > +      if ((flag_implicit_templates
> > +	   && !flag_use_repository)
> 
> Shouldn't that condition involve emit_p, rather than just be
> !flag_use_repository?

That's not needed, we know that emit_p is 2 at this point:

  emit_p = repo_emit_p (decl);
  if (emit_p == 0)
    import_p = true;
  else if (emit_p == 1)
    {
...
      return;
    }

  if (import_p)
    ;
  else if (...)
    {
...
    }
  else if (DECL_TEMPLATE_INSTANTIATION (decl)
           || DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (decl))
    {
// The && !flag_use_repository addition is here.
    }

emit_p can be only 0, 1, 2, if it is 0, then import_p was true
and so we don't hit this point, if it is 1, we returned early.

	Jakub

Re: [C++ PATCH] Fix -frepo (PR c++/34178, take 2)

[Mark Mitchell]

Jakub Jelinek wrote:

> But in repo7.C the const static data member is aggregate, not integral or
> enum.  finish_static_data_member has
>   /* Force the compiler to know when an uninitialized static const
>      member is being used.  */
>   if (CP_TYPE_CONST_P (TREE_TYPE (decl)) && init == 0)
>     TREE_USED (decl) = 1;
> Later on duplicate_decls clears DECL_EXTERNAL on this decl (because a
> definition was parsed) and after tsubst finish_static_data_member
> sets TREE_USED again.  Next in instantiate_decl we call
> 14742     if (TREE_PUBLIC (d) && !DECL_REALLY_EXTERN (d) && !repo_emit_p (d))
> on this, and with DECL_INTEGRAL_CONSTANT_VAR_P test in repo_emit_p that
> returns 0, which means the rest of instantiate_decl is bypassed for this
> decl.  Then this makes into cgraph, which emits it, as the decl
> is !DECL_EXTERNAL, TREE_USED etc. (and even emits it without the
> initialized as common symbol, as DECL_INITIAL is NULL).

I'm confused.  Are we talking about the second time we process the file,
at link-time?

And, at that point, does the .rpo file tells the compiler that it needs
to define "A D<B>::b"?  If so, repo_emit_p should be returning true anyhow.

Otherwise, I think something is just going wrong with explicit template
instantiation.  Once we decide that D<B>::b is going to be emitted in
the front end (i.e., clear DECL_EXTERNAL), we have to process its
initializer.  But, the fact that it's const doesn't seem relevant to me
at all.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

[C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3)

[Jakub Jelinek]

On Thu, Dec 06, 2007 at 08:27:55PM -0800, Mark Mitchell wrote:
> > But in repo7.C the const static data member is aggregate, not integral or
> > enum.  finish_static_data_member has
> >   /* Force the compiler to know when an uninitialized static const
> >      member is being used.  */
> >   if (CP_TYPE_CONST_P (TREE_TYPE (decl)) && init == 0)
> >     TREE_USED (decl) = 1;
> > Later on duplicate_decls clears DECL_EXTERNAL on this decl (because a
> > definition was parsed) and after tsubst finish_static_data_member
> > sets TREE_USED again.  Next in instantiate_decl we call
> > 14742     if (TREE_PUBLIC (d) && !DECL_REALLY_EXTERN (d) && !repo_emit_p (d))
> > on this, and with DECL_INTEGRAL_CONSTANT_VAR_P test in repo_emit_p that
> > returns 0, which means the rest of instantiate_decl is bypassed for this
> > decl.  Then this makes into cgraph, which emits it, as the decl
> > is !DECL_EXTERNAL, TREE_USED etc. (and even emits it without the
> > initialized as common symbol, as DECL_INITIAL is NULL).
> 
> I'm confused.  Are we talking about the second time we process the file,
> at link-time?

No, I'm talking about the original compilation, which creates the .rpo file.

Perhaps even shorter testcase with ./cc1plus -frepo testcase.C is:
struct A { int a; };
template <typename T> struct D { static const A b; };
template<typename T> const A D<T>::b = { 2 };
template class D<A>;
int main () { }

Here there is just one static data member.
finish_static_data_member_decl marks D<T>::b as TREE_USED because it is
const.  duplicate_decls is called on:
olddecl
 <var_decl 0x2aaaaea7f000 b
    type <record_type 0x2aaaaea75b40 A readonly type_1 type_5 type_6 SI
        size <integer_cst 0x2aaaae927a50 constant invariant 32>
        unit size <integer_cst 0x2aaaae9276c0 constant invariant 4>
        align 32 symtab 0 alias set -1 canonical type 0x2aaaaea75b40
        fields <field_decl 0x2aaaaea21f00 a type <integer_type 0x2aaaae93a540 int>
            nonlocal decl_3 SI file repo8.C line 8 col 7 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4>
            align 32 offset_align 128
            offset <integer_cst 0x2aaaae9276f0 constant invariant 0>
            bit offset <integer_cst 0x2aaaae948330 constant invariant 0> context <record_type 0x2aaaaea753c0 A> chain <type_decl 0x2aaaaea75540 A>>
        X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown>
    used public static external decl_3 decl_6 SI file repo8.C line 13 col 18 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4>
    align 32 context <record_type 0x2aaaaea75780 D>
    template-info 0x2aaaaea6bde0 chain <type_decl 0x2aaaaea759c0 D>>
newdecl
 <var_decl 0x2aaaaea7f0a0 b
    type <record_type 0x2aaaaea75b40 A readonly type_1 type_5 type_6 SI
        size <integer_cst 0x2aaaae927a50 constant invariant 32>
        unit size <integer_cst 0x2aaaae9276c0 constant invariant 4>
        align 32 symtab 0 alias set -1 canonical type 0x2aaaaea75b40
        fields <field_decl 0x2aaaaea21f00 a type <integer_type 0x2aaaae93a540 int>
            nonlocal decl_3 SI file repo8.C line 8 col 7 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4>
            align 32 offset_align 128
            offset <integer_cst 0x2aaaae9276f0 constant invariant 0>
            bit offset <integer_cst 0x2aaaae948330 constant invariant 0> context <record_type 0x2aaaaea753c0 A> chain <type_decl 0x2aaaaea75540 A>>
        X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown>
    public static SI file repo8.C line 16 col 36 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4>
    align 32 context <record_type 0x2aaaaea75780 D>
   >
so it still has DECL_AGGR_P set, but is no longer external.  start_decl
later clears DECL_IN_AGGR_P on this.  Then tsubst_decl creates a new
var_decl:
 <var_decl 0x2aaaaea7f1e0 b
    type <record_type 0x2aaaaea75b40 A readonly type_1 type_5 type_6 SI
        size <integer_cst 0x2aaaae927a50 constant invariant 32>
        unit size <integer_cst 0x2aaaae9276c0 constant invariant 4>
        align 32 symtab 0 alias set -1 canonical type 0x2aaaaea75b40
        fields <field_decl 0x2aaaaea21f00 a type <integer_type 0x2aaaae93a540 int>
            nonlocal decl_3 SI file repo8.C line 8 col 7 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4>
            align 32 offset_align 128
            offset <integer_cst 0x2aaaae9276f0 constant invariant 0>
            bit offset <integer_cst 0x2aaaae948330 constant invariant 0> context <record_type 0x2aaaaea753c0 A> chain <type_decl 0x2aaaaea75540 A>>
        X() X(constX&) this=(X&) n_parents=0 use_template=0 interface-unknown>
    readonly used public static tree_2 external SI file repo8.C line 16 col 36 size <integer_cst 0x2aaaae927a50 32> unit size <integer_cst 0x2aaaae9276c0 4>
    align 32 context <record_type 0x2aaaaea75e40 D>
    template-info 0x2aaaaea803c0>
and finish_static_data_member_decl on it 
737       if (CP_TYPE_CONST_P (TREE_TYPE (decl)) && init == 0)
738         TREE_USED (decl) = 1;
739       DECL_INITIAL (decl) = init;
740       DECL_IN_AGGR_P (decl) = 1;
instantiate_class_member -> mark_decl_instantiated marks this as
instantiated and finally instantiate_class_member -> instantiate_decl
calls that
14742     if (TREE_PUBLIC (d) && !DECL_REALLY_EXTERN (d) && !repo_emit_p (d))

If repo_emit_p doesn't return 0 in this case, then instantiate_decl
would later do:
      /* Clear out DECL_RTL; whatever was there before may not be right
         since we've reset the type of the declaration.  */
      SET_DECL_RTL (d, NULL_RTX);
      DECL_IN_AGGR_P (d) = 0;

      /* The initializer is placed in DECL_INITIAL by
         regenerate_decl_from_template.  Pull it out so that
         finish_decl can process it.  */
      init = DECL_INITIAL (d);
      DECL_INITIAL (d) = NULL_TREE;
      DECL_INITIALIZED_P (d) = 0;

      /* Clear DECL_EXTERNAL so that cp_finish_decl will process the
         initializer.  That function will defer actual emission until
         we have a chance to determine linkage.  */
      DECL_EXTERNAL (d) = 0;

      /* Enter the scope of D so that access-checking works correctly.  */
      push_nested_class (DECL_CONTEXT (d));
      finish_decl (d, init, NULL_TREE);
      pop_nested_class ();
But as repo_emit_p above returned 0, it will just go out, but 0x2aaaaea7f1e0
is TREE_USED, !DECL_EXTERNAL so it will be emitted, even without
initializer (because regenerate_decl_from_template which sets DECL_INITIAL
has not been called either).  If I explicitly make d DECL_EXTERNAL again
when repo_emit_p returns 0, this testcase and even one with additional
const A *x = &D<B>::b;
compiles, doesn't contain _ZN1DI1BE1bE definition (only references it in
x var):
rm -f repo7{.o,.s,.rpo}; ./g++ -B ./ -frepo -c repo7.C; nm repo7.o; ./g++ -B ./ -frepo repo7.o -o repo7 -L ../x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs/; cat repo7.rpo; nm repo7 | grep ' _Z'
0000000000000000 R _ZN1B1bE
                 U _ZN1DI1BE1bE
                 U __gxx_personality_v0
0000000000000000 T main
0000000000000000 D x
collect: recompiling repo7.C
collect: relinking
M repo7.C
D /usr/src/gcc/obj/gcc
A '-B' './' '-frepo' '-c' '-shared-libgcc' '-mtune=generic' '-frandom-seed=0xb3d94816' '-shared-libgcc'
C _ZN1DI1BE1bE
000000000040063c R _ZN1B1bE
0000000000400640 V _ZN1DI1BE1bE

which looks ok.  Making it DECL_EXTERNAL and !DECL_NOT_REALLY_EXTERN matches
what import_export_decl (the only other caller of repo_emit_p) is already
doing if repo_emit_p returns 0 - sets import_p and later:
  if (import_p)
    {
      /* If we are importing DECL into this translation unit, mark is
         an undefined here.  */
      DECL_EXTERNAL (decl) = 1;
      DECL_NOT_REALLY_EXTERN (decl) = 0;
    }

I have regtested following patch on x86_64-linux, ok for trunk?

2007-12-07  Jakub Jelinek  <jakub@redhat.com>

	PR c++/34178
	PR c++/34340
	* repo.c (repo_emit_p): Return 2 for DECL_INTEGRAL_CONSTANT_VAR_P
	in class scope rather than DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P.
	* decl2.c (import_export_decl): Don't make VAR_DECLs import_p when
	flag_use_repository and repo_emit_p returned 2.
	* pt.c (instantiate_decl): If repo_emit_p returned 0, mark d as
	DECL_EXTERNAL and !DECL_NOT_REALLY_EXTERN.

	* g++.dg/template/repo6.C: New test.
	* g++.dg/template/repo7.C: New test.

--- gcc/cp/pt.c.jj	2007-12-05 21:42:08.000000000 +0100
+++ gcc/cp/pt.c	2007-12-07 11:11:22.000000000 +0100
@@ -14753,7 +14753,14 @@ instantiate_decl (tree d, int defer_ok,
       if (!(TREE_CODE (d) == FUNCTION_DECL
 	    && flag_inline_trees
 	    && DECL_DECLARED_INLINE_P (d)))
-	goto out;
+	{
+	  /* If we are importing DECL into this translation unit, mark it
+	     as undefined here.  */
+	  DECL_EXTERNAL (d) = 1;
+	  DECL_NOT_REALLY_EXTERN (d) = 0;
+	  DECL_INTERFACE_KNOWN (d) = 1;
+	  goto out;
+	}
     }
 
   need_push = !cfun || !global_bindings_p ();
--- gcc/cp/decl2.c.jj	2007-12-06 12:00:12.000000000 +0100
+++ gcc/cp/decl2.c	2007-12-06 12:00:19.000000000 +0100
@@ -2230,7 +2230,8 @@ import_export_decl (tree decl)
     {
       /* DECL is an implicit instantiation of a function or static
 	 data member.  */
-      if (flag_implicit_templates
+      if ((flag_implicit_templates
+	   && !flag_use_repository)
 	  || (flag_implicit_inline_templates
 	      && TREE_CODE (decl) == FUNCTION_DECL
 	      && DECL_DECLARED_INLINE_P (decl)))
--- gcc/cp/repo.c.jj	2007-12-06 11:53:40.000000000 +0100
+++ gcc/cp/repo.c	2007-12-07 09:35:46.000000000 +0100
@@ -304,10 +304,10 @@ repo_emit_p (tree decl)
 	  && (!TYPE_LANG_SPECIFIC (type)
 	      || !CLASSTYPE_TEMPLATE_INSTANTIATION (type)))
 	return 2;
-      /* Static data members initialized by constant expressions must
+      /* Const static data members initialized by constant expressions must
 	 be processed where needed so that their definitions are
 	 available.  */
-      if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl)
+      if (DECL_INTEGRAL_CONSTANT_VAR_P (decl)
 	  && DECL_CLASS_SCOPE_P (decl))
 	return 2;
     }
--- gcc/testsuite/g++.dg/template/repo6.C.jj	2007-12-06 12:00:19.000000000 +0100
+++ gcc/testsuite/g++.dg/template/repo6.C	2007-12-06 12:00:19.000000000 +0100
@@ -0,0 +1,25 @@
+// PR c++/34178
+// { dg-options "-frepo" }
+// { dg-final { cleanup-repo-files } }
+// { dg-require-host-local "" }
+
+template<typename T>
+class A
+{
+private:
+  static const int x;
+  static int y;
+
+public:
+  int getX () { return x + y; }
+};
+
+template<typename T> const int A<T>::x = 0;
+template<typename T> int A<T>::y = 0;
+
+int
+main ()
+{
+  A<int> a;
+  return a.getX();
+}
--- gcc/testsuite/g++.dg/template/repo7.C.jj	2007-12-06 12:00:12.000000000 +0100
+++ gcc/testsuite/g++.dg/template/repo7.C	2007-12-07 11:07:53.000000000 +0100
@@ -0,0 +1,24 @@
+// PR c++/34340
+// { dg-options "-frepo" }
+// { dg-final { cleanup-repo-files } }
+// { dg-require-host-local "" }
+
+struct A
+{
+  int a;
+};
+
+template <typename T> struct D
+{
+  static const A b;
+};
+
+template<typename T> const A D<T>::b = { 2 };
+template class D<A>;
+
+const A *x = &D<A>::b;
+
+int
+main ()
+{
+}


	Jakub

Re: [C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3)

[Mark Mitchell]

Jakub Jelinek wrote:

> No, I'm talking about the original compilation, which creates the .rpo file.
> 
> Perhaps even shorter testcase with ./cc1plus -frepo testcase.C is:
> struct A { int a; };
> template <typename T> struct D { static const A b; };
> template<typename T> const A D<T>::b = { 2 };
> template class D<A>;
> int main () { }
> 
> Here there is just one static data member.
> finish_static_data_member_decl marks D<T>::b as TREE_USED because it is
> const. 

Do you really mean D<T> or D<A>?  TREE_USED on D<T>::b should not matter
as that's not a real thing; only D<A>::b might show up in the object file.

Anyhow, the explicit instantiation directive should result in us
emitting D<A>::b.  I don't think that -frepo should cause us not to emit
things that the user has explicitly instantiated.

I would expect that the right code in repo_emit_p would be:

  /* Variables that can appear in integral constant-expressions must
     be instantiated so that the values of th variables are
     available.  */
  if (DECL_INTEGRAL_CONSTANT_VAR_P (decl))
    return 2;
  /* Explicit instantiations should be handled normally even when using
     a repository.  */
  if (DECL_EXPLICIT_INSTANTIATION (decl))
    return 2;

Does that work?

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3)

[Jakub Jelinek]

On Fri, Dec 07, 2007 at 09:55:44AM -0800, Mark Mitchell wrote:
> Jakub Jelinek wrote:
> 
> > No, I'm talking about the original compilation, which creates the .rpo file.
> > 
> > Perhaps even shorter testcase with ./cc1plus -frepo testcase.C is:
> > struct A { int a; };
> > template <typename T> struct D { static const A b; };
> > template<typename T> const A D<T>::b = { 2 };
> > template class D<A>;
> > int main () { }
> > 
> > Here there is just one static data member.
> > finish_static_data_member_decl marks D<T>::b as TREE_USED because it is
> > const. 
> 
> Do you really mean D<T> or D<A>?  TREE_USED on D<T>::b should not matter
> as that's not a real thing; only D<A>::b might show up in the object file.

I meant of course D<A>, sorry.

> Anyhow, the explicit instantiation directive should result in us
> emitting D<A>::b.  I don't think that -frepo should cause us not to emit
> things that the user has explicitly instantiated.
> 
> I would expect that the right code in repo_emit_p would be:
> 
>   /* Variables that can appear in integral constant-expressions must
>      be instantiated so that the values of th variables are
>      available.  */
>   if (DECL_INTEGRAL_CONSTANT_VAR_P (decl))
>     return 2;
>   /* Explicit instantiations should be handled normally even when using
>      a repository.  */
>   if (DECL_EXPLICIT_INSTANTIATION (decl))
>     return 2;
> 
> Does that work?

That seems to work too, for make check-g++ at least (but there are just
very few -frepo testcases, which is probably why this could be broken for so
long).

2007-12-07  Jakub Jelinek  <jakub@redhat.com>

	PR c++/34178
	PR c++/34340
	* repo.c (repo_emit_p): Return 2 for DECL_INTEGRAL_CONSTANT_VAR_P
	in class scope rather than DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P.
	Return 2 also if DECL_EXPLICIT_INSTANTIATION.
	* decl2.c (import_export_decl): Don't make VAR_DECLs import_p when
	flag_use_repository and repo_emit_p returned 2.

	* g++.dg/template/repo6.C: New test.
	* g++.dg/template/repo7.C: New test.
	* g++.dg/template/repo8.C: New test.

--- gcc/cp/decl2.c.jj	2007-12-07 12:21:03.000000000 +0100
+++ gcc/cp/decl2.c	2007-12-07 22:24:35.000000000 +0100
@@ -2230,7 +2230,8 @@ import_export_decl (tree decl)
     {
       /* DECL is an implicit instantiation of a function or static
 	 data member.  */
-      if (flag_implicit_templates
+      if ((flag_implicit_templates
+	   && !flag_use_repository)
 	  || (flag_implicit_inline_templates
 	      && TREE_CODE (decl) == FUNCTION_DECL
 	      && DECL_DECLARED_INLINE_P (decl)))
--- gcc/cp/repo.c.jj	2007-12-07 12:21:03.000000000 +0100
+++ gcc/cp/repo.c	2007-12-07 22:26:43.000000000 +0100
@@ -304,16 +304,19 @@ repo_emit_p (tree decl)
 	  && (!TYPE_LANG_SPECIFIC (type)
 	      || !CLASSTYPE_TEMPLATE_INSTANTIATION (type)))
 	return 2;
-      /* Static data members initialized by constant expressions must
+      /* Const static data members initialized by constant expressions must
 	 be processed where needed so that their definitions are
 	 available.  */
-      if (DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl)
+      if (DECL_INTEGRAL_CONSTANT_VAR_P (decl)
 	  && DECL_CLASS_SCOPE_P (decl))
 	return 2;
     }
   else if (!DECL_TEMPLATE_INSTANTIATION (decl))
     return 2;
 
+  if (DECL_EXPLICIT_INSTANTIATION (decl))
+    return 2;
+
   /* For constructors and destructors, the repository contains
      information about the clones -- not the original function --
      because only the clones are emitted in the object file.  */
--- gcc/testsuite/g++.dg/template/repo6.C.jj	2007-12-07 22:24:35.000000000 +0100
+++ gcc/testsuite/g++.dg/template/repo6.C	2007-12-07 22:24:35.000000000 +0100
@@ -0,0 +1,25 @@
+// PR c++/34178
+// { dg-options "-frepo" }
+// { dg-final { cleanup-repo-files } }
+// { dg-require-host-local "" }
+
+template<typename T>
+class A
+{
+private:
+  static const int x;
+  static int y;
+
+public:
+  int getX () { return x + y; }
+};
+
+template<typename T> const int A<T>::x = 0;
+template<typename T> int A<T>::y = 0;
+
+int
+main ()
+{
+  A<int> a;
+  return a.getX();
+}
--- gcc/testsuite/g++.dg/template/repo7.C.jj	2007-12-07 22:24:35.000000000 +0100
+++ gcc/testsuite/g++.dg/template/repo7.C	2007-12-07 22:24:35.000000000 +0100
@@ -0,0 +1,24 @@
+// PR c++/34340
+// { dg-options "-frepo" }
+// { dg-final { cleanup-repo-files } }
+// { dg-require-host-local "" }
+
+struct A
+{
+  int a;
+};
+
+template <typename T> struct D
+{
+  static const A b;
+};
+
+template<typename T> const A D<T>::b = { 2 };
+template class D<A>;
+
+const A *x = &D<A>::b;
+
+int
+main ()
+{
+}
--- gcc/testsuite/g++.dg/template/repo8.C.jj	2007-12-07 22:24:35.000000000 +0100
+++ gcc/testsuite/g++.dg/template/repo8.C	2007-12-07 22:24:35.000000000 +0100
@@ -0,0 +1,23 @@
+// PR c++/34340
+// { dg-options "-frepo" }
+// { dg-final { cleanup-repo-files } }
+// { dg-require-host-local "" }
+
+struct A
+{
+  int a;
+};
+
+template <typename T> struct D
+{
+  static const A b;
+};
+
+template<typename T> const A D<T>::b = { 2 };
+
+const A *x = &D<A>::b;
+
+int
+main ()
+{
+}


	Jakub

Re: [C++ PATCH] Fix -frepo (PR c++/34178, c++/34340, take 3)

[Mark Mitchell]

Jakub Jelinek wrote:

> That seems to work too, for make check-g++ at least (but there are just
> very few -frepo testcases, which is probably why this could be broken for so
> long).

> 	PR c++/34178
> 	PR c++/34340
> 	* repo.c (repo_emit_p): Return 2 for DECL_INTEGRAL_CONSTANT_VAR_P
> 	in class scope rather than DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P.
> 	Return 2 also if DECL_EXPLICIT_INSTANTIATION.
> 	* decl2.c (import_export_decl): Don't make VAR_DECLs import_p when
> 	flag_use_repository and repo_emit_p returned 2.
> 
> 	* g++.dg/template/repo6.C: New test.
> 	* g++.dg/template/repo7.C: New test.
> 	* g++.dg/template/repo8.C: New test.

If Jason doesn't object within 24 hours, please go ahead and put this
in.  This seems sensible (at least to me!) so I think it's worth a try.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713