Bug 25915 - use ODR rules to make C++ objects not be TREE_PUBLIC
Summary: use ODR rules to make C++ objects not be TREE_PUBLIC
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: unknown
: P3 enhancement
Target Milestone: 4.2.0
Assignee: Jason Merrill
URL:
Keywords: missed-optimization
: 31462 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-01-22 23:56 UTC by Chris Lattner
Modified: 2007-04-03 20:03 UTC (History)
14 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-04-05 22:49:40


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Lattner 2006-01-22 23:56:42 UTC
Consider:

---
template <typename foo>
void bar() {}
namespace { class C {};}
void (*FP)() = bar<C>;
---

Because 'bar' is instantiated with 'C', a class in an anonymous namespace, this instantiation can never be shared across other translation units.  As such, it should be marked static, to improve static and dynamic link times and improve future IPO potential.

On PPC Darwin, the instantion of bar turns into:

.section __TEXT,__textcoal_nt,coalesced,no_toc
        .align 2
.weak_definition __Z3barIN21_GLOBAL__N_t.cczMkGAb1CEEvv
.private_extern __Z3barIN21_GLOBAL__N_t.cczMkGAb1CEEvv
.section __TEXT,__textcoal_nt,coalesced,no_toc
        .align 2
__Z3barIN21_GLOBAL__N_t.cczMkGAb1CEEvv:
        blr


-Chris
Comment 1 Andrew Pinski 2006-01-23 14:43:31 UTC
Confirmed, related to PR 10591.
Comment 2 Geoff Keating 2006-01-23 20:43:29 UTC
Let's make this more general.

Any entity which could be defined more than once (like a class or an inline function) but whose token stream refers to a function or variable which is not TREE_PUBLIC, actually can't be defined more than once, and so every part of such entity can be made not-TREE_PUBLIC.

Exception: if the object referred to is 'const', is of scalar type, is initialized with a constant expression, and the value but not the address of the object is used, it doesn't count.  For additional details, including additional cases where this applies and an explanation of how it applies to templates, see [basic.def.odr] paragraph 5.

For example

static int x;

struct myclass {
  int foo() { return x; }
  int bar() { return 1; }
};

in this example, myclass::foo, myclass::bar, and the typeinfo for myclass, can all be made non-TREE_PUBLIC, because of the reference to 'x' in the definition of myclass.  What's more, if someone says later

struct my_other_class : myclass {
  ...
}

then my_other_class can also be made non-TREE_PUBLIC.  If 'x' was in an anonymous namespace the logic would be the same.
Comment 3 Chris Lattner 2006-01-23 21:23:58 UTC
Absolutely, it would be great to handle that as well.  The risk of making a particular bugzilla PR more general is that it reduces the chance that it will ever get fixed though.

-Chris
Comment 4 gdr@cs.tamu.edu 2006-01-23 21:58:14 UTC
Subject: Re:  instantiated templates with anonymous namespace class as arguments should be static

"geoffk at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| Let's make this more general.

Geoff and me had discussed this on the GCC mailing list in the past.

Just to clarify and make sure we are all at the same page., the
notion of "static" shall not be that of the C or C++ source language
rule, but that of the linker.  
This PR subject should be changed to reflect that.

-- Gaby
Comment 5 Chris Lattner 2006-01-23 22:00:22 UTC
Subject: Re:  use ODR rules to make C++ objects not be
 TREE_PUBLIC


Already done.  The subject is now "use ODR rules to make C++ objects not 
be TREE_PUBLIC"

Thanks,

-Chris

On Mon, 23 Jan 2006, gdr at cs dot tamu dot edu wrote:

>
>
> ------- Comment #4 from gdr at cs dot tamu dot edu  2006-01-23 21:58 -------
> Subject: Re:  instantiated templates with anonymous namespace class as
> arguments should be static
>
> "geoffk at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:
>
> | Let's make this more general.
>
> Geoff and me had discussed this on the GCC mailing list in the past.
>
> Just to clarify and make sure we are all at the same page., the
> notion of "static" shall not be that of the C or C++ source language
> rule, but that of the linker.
> This PR subject should be changed to reflect that.
>
> -- Gaby
>
>
>

