diff mbox

[C++] fix alias and members

Message ID 4C0E1492.8020506@codesourcery.com
State Accepted
Headers show

Commit Message

Nathan Sidwell June 8, 2010, 9:59 a.m. UTC
I've committed this patch to fix uses of the alias attribute with member 
functions. Broken out of the IFUNC patch as these are independent goodness.

built and tested on i686-pc-linux-gnu.

nathan

Comments

Laurynas Biveinis June 8, 2010, 10:04 a.m. UTC | #1
2010/6/8 Nathan Sidwell <nathan@codesourcery.com>:
> I've committed this patch to fix uses of the alias attribute with member

The trunk is in freeze currently.
Nathan Sidwell June 8, 2010, 10:08 a.m. UTC | #2
On 06/08/10 11:04, Laurynas Biveinis wrote:
> 2010/6/8 Nathan Sidwell<nathan@codesourcery.com>:
>> I've committed this patch to fix uses of the alias attribute with member
>
> The trunk is in freeze currently.

It is?

http://gcc.gnu.org/
Active development:  GCC 4.6.0 (changes)
     Status: 2010-04-06 Stage 1, open for development.

nathan
Laurynas Biveinis June 8, 2010, 10:11 a.m. UTC | #3
2010/6/8 Nathan Sidwell <nathan@codesourcery.com>:
>> The trunk is in freeze currently.
>
> It is?
>
> http://gcc.gnu.org/
> Active development:  GCC 4.6.0 (changes)
>    Status: 2010-04-06 Stage 1, open for development.

At least it has been announced on the mailing lists:

http://gcc.gnu.org/ml/gcc/2010-06/msg00272.html
http://gcc.gnu.org/ml/gcc/2010-06/msg00307.html
Nathan Sidwell June 8, 2010, 10:49 a.m. UTC | #4
On 06/08/10 11:11, Laurynas Biveinis wrote:

> At least it has been announced on the mailing lists:
>
> http://gcc.gnu.org/ml/gcc/2010-06/msg00272.html
> http://gcc.gnu.org/ml/gcc/2010-06/msg00307.html

oh.  I see my cp/ commit has landed after your cp/ commit, would you still like 
it reverted?

nathan
Laurynas Biveinis June 8, 2010, 11:31 a.m. UTC | #5
2010/6/8 Nathan Sidwell <nathan@codesourcery.com>:
> oh.  I see my cp/ commit has landed after your cp/ commit, would you still
> like it reverted?

Should be fine to leave it in. Thanks.
Jason Merrill June 8, 2010, 1:46 p.m. UTC | #6
On 06/08/2010 05:59 AM, Nathan Sidwell wrote:
> +  alias = lookup_attribute ("alias", DECL_ATTRIBUTES (decl)) != 0;
> +
> +  if (alias && TREE_CODE (decl) == FUNCTION_DECL)
> +    record_key_method_defined (decl);

This seems wrong for weak aliases.

Jason
Nathan Sidwell June 8, 2010, 6:19 p.m. UTC | #7
On 06/08/10 14:46, Jason Merrill wrote:
> On 06/08/2010 05:59 AM, Nathan Sidwell wrote:
>> + alias = lookup_attribute ("alias", DECL_ATTRIBUTES (decl)) != 0;
>> +
>> + if (alias && TREE_CODE (decl) == FUNCTION_DECL)
>> + record_key_method_defined (decl);
>
> This seems wrong for weak aliases.

I'm not sure.  aliases have to be to another symbol defined in the same 
translation unit.  So, for the target to be in multiple TU's it'd have to be 
weak/comdat, which implies either an out-of-line inline function definition or a 
template instantiation.  Is that something that makes sense to support?

My understanding of the use-case for weak aliases was to a non-weak unique 
symbol.  So it seemed ok to me for the aliased symbol to be a key method.

nathan
Jason Merrill June 8, 2010, 7:19 p.m. UTC | #8
On 06/08/2010 02:19 PM, Nathan Sidwell wrote:
> On 06/08/10 14:46, Jason Merrill wrote:
>> On 06/08/2010 05:59 AM, Nathan Sidwell wrote:
>>> + alias = lookup_attribute ("alias", DECL_ATTRIBUTES (decl)) != 0;
>>> +
>>> + if (alias && TREE_CODE (decl) == FUNCTION_DECL)
>>> + record_key_method_defined (decl);
>>
>> This seems wrong for weak aliases.
>
> I'm not sure. aliases have to be to another symbol defined in the same
> translation unit.

Not if they're weak references:

static void f() __attribute ((weakref, alias ("g")));

int main()
{
   if (f)
     f();
}

does not require a definition of g.  Apparently weakrefs are different 
from weak aliases; I was previously confusing the two.  But this case 
still involves attribute "alias".

Jason
Nathan Sidwell June 8, 2010, 7:29 p.m. UTC | #9
On 06/08/10 20:19, Jason Merrill
> static void f() __attribute ((weakref, alias ("g")));
>
> int main()
> {
> if (f)
> f();
> }
>
> does not require a definition of g. Apparently weakrefs are different
> from weak aliases; I was previously confusing the two. But this case
> still involves attribute "alias".

hm, ok, but a weakref must have static (local) linkage, which is something a 
member function cannot have.  The case where a member function could behave 
as-if it had static linkage would be in an anonymous namespace.  But in that 
case the class itself is local to the TU.  So again, I think we're ok.

nathan
Jason Merrill June 8, 2010, 7:46 p.m. UTC | #10
On 06/08/2010 03:29 PM, Nathan Sidwell wrote:
> hm, ok, but a weakref must have static (local) linkage, which is
> something a member function cannot have. The case where a member
> function could behave as-if it had static linkage would be in an
> anonymous namespace. But in that case the class itself is local to the
> TU. So again, I think we're ok.

