diff mbox

[libobjc] export __objc_get_forward_imp, get_imp again

Message ID 54BF6B0F.5040502@ubuntu.com
State New
Headers show

Commit Message

Matthias Klose Jan. 21, 2015, 9:02 a.m. UTC
__objc_get_forward_imp and get_imp were exported in libobjc since GCC 4.1, for
some reason these are not exported anymore in GCC 5 (both declared inline).  So
either export these as before, or don't export them and bump the soname.  The
latter seems to be unwanted, and at least gnustep-base is using the get_imp
function.  So better keep the references in GCC 5?

Is this an intended change in GCC 5 to not to export inline methods anymore?

  Matthias

Comments

Marek Polacek Jan. 21, 2015, 9:09 a.m. UTC | #1
On Wed, Jan 21, 2015 at 10:02:07AM +0100, Matthias Klose wrote:
> __objc_get_forward_imp and get_imp were exported in libobjc since GCC 4.1, for
> some reason these are not exported anymore in GCC 5 (both declared inline).  So
> either export these as before, or don't export them and bump the soname.  The
> latter seems to be unwanted, and at least gnustep-base is using the get_imp
> function.  So better keep the references in GCC 5?
> 
> Is this an intended change in GCC 5 to not to export inline methods anymore?

See https://gcc.gnu.org/gcc-5/porting_to.html

