Bug 191201 – Do not re-declare soft-linked functions in SOFT_LINK* macros

• Component: New Bugs
• Status: NEW

[Jer Noble]

Do not re-declare soft-linked functions in SOFT_LINK* macros

[Jer Noble]

Created attachment 353718 [details]
Patch

[Jer Noble]

Created attachment 353719 [details]
Patch

[Jer Noble]

Created attachment 353721 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 353721 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=353721&action=review

> Source/WTF/ChangeLog:8
> +        Re-declaring soft-linked functions can cause build errors when upstream frameworks change, for example, their export attributes

This is a good thing, we don’t want to diverge from what the framework declares. 

It’s similar to how we redeclare as much as possible in SOI headers.

[Jer Noble]

Comment on attachment 353721 [details]
Patch

(In reply to Alexey Proskuryakov from comment #4)
> Comment on attachment 353721 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=353721&action=review
> 
> > Source/WTF/ChangeLog:8
> > +        Re-declaring soft-linked functions can cause build errors when upstream frameworks change, for example, their export attributes
> 
> This is a good thing, we don’t want to diverge from what the framework
> declares. 
> 
> It’s similar to how we redeclare as much as possible in SOI headers.

I get the goal. But we explicitly redeclare them as __always_inline__ and visibility('hidden') attributes to our own redeclaration, so we are explicitly redeclaring them to be different than the framework.  So if this is our goal, we need to stop doing that. Until then, your premise is invalid.

[Jer Noble]

(In reply to Jer Noble from comment #5)
> Comment on attachment 353721 [details]
> Patch
> 
> (In reply to Alexey Proskuryakov from comment #4)
> > Comment on attachment 353721 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=353721&action=review
> > 
> > > Source/WTF/ChangeLog:8
> > > +        Re-declaring soft-linked functions can cause build errors when upstream frameworks change, for example, their export attributes
> > 
> > This is a good thing, we don’t want to diverge from what the framework
> > declares. 
> > 
> > It’s similar to how we redeclare as much as possible in SOI headers.
> 
> I get the goal. But we explicitly redeclare them as __always_inline__ and
> visibility('hidden') attributes to our own redeclaration, so we are
> explicitly redeclaring them to be different than the framework.  So if this
> is our goal, we need to stop doing that. Until then, your premise is invalid.

Also, we do /not/ match the declarations in the SPI headers either, since we routinely have to add "NS_ASSUME_NONNULL_BEGIN/END" macros around classes and methods.

[Alex Christensen]

Comment on attachment 353721 [details]
Patch

This has been requesting review for more than one year.  If this is still needed, please rebase and re-request review.