Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

-Wfloat-equal produces compiler warning in Mapbox SDK #11898

Closed
leviathan opened this issue May 14, 2018 · 4 comments
Closed

-Wfloat-equal produces compiler warning in Mapbox SDK #11898

leviathan opened this issue May 14, 2018 · 4 comments
Labels
archived Archived because of inactivity bug iOS Mapbox Maps SDK for iOS

Comments

@leviathan
Copy link

Platform:

MacOS high sierra - 10.13.4 (17E202)
Xcode Version 9.3 (9E145)
Carthage 0.29.0

Mapbox SDK version:

Maps SDK for iOS v4.0.0

Steps to trigger behavior

  1. Include Mapbox SDK via carthage into the project.
  2. In Xcode build settings Treat Warnings as Errors = YES
  3. In xcconfig set WARNING_CFLAGS = -Wfloat-equal

Expected behavior

No warning is produced, because float numbers are compared for equality as suggested by Apple (e.g. `fabs(...) & epsilon).

Actual behavior

Compiler warning is produced:

Mapbox.framework/Headers/MGLGeometry.h:33:33: Comparing floating point with == or != is unsafe

screenshot 2018-05-14 09 01 00

@friedbunny friedbunny added bug iOS Mapbox Maps SDK for iOS labels May 14, 2018
@friedbunny
Copy link
Contributor

friedbunny commented May 14, 2018

Thanks for the report — I’m unsure if this warning represents a practical issue, as there’s some dispute about this tricky topic.

As for how MGLCoordinateSpans are generated and used, I think manually input values could be problematic in terms of comparison:

- (void)testCoordinateSpan {
    MGLCoordinateBounds bounds = MGLCoordinateBoundsMake(CLLocationCoordinate2DMake(38.9131982, -77.0325453144239),
                                                         CLLocationCoordinate2DMake(37.7757368, -122.4135302));

    MGLCoordinateSpan span = MGLCoordinateBoundsGetCoordinateSpan(bounds);
    MGLCoordinateSpan expectedSpan = MGLCoordinateSpanMake(-1.1374614, -45.3809848856);

//    error: -[MGLGeometryTests testCoordinateSpan] : ((span.latitudeDelta) equal to (expectedSpan.latitudeDelta)) failed: ("-1.1374614") is not equal to ("-1.1374614")
//    error: -[MGLGeometryTests testCoordinateSpan] : ((span.longitudeDelta) equal to (expectedSpan.longitudeDelta)) failed: ("-45.3809848856") is not equal to ("-45.3809848856")

    expectedSpan = MGLCoordinateSpanMake(-1.1374613999999994, -45.380984885576098);

    XCTAssertEqual(span.latitudeDelta, expectedSpan.latitudeDelta);
    XCTAssertEqual(span.longitudeDelta, expectedSpan.longitudeDelta);

    XCTAssertTrue(MGLCoordinateSpanEqualToCoordinateSpan(span, expectedSpan));

    //
}

... but I don’t think manual input is a use case we’ve ever seen — MGLCoordinateSpan is always generated by the SDK from existing bounds, where full precision is maintained. This isn’t a topic I’m particularly well versed in, though, so perhaps @1ec5 and @julianrex have thoughts.

@1ec5
Copy link
Contributor

1ec5 commented May 14, 2018

MGLCoordinateSpanEqualToCoordinateSpan() is a pretty obscure function provided for completeness rather than a concrete need in the SDK itself. MapKit apparently doesn’t have a corresponding MKCoordinateSpanEqualToCoordinateSpan() function, even though that library uses coordinate spans a lot more heavily than this SDK does.

Some options:

  • Introduce an epsilon tolerance into the comparison. Does this change the expected semantics of an “is equal” function?
  • Move the implementation of this function into MGLGeometry.mm so that the inlined code doesn’t pollute application targets’ build logs.
  • Use a pragma to ignore the warning.

/ref #6060

@leviathan
Copy link
Author

Regarding severity, for me this issue is rather low priority.
I did solve it on our end, by "just" removing the -Wfloat-equal warning flag.

For me it would have been helpful to find a hint about this (or other conflicting compiler flags) in the Mapbox docs. So, that I know that the Mapbox SDK is working properly and I don’t get the impression that the SDK version, I’m trying to build is "kaputt".

-Wundef does trigger a compiler error as well, e.g. "TARGET_INTERFACE_BUILDER" not defined".

@stale stale bot added the archived Archived because of inactivity label Nov 11, 2018
@stale
Copy link

stale bot commented Nov 26, 2018

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Nov 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived Archived because of inactivity bug iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

No branches or pull requests

3 participants