> libobjc/
> 
> 	* sendmsg.c (__objc_get_forward_imp, get_imp): Declare extern inline.
> 
> --- a/src/libobjc/sendmsg.c
> +++ b/src/libobjc/sendmsg.c
> @@ -105,7 +105,7 @@
>  id nil_method (id, SEL);
>  
>  /* Given a selector, return the proper forwarding implementation.  */
> -inline
> +extern inline
>  IMP
>  __objc_get_forward_imp (id rcv, SEL sel)
>  {
> @@ -320,7 +320,7 @@
>    return res;
>  }
>  
> -inline
> +extern inline
>  IMP
>  get_imp (Class class, SEL sel)
>  {

Looks ok.

	Marek
Jakub Jelinek Jan. 21, 2015, 9:14 a.m. UTC | #2
On Wed, Jan 21, 2015 at 10:02:07AM +0100, Matthias Klose wrote:
> __objc_get_forward_imp and get_imp were exported in libobjc since GCC 4.1, for
> some reason these are not exported anymore in GCC 5 (both declared inline).  So
> either export these as before, or don't export them and bump the soname.  The
> latter seems to be unwanted, and at least gnustep-base is using the get_imp
> function.  So better keep the references in GCC 5?
> 
> Is this an intended change in GCC 5 to not to export inline methods anymore?

This will make the code require -std=c99 or later, it will fail to export
the functions with -std=gnu89.
Wouldn't it be better to just add
extern IMP __objc_get_forward_imp (id, SEL);
extern IMP get_imp (Class, SEL);
prototypes before the inline function definitions (in some header, whatever)?
That way it seems both -std=gnu89 and -std=gnu99 export those two functions
as well as make them inline.

> libobjc/
> 
> 	* sendmsg.c (__objc_get_forward_imp, get_imp): Declare extern inline.
> 
> --- a/src/libobjc/sendmsg.c
> +++ b/src/libobjc/sendmsg.c
> @@ -105,7 +105,7 @@
>  id nil_method (id, SEL);
>  
>  /* Given a selector, return the proper forwarding implementation.  */
> -inline
> +extern inline
>  IMP
>  __objc_get_forward_imp (id rcv, SEL sel)
>  {
> @@ -320,7 +320,7 @@
>    return res;
>  }
>  
> -inline
> +extern inline
>  IMP
>  get_imp (Class class, SEL sel)
>  {


	Jakub
Andrew Pinski Jan. 21, 2015, 4:41 p.m. UTC | #3
> On Jan 21, 2015, at 1:02 AM, Matthias Klose <doko@ubuntu.com> wrote:
> 
> __objc_get_forward_imp and get_imp were exported in libobjc since GCC 4.1, for
> some reason these are not exported anymore in GCC 5 (both declared inline).  So
> either export these as before, or don't export them and bump the soname.  The
> latter seems to be unwanted, and at least gnustep-base is using the get_imp
> function.  So better keep the references in GCC 5?
> 
> Is this an intended change in GCC 5 to not to export inline methods anymore?

Just remove the inline instead. 

Thanks,
Andrew

> 
>  Matthias
> 
> <libobjc-extern-inline.diff>
Jakub Jelinek Jan. 21, 2015, 4:51 p.m. UTC | #4
On Wed, Jan 21, 2015 at 08:41:46AM -0800, pinskia@gmail.com wrote:
> > On Jan 21, 2015, at 1:02 AM, Matthias Klose <doko@ubuntu.com> wrote:
> > 
> > __objc_get_forward_imp and get_imp were exported in libobjc since GCC 4.1, for
> > some reason these are not exported anymore in GCC 5 (both declared inline).  So
> > either export these as before, or don't export them and bump the soname.  The
> > latter seems to be unwanted, and at least gnustep-base is using the get_imp
> > function.  So better keep the references in GCC 5?
> > 
> > Is this an intended change in GCC 5 to not to export inline methods anymore?
> 
> Just remove the inline instead. 

The comments like:

/* The new name of get_imp().  */
IMP
class_getMethodImplementation (Class class_, SEL selector)
{
  if (class_ == Nil  ||  selector == NULL)
    return NULL;
  
  /* get_imp is inlined, so we're good.  */
  return get_imp (class_, selector);
}

don't make me very confident in such a change.
The extern prototypes really work with both -std=gnu89 and -std=gnu11 and
thus will at least keep status quo.

	Jakub
Andrew Pinski Jan. 21, 2015, 11:56 p.m. UTC | #5
On Wed, Jan 21, 2015 at 8:51 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Jan 21, 2015 at 08:41:46AM -0800, pinskia@gmail.com wrote:
>> > On Jan 21, 2015, at 1:02 AM, Matthias Klose <doko@ubuntu.com> wrote:
>> >
>> > __objc_get_forward_imp and get_imp were exported in libobjc since GCC 4.1, for
>> > some reason these are not exported anymore in GCC 5 (both declared inline).  So
>> > either export these as before, or don't export them and bump the soname.  The
>> > latter seems to be unwanted, and at least gnustep-base is using the get_imp
>> > function.  So better keep the references in GCC 5?
>> >
>> > Is this an intended change in GCC 5 to not to export inline methods anymore?
>>
>> Just remove the inline instead.
>
> The comments like:
>
> /* The new name of get_imp().  */
> IMP
> class_getMethodImplementation (Class class_, SEL selector)
> {
>   if (class_ == Nil  ||  selector == NULL)
>     return NULL;
>
>   /* get_imp is inlined, so we're good.  */
>   return get_imp (class_, selector);
> }
>
> don't make me very confident in such a change.
> The extern prototypes really work with both -std=gnu89 and -std=gnu11 and
> thus will at least keep status quo.

Let's do that then.

This also fixes bug 63863.

Thanks,
Andrew Pinski


>
>         Jakub
Matthias Klose Jan. 22, 2015, 4:09 p.m. UTC | #6
On 01/22/2015 12:56 AM, Andrew Pinski wrote:
> On Wed, Jan 21, 2015 at 8:51 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Wed, Jan 21, 2015 at 08:41:46AM -0800, pinskia@gmail.com wrote:
>>>> On Jan 21, 2015, at 1:02 AM, Matthias Klose <doko@ubuntu.com> wrote:
>>>>
>>>> __objc_get_forward_imp and get_imp were exported in libobjc since GCC 4.1, for
>>>> some reason these are not exported anymore in GCC 5 (both declared inline).  So
>>>> either export these as before, or don't export them and bump the soname.  The
>>>> latter seems to be unwanted, and at least gnustep-base is using the get_imp
>>>> function.  So better keep the references in GCC 5?
>>>>
>>>> Is this an intended change in GCC 5 to not to export inline methods anymore?
>>>
>>> Just remove the inline instead.
>>
>> The comments like:
>>
>> /* The new name of get_imp().  */
>> IMP
>> class_getMethodImplementation (Class class_, SEL selector)
>> {
>>   if (class_ == Nil  ||  selector == NULL)
>>     return NULL;
>>
>>   /* get_imp is inlined, so we're good.  */
>>   return get_imp (class_, selector);
>> }
>>
>> don't make me very confident in such a change.
>> The extern prototypes really work with both -std=gnu89 and -std=gnu11 and
>> thus will at least keep status quo.
> 
> Let's do that then.

get_imp was renamed to class_getMethodImplementation, which is exported from
objc/runtime.h.  GNUstep-base uses get_imp to define it's own
class_getMethodImplementation, so get_imp isn't really needed. So either make
the two functions inline, and don't export them, or declare the prototypes.  For
the latter I would suggest objc-private/runtime.h (class_getMethodImplementation
is declared in objc/runtime.h).

  Matthias
diff mbox

Patch

libobjc/

	* sendmsg.c (__objc_get_forward_imp, get_imp): Declare extern inline.

--- a/src/libobjc/sendmsg.c
+++ b/src/libobjc/sendmsg.c
@@ -105,7 +105,7 @@ 
 id nil_method (id, SEL);
 
 /* Given a selector, return the proper forwarding implementation.  */
-inline
+extern inline
 IMP
 __objc_get_forward_imp (id rcv, SEL sel)
 {
@@ -320,7 +320,7 @@ 
   return res;
 }
 
-inline
+extern inline
 IMP
 get_imp (Class class, SEL sel)
 {