-Chris

Comment 6 Geoff Keating 2006-03-09 00:35:31 UTC

*** This bug has been marked as a duplicate of 10591 ***
Comment 7 Jason Merrill 2006-04-05 22:27:53 UTC
I've changed the summary for 10591 such that this is no longer a duplicate.
Comment 8 Andrew Pinski 2006-06-21 18:44:32 UTC
(In reply to comment #2)
> then my_other_class can also be made non-TREE_PUBLIC.  If 'x' was in an
> anonymous namespace the logic would be the same.

Actually it cannot because I define in a different TU my_other_class and now I get an ODR violation.  Likewise for myclass.  Yes this is all undefined but I rather have it be diagnose than having it be undefined behavior.
Comment 9 Gabriel Dos Reis 2006-06-21 19:43:49 UTC
Subject: Re:  use ODR rules to make C++ objects not be TREE_PUBLIC

"pinskia at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:

| Yes this is all undefined but I rather have it be diagnose than
| having it be undefined behavior. 

interesting shift of perspective :-)

-- Gaby
Comment 10 Geoff Keating 2006-06-21 20:44:09 UTC
Hi Andrew,
I'm not sure what diagnostic you're talking about.  If you violate the ODR and define myclass in a different TU, with a different definition for myclass::foo, you will typically get no diagnostic; just inconsistent behaviour.
Comment 11 Jason Merrill 2006-06-30 01:16:31 UTC
Subject: Bug 25915

Author: jason
Date: Fri Jun 30 01:15:56 2006
New Revision: 115086

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115086
Log:
        PR c++/26905
        PR c++/26612
        PR c++/27000
        PR c++/26984
        PR c++/19134
        * tree.c (build_decl_stat): Don't hande #pragma visibility here.
        * c-common.c (c_determine_visibility): Handle it here.
        * c-decl.c (finish_decl): Call c_determine_visibility for
        functions, too.
        * flags.h (enum symbol_visibility): Sort from most to least visibility.
        * tree.h: Likewise.
        * varasm.c (default_assemble_visibility): Likewise.
        * c-common.c (handle_visibility_attribute): Complain about trying
        to give visibility to an already defined class, or trying to change
        declared visibility. Always attach the attribute.
        * cp/decl2.c (determine_visibility): Overhaul.
        (determine_visibility_from_class): Likewise.
        (min_vis_r, type_visibility, constrain_visibility): New fns.
        (constrain_visibility_for_template): Likewise.
        (constrain_class_visibility): Likewise.
        * cp/decl.c (cp_finish_decl): Call determine_visibility for function
        decls, too.
        * cp/name-lookup.c (pushtag): Call determine_visibility.
        * cp/decl.c (duplicate_decls): Don't copy visibility from template to
        specialization.
        * cp/pt.c (check_explicit_specialization): Likewise.
        (lookup_template_class, tsubst_decl): Call determine_visibility.
        * cp/class.c (finish_struct_1): Call constrain_class_visibility.

        PR c++/26905
        PR c++/21675
        PR c++/17470
        * cp/parser.c (cp_parser_explicit_instantiation): Pass the attributes
        to grokdeclarator.
        (cp_parser_type_specifier): Allow 'enum __attribute ((...)) E'.
        (cp_parser_enum_specifier): Likewise.
        (cp_parser_elaborated_type_specifier): Apply attributes if this
        declares only the class.
        (cp_parser_class_specifier): Apply leading attributes immediately.
        * cp/semantics.c (begin_class_definition): Add attributes parameter,
        apply them to the type.
        * attribs.c (decl_attributes): Ignore type-in-place attributes
        once the type has been defined.

        PR c++/21581
        PR c++/25915
        * cp/tree.c (decl_anon_ns_mem_p): New function.
        * cp/cp-tree.h: Declare it.
        * cp/decl2.c (determine_visibility): Make anonymous namespace
        members static.
        (min_vis_r, constrain_visibility): Likewise.
        * cp/rtti.c (create_pseudo_type_info): Set TREE_PUBLIC on
        pseudo-types.
        * cp/decl.c (cxx_init_decl_processing): Set TREE_PUBLIC on
        global_namespace.
        * cp/name-lookup.c (push_namespace_with_attribs): Don't set TREE_PUBLIC
        on anonymous namespaces.

Added:
    trunk/gcc/testsuite/g++.dg/ext/visibility/anon2.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/class1.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/prop1.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/redecl1.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/template1.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/template2.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/template3.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/template4.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/typeinfo1.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/warn1.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/warn2.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/warn3.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/warn4.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/attribs.c
    trunk/gcc/c-common.c
    trunk/gcc/c-decl.c
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/class.c
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/decl.c
    trunk/gcc/cp/decl2.c
    trunk/gcc/cp/name-lookup.c
    trunk/gcc/cp/parser.c
    trunk/gcc/cp/pt.c
    trunk/gcc/cp/rtti.c
    trunk/gcc/cp/semantics.c
    trunk/gcc/cp/tree.c
    trunk/gcc/doc/extend.texi
    trunk/gcc/doc/invoke.texi
    trunk/gcc/flags.h
    trunk/gcc/testsuite/g++.dg/ext/attrib14.C
    trunk/gcc/testsuite/g++.dg/ext/attrib9.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/anon1.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/assign1.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/fvisibility-override2.C
    trunk/gcc/testsuite/g++.dg/ext/visibility/virtual.C
    trunk/gcc/testsuite/g++.old-deja/g++.pt/enum5.C
    trunk/gcc/tree.c
    trunk/gcc/tree.h
    trunk/gcc/varasm.c

Comment 12 Andrew Pinski 2006-06-30 15:27:28 UTC
Fixed.
Comment 13 Jakub Jelinek 2006-08-28 12:27:17 UTC
Subject: Bug 25915

Author: jakub
Date: Mon Aug 28 12:26:41 2006
New Revision: 116505

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116505
Log:
2006-07-06  Jason Merrill  <jason@redhat.com>

cp/
	PR c++/28279
	* decl2.c (finish_static_data_member_decl): Don't assert
	TREE_PUBLIC.

2006-07-01  Jason Merrill  <jason@redhat.com>

cp/
	PR c++/28215
	* method.c (make_thunk): Unset DECL_USE_TEMPLATE and 
	DECL_TEMPLATE_INFO.

2006-06-30  Jason Merrill  <jason@redhat.com>

objcp/
	* objcp-decl.c (objcp_start_struct): Pass null attributes argument
	to begin_class_definition.

2006-06-29  Jason Merrill  <jason@redhat.com>

	PR c++/26905
	PR c++/26612
	PR c++/27000
	PR c++/26984
	PR c++/19134
	* tree.c (build_decl_stat): Don't hande #pragma visibility here.
	* c-common.c (c_determine_visibility): Handle it here.
	* c-decl.c (finish_decl): Call c_determine_visibility for 
	functions, too.
	* flags.h (enum symbol_visibility): Sort from most to least visibility.
	* tree.h: Likewise.
	* varasm.c (default_assemble_visibility): Likewise.
	* c-common.c (handle_visibility_attribute): Complain about trying
	to give visibility to an already defined class, or trying to change
	declared visibility. Always attach the attribute.

	PR c++/26905
	PR c++/21675
	PR c++/17470
	* attribs.c (decl_attributes): Ignore type-in-place attributes
	once the type has been defined.
cp/
	PR c++/26905
	PR c++/26612
	PR c++/27000
	PR c++/26984
	PR c++/19134
	* decl2.c (determine_visibility): Overhaul.
	(determine_visibility_from_class): Likewise.
	(min_vis_r, type_visibility, constrain_visibility): New fns.
	(constrain_visibility_for_template): Likewise.
	(constrain_class_visibility): Likewise.
	* decl.c (cp_finish_decl): Call determine_visibility for function
	decls, too.
	* name-lookup.c (pushtag): Call determine_visibility.
	* decl.c (duplicate_decls): Don't copy visibility from template to
	specialization.
	* pt.c (check_explicit_specialization): Likewise.
	(lookup_template_class, tsubst_decl): Call determine_visibility.
	* class.c (finish_struct_1): Call constrain_class_visibility.

	PR c++/26905
	PR c++/21675
	PR c++/17470
	* parser.c (cp_parser_explicit_instantiation): Pass the attributes
	to grokdeclarator.
	(cp_parser_type_specifier): Allow 'enum __attribute ((...)) E'.
	(cp_parser_enum_specifier): Likewise.
	(cp_parser_elaborated_type_specifier): Apply attributes if this
	declares only the class.
	(cp_parser_class_specifier): Apply leading attributes immediately.
	* semantics.c (begin_class_definition): Add attributes parameter,
	apply them to the type.

	PR c++/21581
	PR c++/25915
	* tree.c (decl_anon_ns_mem_p): New function.
	* cp-tree.h: Declare it.
	* decl2.c (determine_visibility): Make anonymous namespace
	members static.
	(min_vis_r, constrain_visibility): Likewise.
	* rtti.c (create_pseudo_type_info): Set TREE_PUBLIC on
	pseudo-types.
	* decl.c (cxx_init_decl_processing): Set TREE_PUBLIC on
	global_namespace.
	* name-lookup.c (push_namespace_with_attribs): Don't set TREE_PUBLIC
	on anonymous namespaces.

Added:
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/anon1.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/anon2.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/class1.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/prop1.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/redecl1.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/template1.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/template2.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/template3.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/template4.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/typeinfo1.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/warn1.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/warn2.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/warn3.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/warn4.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/template/anon2.C
Modified:
    branches/redhat/gcc-4_1-branch/gcc/ChangeLog
    branches/redhat/gcc-4_1-branch/gcc/attribs.c
    branches/redhat/gcc-4_1-branch/gcc/c-common.c
    branches/redhat/gcc-4_1-branch/gcc/c-decl.c
    branches/redhat/gcc-4_1-branch/gcc/cp/ChangeLog
    branches/redhat/gcc-4_1-branch/gcc/cp/class.c
    branches/redhat/gcc-4_1-branch/gcc/cp/cp-tree.h
    branches/redhat/gcc-4_1-branch/gcc/cp/decl.c
    branches/redhat/gcc-4_1-branch/gcc/cp/decl2.c
    branches/redhat/gcc-4_1-branch/gcc/cp/method.c
    branches/redhat/gcc-4_1-branch/gcc/cp/name-lookup.c
    branches/redhat/gcc-4_1-branch/gcc/cp/parser.c
    branches/redhat/gcc-4_1-branch/gcc/cp/pt.c
    branches/redhat/gcc-4_1-branch/gcc/cp/rtti.c
    branches/redhat/gcc-4_1-branch/gcc/cp/semantics.c
    branches/redhat/gcc-4_1-branch/gcc/cp/tree.c
    branches/redhat/gcc-4_1-branch/gcc/doc/extend.texi
    branches/redhat/gcc-4_1-branch/gcc/doc/invoke.texi
    branches/redhat/gcc-4_1-branch/gcc/flags.h
    branches/redhat/gcc-4_1-branch/gcc/objcp/ChangeLog
    branches/redhat/gcc-4_1-branch/gcc/objcp/objcp-decl.c
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/attrib14.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/attrib9.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/assign1.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/fvisibility-override2.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.dg/ext/visibility/virtual.C
    branches/redhat/gcc-4_1-branch/gcc/testsuite/g++.old-deja/g++.pt/enum5.C
    branches/redhat/gcc-4_1-branch/gcc/tree.c
    branches/redhat/gcc-4_1-branch/gcc/tree.h
    branches/redhat/gcc-4_1-branch/gcc/varasm.c

Comment 14 Andrew Pinski 2007-04-03 20:03:35 UTC
*** Bug 31462 has been marked as a duplicate of this bug. ***