Bug 193815 – Move soft-linking of Lookup.framework out of LookupSPI.h

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[David Kilzer (:ddkilzer)]

Move soft-linking of Lookup.framework out of LookupSPI.h.

Header should never soft-link frameworks or libraries since that code can be included in multiple translation units (source files), causing duplicate code to be generated.  See Bug 193750 for a new check-webkit-style check that flags this issue.

[David Kilzer (:ddkilzer)]

Created attachment 360083 [details]
Patch v1

[Radar WebKit Bug Importer]

<rdar://problem/47540816>

[Tim Horton]

Comment on attachment 360083 [details]
Patch v1

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

> Source/WTF/wtf/cocoa/SoftLinking.h:632
> +#define SOFT_LINK_VARIABLE_MAY_FAIL_FOR_HEADER(functionNamespace, framework, variableName, variableType) \

'Variable'?! Why is this different from SOFT_LINK_CONSTANT_*? (I'm too lazy to compare)

[David Kilzer (:ddkilzer)]

(In reply to Tim Horton from comment #3)
> Comment on attachment 360083 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=360083&action=review
> 
> > Source/WTF/wtf/cocoa/SoftLinking.h:632
> > +#define SOFT_LINK_VARIABLE_MAY_FAIL_FOR_HEADER(functionNamespace, framework, variableName, variableType) \
> 
> 'Variable'?! Why is this different from SOFT_LINK_CONSTANT_*? (I'm too lazy
> to compare)

I made a bad design decision when implementing the SOFT_LINK_CONSTANT*() macros, which was to include a local declaration of the constant like this:

#define SOFT_LINK_CONSTANT_FOR_HEADER(functionNamespace, framework, variableName, variableType) \
    WTF_EXTERN_C_BEGIN \
    extern const variableType variableName; \
    WTF_EXTERN_C_END \
    namespace functionNamespace { \
    variableType get_##framework##_##variableName(); \
    }

If that declaration exactly doesn't match the original (either in an *SPI.h header or the "real" header), then we get compiler warnings-as-errors.

Furthermore, the actual soft-linking code doesn't care about `const` keywords in the `variableType`, so to keep the declaration and keep the macros working, we'd need two different types (one that includes the original `const` keywords and one that doesn't).

The simple fix is to separate concerns and let the declaration reside in an *SPI.h header or the "real" header, and remove the declaration from the SOFT_LINK_CONSTANT*() macros.  (I tried it and the macOS Release build didn't break, so I'll make that change along with this patch.)

[David Kilzer (:ddkilzer)]

Created attachment 360124 [details]
Patch v2

[WebKit Commit Bot]

Comment on attachment 360124 [details]
Patch v2

Clearing flags on attachment: 360124

Committed r240494: <https://trac.webkit.org/changeset/240494>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.