Fair enough.

Jason
diff mbox

Patch

2010-06-08  Nathan Sidwell  <nathan@codesourcery.com>

	cp/
	* decl.c (record_key_method_defined): New, broken out of ...
	(finish_function): ... here.  Call it.	
	(start_decl): Treat aliases as definitions.

	testsuite/
	* g++.dg/ext/attr-alias-1.C: New.
	* g++.dg/ext/attr-alias-2.C: New.

Index: testsuite/g++.dg/ext/attr-alias-1.C
===================================================================
--- testsuite/g++.dg/ext/attr-alias-1.C	(revision 0)
+++ testsuite/g++.dg/ext/attr-alias-1.C	(revision 0)
@@ -0,0 +1,37 @@ 
+/* { dg-do run }  */
+/* { dg-require-alias "" } */
+
+#include <typeinfo>
+
+struct Klass
+{
+  int implementation () const;
+  int magic () const;
+};
+
+int Klass::implementation (void) const
+{
+  return 0;
+}
+
+int Klass::magic () const
+  __attribute__ ((alias ("_ZNK5Klass14implementationEv")));
+
+int __attribute__ ((noinline))
+  Foo (Klass const *ptr)
+{
+  if (ptr->magic () != 0)
+    return 1;
+
+  if (typeid (*ptr) != typeid (Klass))
+    return 2;
+
+  return 0;
+}
+
+int main ()
+{
+  Klass obj;
+  
+  return Foo (&obj);
+}
Index: testsuite/g++.dg/ext/attr-alias-2.C
===================================================================
--- testsuite/g++.dg/ext/attr-alias-2.C	(revision 0)
+++ testsuite/g++.dg/ext/attr-alias-2.C	(revision 0)
@@ -0,0 +1,37 @@ 
+/* { dg-do run }  */
+/* { dg-require-alias "" } */
+
+#include <typeinfo>
+
+struct Klass
+{
+  int implementation () const;
+  virtual int magic () const;
+};
+
+int Klass::implementation (void) const
+{
+  return 0;
+}
+
+int Klass::magic () const
+  __attribute__ ((alias ("_ZNK5Klass14implementationEv")));
+
+int __attribute__ ((noinline))
+  Foo (Klass const *ptr)
+{
+  if (ptr->magic () != 0)
+    return 1;
+
+  if (typeid (*ptr) != typeid (Klass))
+    return 2;
+
+  return 0;
+}
+
+int main ()
+{
+  Klass obj;
+  
+  return Foo (&obj);
+}
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 160427)
+++ cp/decl.c	(working copy)
@@ -90,6 +90,7 @@ 
 static void finish_constructor_body (void);
 static void begin_destructor_body (void);
 static void finish_destructor_body (void);
+static void record_key_method_defined (tree);
 static tree create_array_type_for_decl (tree, tree, tree);
 static tree get_atexit_node (void);
 static tree get_dso_handle_node (void);
@@ -4129,6 +4130,7 @@ 
   tree context;
   bool was_public;
   int flags;
+  bool alias;
 
   *pushed_scope_p = NULL_TREE;
 
@@ -4190,6 +4192,10 @@ 
       if (toplevel_bindings_p ())
 	TREE_STATIC (decl) = 1;
     }
+  alias = lookup_attribute ("alias", DECL_ATTRIBUTES (decl)) != 0;
+  
+  if (alias && TREE_CODE (decl) == FUNCTION_DECL)
+    record_key_method_defined (decl);
 
   /* If this is a typedef that names the class for linkage purposes
      (7.1.3p8), apply any attributes directly to the type.  */
@@ -4292,7 +4298,9 @@ 
 	    DECL_EXTERNAL (decl) = 1;
 	}
 
-      if (DECL_EXTERNAL (decl) && ! DECL_TEMPLATE_SPECIALIZATION (decl))
+      if (DECL_EXTERNAL (decl) && ! DECL_TEMPLATE_SPECIALIZATION (decl)
+	  /* Aliases are definitions. */
+	  && !alias)
 	permerror (input_location, "declaration of %q#D outside of class is not definition",
 		   decl);
 
@@ -12502,6 +12510,22 @@ 
   return block;
 }
 
+/* If FNDECL is a class's key method, add the class to the list of
+   keyed classes that should be emitted.  */
+
+static void
+record_key_method_defined (tree fndecl)
+{
+  if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl)
+      && DECL_VIRTUAL_P (fndecl)
+      && !processing_template_decl)
+    {
+      tree fnclass = DECL_CONTEXT (fndecl);
+      if (fndecl == CLASSTYPE_KEY_METHOD (fnclass))
+	keyed_classes = tree_cons (NULL_TREE, fnclass, keyed_classes);
+    }
+}
+
 /* Finish up a function declaration and compile that function
    all the way to assembler language output.  The free the storage
    for the function definition.
@@ -12528,14 +12552,7 @@ 
   gcc_assert (!defer_mark_used_calls);
   defer_mark_used_calls = true;
 
-  if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fndecl)
-      && DECL_VIRTUAL_P (fndecl)
-      && !processing_template_decl)
-    {
-      tree fnclass = DECL_CONTEXT (fndecl);
-      if (fndecl == CLASSTYPE_KEY_METHOD (fnclass))
-	keyed_classes = tree_cons (NULL_TREE, fnclass, keyed_classes);
-    }
+  record_key_method_defined (fndecl);
 
   nested = function_depth > 1;
   fntype = TREE_TYPE (fndecl);