[ios] Improve geometry method definitions #12445
Conversation
| @@ -100,7 +98,7 @@ CLLocationDirection MGLDirectionBetweenCoordinates(CLLocationCoordinate2D firstC | |||
|
|
|||
| CGPoint MGLPointRounded(CGPoint point) { | |||
| #if TARGET_OS_IPHONE || TARGET_OS_SIMULATOR | |||
| CGFloat scaleFactor = [UIScreen instancesRespondToSelector:@selector(nativeScale)] ? [UIScreen mainScreen].nativeScale : [UIScreen mainScreen].scale; | |||
| CGFloat scaleFactor = [UIScreen mainScreen].nativeScale; | |||
There was a problem hiding this comment.
This is the iOS 7 compatibility that’s being removed, by the way. Removal of iOS 8 compatibility might be similarly subtle in the future.
/cc @lloydsheng
friedbunny
added
the
needs changelog
There was a problem hiding this comment.
Does MGLSubexpressionsWithJSONObjects in NSExpression+MGLPrivateAdditions.h also need the modified extern?
Do you think it would be beneficial to use FOUNDATION_EXTERN everywhere so that the code is consistent in how functions are exported. Would also prevent copy-🍝 from allowing this in the future.
|
Per @asheemmamoowala’s review:
You’re correct that this function could also suffer from the same mangling issue as Before: After:
This seems prudent to me — switching all of the Our docs even remain the same, since it’s just an alias:
|
`MGLMapPointForCoordinate()` was `extern`-defined in a C header, but implemented in an Obj-C++ source file and mangled as a C++ symbol. `FOUNDATION_EXTERN` is C++-aware and will force C-style symbol mangling, which fixes the undefined symbol issue in consumer Obj-C projects.
`UIScreen.nativeScale` is available beginning in iOS 8, so the conditional is no longer necessary.
FOUNDATION_EXTERN is an alias for extern in most cases, but also covers us in the event that a method could be mismangled by the compiler as C++ (when we want C-style mangling, for Obj-C compatibility).
friedbunny
force-pushed
the
fb-geometry-externs
branch
from
43547f2 to
cdd3553
Compare
friedbunny
removed
the
needs changelog
Fixes #12079.
Here’s my (potentially incorrect) understanding of this fix:
MGLMapPointForCoordinate()wasextern-defined in a C header, but implemented in an Obj-C++ source file and mangled as a C++ symbol.FOUNDATION_EXTERNis C++-aware and will force C-style symbol mangling, which fixes the undefined symbol issue in consumer Obj-C projects (#12079).You can verify the change in symbol mangling using
nm:Before:
After:
This PR also:
MGL_EXPORTfor two private methods toMGLGeometry_Private.h, rather than defining them with the implementation.MGLPointRounded()and removes obsolete iOS 7 compatibility./cc @1ec5 @julianrex @kkaefer @asheemmamoowala