maplibre / maplibre-native

MapLibre Native - Interactive vector tile maps for iOS, Android and other platforms.
https://maplibre.org
BSD 2-Clause "Simplified" License
1.09k stars 322 forks source link

Remove patched NSExpressions #331

Open nvanfleet opened 2 years ago

nvanfleet commented 2 years ago

I've started getting an error log on device when using iOS 15.5. I'm not sure if it would happen in simulator since Maplibre currently crashes for iOS 15.

Xcode 13.4

2022-06-06 11:34:47.472457-0700 Lyft[42141:5886591] [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden.

I tried to look around at things in terms of how we use this Expression in our codebase. We basically use: mgl_interpolate:withCurveType:parameters:stops:($lineProgress, 'linear', nil, %@)

I was able to find some things online that said you can wrap the function here https://github.com/mapbox/mapbox-gl-native-ios/issues/271

So I tried that by doing (mgl_interpolate:withCurveType:parameters:stops:($lineProgress, 'linear', nil, %@)) but it still issues errors.

I also tried to migrate to use the NSExpression conveniences from Maplibre. But I still get errors issues from those, so it seems they are also mis-formatted (?).

This is definitely polluting the logs but I am worried that somehow this will not be supported in the future by Apple. I think I was also getting some failures in my tests because it caused an unhandled exception.

wipfli commented 2 years ago

Did the error not appear in older versions?

nvanfleet commented 2 years ago

It did not. I am not sure if this is related to iOS or Xcode updates.

Sent from my iPhone

On Jun 15, 2022, at 11:16 AM, Oliver Wipfli @.***> wrote:

ο»Ώ Did the error not appear in older versions?

β€” Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

nvanfleet commented 2 years ago

I'd be surprised if this wasn't something that could be repro'd by others πŸ€”

wipfli commented 2 years ago

@nvanfleet do you know if we can reproduce this bug in our iOS CI?

xD3CODER commented 2 years ago

I got same error on iOS 15.5 too

roblabs commented 2 years ago

In Xcode there are XCTests in the MapLibre CI scheme that can replicate this issue of NSExpression function is forbidden.

To replicate in the Mapbox Unit Tests -> switch to CI scheme -> Set simulator SDK -> ⌘U (Test).

Simulator SDK NSExpression Tests in CI Scheme
14.5 βœ…
15.4 βœ…
15.5 ❌ β€”Β (30/68 NSExpression tests failing)

Inspecting the logs shows there are two types of Exceptions.

  1. is not a supported method. (NSInternalInconsistencyException)

Assertions: Uncaught Exception at MGLDocumentationExampleTests.swift:109 mgl_interpolate:withCurveType:parameters:stops: is not a supported method. (NSInternalInconsistencyException) File: MGLDocumentationExampleTests.swift:109

  1. Unable to parse function name (NSInvalidArgumentException)

Assertions: Uncaught Exception at MGLDocumentationExampleTests.swift:327: Unable to parse function name 'mgl_interpolate:withCurveType:parameters:stops:' into supported selector (mgl_interpolate:withCurveType:parameters:stops:) (NSInvalidArgumentException) File: MGLDocumentationExampleTests.swift:327


Xcode Tests


The Xcode logs show events for [general] NSPredicate which are likely coming from the operating system. There are two types of log entries.

  1. Use of '<selector>' as an NSExpression function is forbidden.
  2. NSFunctionExpression with selector '<selector>' is forbidden.
  3. List of 34 warnings from the Xcode console when running tests on CI scheme.

[general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_step:from:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'MGL_MATCH' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_step:from:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_step:from:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_round:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_coalesce:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'MGL_IF' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_step:from:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_attributed:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'MGL_FUNCTION' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_coalesce:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_does:have:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'MGL_MATCH' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_distanceFrom:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_step:from:stops:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_join:' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'mgl_log2:' as an NSExpression function is forbidden. [general] NSPredicate: NSFunctionExpression with selector 'mgl_number' is forbidden. [general] NSPredicate: NSFunctionExpression with selector 'mgl_numberWithFallbackValues:' is forbidden. [general] NSPredicate: NSFunctionExpression with selector 'mgl_numberWithFallbackValues:' is forbidden. [general] NSPredicate: NSFunctionExpression with selector 'mgl_numberWithFallbackValues:' is forbidden. [general] NSPredicate: Use of 'MGL_FUNCTION' as an NSExpression function is forbidden. [general] NSPredicate: Use of 'MGL_LET' as an NSExpression function is forbidden.

georgbachmann commented 2 years ago

I am seeing the same error. Does anybody have a solution yet?

yanniks commented 2 years ago

Also effected. Problem also occurs on iOS 16 / Xcode 14 beta.

birkskyum commented 2 years ago

fixed by #411

CooperWolfe commented 2 years ago

How would one use this syntax with feature attributes? In the diffs, it looks like the magnitude attribute was mistakenly replaced with .zoomLevelVariable: https://github.com/maplibre/maplibre-gl-native/pull/411/files#diff-497e9b9ec44106c19f48191690d47af4a33123966eae167946b2eb81523deeb2

nvanfleet commented 2 years ago

@CooperWolfe I think generally just use the extensions in NSExpression+MGLAdditions.h and things should be fine for you. I don't know if that covers all use cases of NSExpression but for me it was exhaustive.

CooperWolfe commented 2 years ago

Which one? The constants don't include custom feature attributes, and in fact you've changed the example in the linked diff. NSExpression(forConstantValue: "magnitude") and NSExpression(forVariable: "magnitude") throw errors. Would it be NSExpression(forKeyPath: "magnitude")? It doesn't throw an error, but neither does NSExpression(forKeyPath: "bogus").

nvanfleet commented 2 years ago

@CooperWolfe I haven't changed anything. If there isn't a mgl_expressionFor that satisfies what you need you can look at how the NSEspressions are initialized in the NSExpression+MGLAdditions.mm using selectorName: to avoid getting the error.

roblabs commented 2 years ago

@CooperWolfe β€” You may find useful the docs for Expressions


The example you are referring to came from the original open source project and I agree it is incomplete. The links I posted here may be of more use.

CooperWolfe commented 2 years ago

I appreciate you both trying to point me in the right direction, but the matter stands that there are no examples provided here, either in NSExpression.MGLAdditions.mm, nor any of those docs, of instantiating an NSExpression to interpolate on a custom feature attribute without getting the error log that prompted this issue in the first place, namely:

NSPredicate: Use of 'mgl_interpolate:withCurveType:parameters:stops:' as an NSExpression function is forbidden

I am successfully able to do what I need to do, as your referenced documentation offers, but not without that error log. If this issue is indeed

fixed by #411

then an example of it working with a custom feature attribute would be nice. Given that #411 alters precisely one of said examples in the comments of MGLHeatmapStyleLayer.h, but does so incorrectly, altering the example (to .zoomLevelVariable) rather than transposing it to the new way MGL Expressions are defined, I ask the question here what the correct transposition of that example would be.

If, instead, #411

is incomplete

then I'd argue so is this issue. I don't believe that to be the case, and instead believe that within NSExpression.MGLAdditions.mm is a solution. However, I can't find said solution anywhere, in published documentation or otherwise.

ianthetechie commented 1 year ago

Sorry for the necro, but I'm a bit confused if this is supposed to be fixed in iOS v5.13. It seems like it should be, but I must be missing something:

Swift code (minimal, obviously useless example):

NSExpression(forMGLMatchingKey: NSExpression(forConstantValue: "attribute"),
                                            in: [:],
                                            default: NSExpression(forConstantValue: nil))

Crash:

NSPredicate: Use of 'MGL_MATCH' as an NSExpression function is forbidden.
/Users/ianthetechie/MapLibreSwiftUI/Tests/MapLibreSwiftDSLTests/PointFeatureTests.swift:26: error: -[MapLibreSwiftDSLTests.PointFeatureTests testPointFeatureBuilderSetAttributes] : MGL_MATCH is not a supported method. (NSInternalInconsistencyException)

This code is using an "approved" extension method, so I'm not really sure why it's blowing up as this appears to have been fixed before the last release.

ianthetechie commented 1 year ago

Quick update here copying over from a Slack discussion.

I attempted using a slightly different construction and parsing from a simple object/array construction like you'd use in JSON:

NSExpression(mglJSONObject: [
    "match",
    ["get", "attribute"],
    "fooAttr", "fooValue",
    "barAttr", "barValue",
    "default"
])

This resulted in a similar error:

2023-09-11 18:06:16.379420+0900 xctest[53031:65866663] [general] NSPredicate: Use of 'MGL_MATCH' as an NSExpression function is forbidden.
/Users/ianthetechie/MapLibreSwiftUI/Tests/MapLibreSwiftDSLTests/PointFeatureTests.swift:26: error: -[MapLibreSwiftDSLTests.PointFeatureTests testPointFeatureBuilderSetAttributes] : MGL_MATCH is not a supported method. (NSInternalInconsistencyException)

Long story short, I'm not quite sure what the "right" way is to create an expression in iOS v5.13.0, but they all seem broken. I would be happy to test once the Swift package is updated, since I'm currently a bit too lazy to develop off master and build a new Swift package locally myself πŸ˜‚ @louwers already mentioned he's working on automatic releases from master for SPM so I'm just waiting till that comes out (I already want to be able to do that anyways so I can track 6.x changes since the projects I'm working on are longer burn and will target latest of everything) πŸŽƒ If it's still broken there, I will go through the trouble to build + troubleshoot locally.

ianthetechie commented 1 year ago

Sadly it seems to be still broken in the latest master builds. cc @louwers.

image
Heidi0039 commented 9 months ago

I wonder why this is closed? Is this resolved?

Mikhail-Liapich commented 9 months ago

We encounter the same problem when we try to parse expression with 'MGL_IF'

image
andresinaka commented 7 months ago

@Mikhail-Liapich did you find any solution to avoid the warnings?

georgbachmann commented 7 months ago

I am also still seeing a ton of those warnings...

NSPredicate: Use of 'mgl_join:' as an NSExpression function is forbidden.
NSPredicate: Use of 'mgl_attributed:' as an NSExpression function is forbidden.
NSPredicate: Use of 'mgl_coalesce:' as an NSExpression function is forbidden.
ameridagenasys commented 7 months ago

I've got this error as well:

NSPredicate: Use of 'MGL_FUNCTION' as an NSExpression function is forbidden

using iPhone 15 pro simulator. Any solution?

louwers commented 7 months ago

Thanks, I think Apple 'nerfed' NSExpressions, and that is why we are seeing these problems.

I will look into this a bit, but we may have to design another API and drop the NSExpression wrappers altogether (see https://github.com/maplibre/maplibre-native/issues/1820).

ianthetechie commented 7 months ago

Thanks, I think Apple 'nerfed' NSExpressions, and that is why we are seeing these problems.

Ahh, that's an interesting idea... I can't find anything about that in docs, but it could be that I missed something / that docs are wrong (wouldn't be the first time).

Now that I look at this a bit deeper... I wonder if what actually happened is that something changed with the structure so that the selector fails to resolve (ex: the receiver)? There's a bunch of areas where something could go wrong between the #define-as-a-code-generation-mechanism stuff to "install" methods.

I will look into this a bit, but we may have to design another API and drop the NSExpression wrappers altogether (see https://github.com/maplibre/maplibre-native/issues/1820).

I like the idea of designing a new API, though that doesn't necessarily mean we should do it ;) There are some tradeoffs to weight:

Good:

Bad:

I guess on the plus side though, it's hard to make something less friendly than NSExpressions. So if we can improve incrementally on UX or keep it the same and rid ourselves of the rat's nest of complexity, I think that'd be a win.

louwers commented 5 months ago

This is probably why:

"See No Eval: Runtime Dynamic Code Execution in Objective-C"

It’s just like the eval() function of Objective-C.

https://codecolor.ist/2021/01/16/see-no-eval-runtime-code-execution-objc/

louwers commented 5 months ago
image

Inclined to close this, it cannot be made to work in the same way without bypassing Apple's security measures, which would cause App Store denials.

ianthetechie commented 5 months ago

Yeah, it's been broken for quite some time now. I think we have to find a new approach at this point.

louwers commented 5 months ago

We now have some examples in the documentation that show how you can achieve the same result as these patched NSExpression additions.

https://maplibre.org/maplibre-native/ios/latest/documentation/maplibre-native-for-ios/animatedlineexample

https://maplibre.org/maplibre-native/ios/latest/documentation/maplibre-native-for-ios/linestylelayerexample

I'll make a PR to remove the aftermarket NSExpression installer as it's broken since iOS 15.

louwers commented 5 months ago

The JSON syntax can also be used:

        let stops = [
            0: UIColor(red: 1.00, green: 0.72, blue: 0.85, alpha: 1.0),
            2: UIColor(red: 0.69, green: 0.48, blue: 0.73, alpha: 1.0),
            4: UIColor(red: 0.61, green: 0.31, blue: 0.47, alpha: 1.0),
            7: UIColor(red: 0.43, green: 0.20, blue: 0.38, alpha: 1.0),
           16: UIColor(red: 0.33, green: 0.17, blue: 0.25, alpha: 1.0)
        ]

        // Style the circle layer color based on the above stops dictionary.
        layer.circleColor = NSExpression(format: "mgl_step:from:stops:(AGE, %@, %@)", UIColor(red: 1.0, green: 0.72, blue: 0.85, alpha: 1.0), stops)

E.g. would become something like

        layer.circleColor = NSExpression(mglJSONObject: ["step", ["get", "rank"], 0, "red", 10, "green", 20, "blue", 30, "purple", 40, "yellow"])
andresinaka commented 5 months ago

@louwers I'm using the NSExpression(mglJSONObject: and I'm still getting warnings:

NSExpression(mglJSONObject: [
        "match",
        ["get", "style"],
        "White",
        "#fbb03b",
        "#223b53",
])

I was using the following before:

NSExpression(
        forMLNMatchingKey: NSExpression(format: "CAST(style, 'NSString')"),
        in: [NSExpression(forConstantValue: "White"): NSExpression(forConstantValue: "#fbb23b")],
        default: NSExpression(forConstantValue: "#223b53")
)

Both ways are giving me:

NSPredicate: Use of 'MLN_FUNCTION' as an NSExpression function is forbidden.

I see the NSExpression(forMLNMatchingKey: is being used in this test:

https://github.com/maplibre/maplibre-native/blob/61c3b280ba9d69bd06829e9a49aff6dc6c95c431/platform/darwin/test/MLNDocumentationGuideTests.swift#L270-L272

ianthetechie commented 4 months ago

Same.

My application is related to switching icon images based on a match expression that looks at an attribute, similar to what @andresinaka was discussing above.

Here is an example of a trivial match in JSON format. The gibberish are the sha256 of the image, but that's not really relevant here.

let object = [
  "match",
  [
    "get",
    "icon"
  ],
  "club", "4cf90c987a43fb76edc2647549fc0d6ab16c6068f0f3b6ed72689c4e48da417f",
  "missing", "ee88ac0ee4c0f5f56c9ab373e6abf6ce405e6a18c14511129d64ff6b9bea5655",
  "7a227a00e4a3d609f8202efa2bf4ddd61904f1f95c4350c59809dd2c817850c2"
]

let expression = NSExpression(mglJSONObject: object)

This will crash with the following stack trace on iOS 17.4 and MapLibre Native 5.6.0 (via SPM).

Last Exception Backtrace:
0   CoreFoundation                         0x1804ae0ec __exceptionPreprocess + 160
1   libobjc.A.dylib                        0x180087db4 objc_exception_throw + 56
2   Foundation                             0x180d17704 +[NSExpression expressionForFunction:arguments:] + 364
3   MapLibre                               0x1159ff9e0 0x1159a8000 + 358880
4   ...BF649B70C4C6_PackageProduct         0x114a63e2c @nonobjc NSExpression.init(mglJSONObject:) + 132
5   ...BF649B70C4C6_PackageProduct         0x114a63b74 closure #1 in SymbolStyleLayer.iconImage(attribute:mappings:default:) + 884 (Symbol.swift:79)
6   ...BF649B70C4C6_PackageProduct         0x114a47258 modified<A>(_:using:) + 268 (Utilities.swift:12)
7   ...BF649B70C4C6_PackageProduct         0x114a637d0 SymbolStyleLayer.iconImage(attribute:mappings:default:) + 308 (Symbol.swift:60)

The test that you mention above @andresinaka is, as you can see one line before the one you linked, expected to fail.

There's a 10 month old Slack thread here: https://osmus.slack.com/archives/C04K0NBUEEM/p1694414107867269, and I'll try to reproduce the salient parts.

@louwers suggested to try using mgl_jsonMatchExpressionObject directly, but it crashes as already noted above.

My frustration at this point is that we seem to have no working implementation of match expressions in particular, but this may apply elsewhere. Every test case I can find for these, either in Obj-C++ or Swift, are simply marked as broken on iOS.

https://github.com/maplibre/maplibre-native/blob/1a3aff08deb5d00f5446fb632e8e78bb8bed1933/platform/darwin/test/MLNExpressionTests.mm#L1329

I have working examples for interpolation and others, but match in particular is broken.

louwers commented 4 months ago

@ianthetechie Can you share the entire snippet that crashes? In this example I tried this:

        layer.circleColor = NSExpression(mglJSONObject: [
                "match",
                ["get", "class"],
                "shop",
                "#FFA500",
                "#223b53",
        ])

And it works.

andresinaka commented 4 months ago

I can confirm match is working fine for me on iOS 17.4 with MapLibre Native 5.6.0.

ianthetechie can you try with the following:

let object: [Any] = [
      "match", ["get", "icon"],
       "club", ["image","4cf90c987a43fb76edc2647549fc0d6ab16c6068f0f3b6ed72689c4e48da417f"],
       "missing", ["image", "ee88ac0ee4c0f5f56c9ab373e6abf6ce405e6a18c14511129d64ff6b9bea5655"],
        ["image", "7a227a00e4a3d609f8202efa2bf4ddd61904f1f95c4350c59809dd2c817850c2"]
]

If it works it's because you need to define the images as images, not strings: https://maplibre.org/maplibre-style-spec/expressions/#image

Remember to also add the images you want to show in the map to your map using:

self?.mapView?.style?.setImage(image, forName: "imageName")

Let me know how it goes!

ianthetechie commented 4 months ago

🀯 oooooooooh! Wow... That's totally my mistake in that case. I'll give that a shot this evening and report back, but I bet that's the problem with mine.

louwers commented 4 months ago

I think iconImageName should actually evaluate to a string expression...

I tested it, and both seem to work.

sargunv commented 1 week ago

I’m also seeing these warnings:

NSPredicate: Use of 'mgl_does:have:' as an NSExpression function is forbidden.
NSPredicate: Use of 'mgl_join:' as an NSExpression function is forbidden.
NSPredicate: Use of 'mgl_coalesce:' as an NSExpression function is forbidden.

I’m instantiating expressions with NSExpression(mlnJSONObject:) as suggested in this thread, and the expression is working, but I get these warnings logged.

This is specifically on a MLNSymbolStyleLayer.text; I’ve seen this warning on no other layer properties.

And I don't have the warning when loading the a JSON style URL, just when programmatically constructing the expression.