Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Predicate expression with optional key path throws error #476

Open
axelandersson opened this issue Mar 12, 2024 · 9 comments
Open

Predicate expression with optional key path throws error #476

axelandersson opened this issue Mar 12, 2024 · 9 comments

Comments

@axelandersson
Copy link

Hello!

I'm using PredicateExpressions to build dynamically build predicates. I'm getting an exception when creating a PredicateExpressions.KeyPath with a keypath like \Dummy.optionalSub?.optionalString where both optionalSub and optionalString are optional values. The exception is:

Predicate does not support keypaths with multiple components

This message is a bit misleading as a predicate with a keypath like \Dummy.sub.optionalString, where only optionalString is optional, can be created without issue.

If optionalSub is made non-optional, the expression can be created.

Am I not creating the expressions correctly, or am I running into an issue here?

I've created a sample project showing the issue here: https://github.com/ordo-one/external-reproducers/tree/main/swift/predicate-optional-keypath.

@jmschonfeld
Copy link
Contributor

This message is a bit misleading as a predicate with a keypath like \Dummy.sub.optionalString, where only optionalString is optional, can be created without issue.

I think this behavior that you're seeing is actually unintentional. I'll take a look at the sample project, but this construction should also produce the same fatal error. We likely have a bug in the check that we do for valid key paths that incorrectly lets this one through - thanks for catching that!

Am I not creating the expressions correctly, or am I running into an issue here?

PredicateExpressions.KeyPath is designed to only support a single component of a keypath. This enables introspection of each component of the keypath while walking the predicate expression tree (since KeyPath itself isn't introspectable). The #Predicate macro takes care of splitting keypaths into individual components (if you have a predicate that looks like #Predicate { $0.foo.bar} you'll see that there are two build_KeyPath calls, one for \.foo and one for \.bar) so you'll want to take care to construct predicates dynamically in the same manner. In your case you'll want to create one keypath operator for the \.optionalString keypath and one operator for the \.optionalSub keypath.

As for the optional handling within a keypath, the #Predicate macro handles this by using the PredicateExpressions.OptionalFlatMap operator. You can do the same thing by dynamically constructing a flat map operator to represent the optional chaining that is part of your keypath. The OptionalFlatMap operator will allow you to provide one input that produces an optional value (the \.optionalSub keypath operator) and a closure that provides an operator to execute if the value was non-nil (the \.optionalString keypath operator).

@axelandersson
Copy link
Author

Thanks @jmschonfeld.

I'm able to use PredicateExpressions.OptionalFlatMap as you recommend—as long as I know the type of the key paths.

In my project the key paths are provided in runtime and are partially type erased. This has worked fine when dealing with a generic root type and some simple properties like Ints, because I can cast the type erased key paths from PartialKeyPath<RootType> to KeyPath<RootType, Int>, etc. But when the key paths are a chain KeyPath<RootType, LeafType?> & KeyPath<LeafType, Int>, I run into issues.

I created a sample project here: https://github.com/ordo-one/external-reproducers/blob/main/swift/predicate-optional-keypath2/Sources/PredicateKeyPathProviding.swift.

The idea is that the user of my API would provide key paths through a protocol, that I can then use to create predicates. Since the key paths can be of varying types, they are type erased with PartialKeyPath and AnyKeyPath when returned from the protocol method.

I suppose this is more of a question about key paths than it is about predicates, but any advice would be much appreciated, thanks!

@jmschonfeld
Copy link
Contributor

You're definitely on the right track! Two things from your sample code to take a look at:

  1. On Line 65: The type of the parameter ofType is LeafType, but you're providing Root.Leaf.self. This ends up causing LeafType to be Root.Leaf.Type.Type. Instead, you likely want ofType: LeafType.Type so that LeafType represents the Metatype of the leaf, and not the Metatype of the Metatype of the leaf. Making this change causes the first cast to succeed.
  2. On Line 73 this cast fails because the keypath is a KeyPath<Leaf, Int?> but you're casting it to a KeyPath<Leaf, Int>. Changing Int to Int? on this line causes the cast to succeed. The remainder of the function compiles because when you create a Value(1) the type of 1 is inferred to be Int? instead of Int (based on the return type of PredicateExpressions.KeyPath and the same type constraint imposed by PredicateExpressions.Equal.

With those two changes, your sample code code executes successfully. Does that help?

@axelandersson
Copy link
Author

Thanks @jmschonfeld. That definitely helps!

I've updated the sample project at https://github.com/ordo-one/external-reproducers/blob/main/swift/predicate-optional-keypath2/Sources/PredicateKeyPathProviding.swift so it now compiles.

However, my original goal of doing this while agnostic of the types remain. In the updated sample, both workingType and failingType resolve to the same Leaf in runtime, but failingType is type erased so I can't pass it to createPredicate2(). I assume I need to keep the type information somehow, but not sure how to go about it. Any ideas? Thanks!

@jmschonfeld
Copy link
Contributor

In your case, this is a scenario where you're trying to unwrap an existential and pass it to a generic function. SE-0352 lifted some restrictions here and current allows doing this for constrained generic parameters, but (I believe) due to source compatibility concerns limits the behavior for non-constrained generic parameters (like your LeafType parameter) to Swift 6 mode. I tried to do this myself with Swift 6 mode enabled, but for some reason I wasn't able to get it to work. @DougGregor do you know if the SE-0352 capabilities for implicitly opening non-constrained parameters has been enabled yet in the Swift 6 snapshots? I think once that feature is enabled (to replace the current underscored _openExistential functionality) you'll be able to do what you're hoping for here

@axelandersson
Copy link
Author

Thanks @jmschonfeld—I will try to test this in Swift 6.

@axelandersson
Copy link
Author

@jmschonfeld I've now tested this with the Swift 6 with ImplicitOpenExistentials turned on, and can happily report that it works there.

I updated my example at https://github.com/ordo-one/external-reproducers/blob/main/swift/predicate-optional-keypath2/Sources/PredicateKeyPathProviding.swift. The code inside #if hasFeature(ImplicitOpenExistentials) now works as expected.

@vanvoorden
Copy link

#if hasFeature(ImplicitOpenExistentials)
        let predicate3 = factory.createPredicate2(rootType: Root.self, leafType: failingType)

        print("predicate3=\(predicate3)")
#else
      func body<Value>(_: Value.Type) -> Value.Type { Value.self }
      let predicate3 = factory.createPredicate2(
        rootType: Root.self,
        leafType: _openExistential(
          failingType,
          do: body
        )
      )
      
      print("predicate3=\(predicate3)")
#endif

@axelandersson AFAIK _openExistential (referenced by @jmschonfeld) is the legit workaround to compile this on 5.10… but there might be another solution I do not know about (other than waiting for 6.0 to ship).

@axelandersson
Copy link
Author

@vanvoorden Thanks! For future reference I have updated the example at https://github.com/ordo-one/external-reproducers/blob/main/swift/predicate-optional-keypath2/Sources/PredicateKeyPathProviding.swift so it now works both with ImplicitOpenExistentials and _openExistential.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants