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

Comparison predicate cannot refer to ComparisonResult enum #318

Open
axelandersson opened this issue Nov 15, 2023 · 3 comments
Open

Comparison predicate cannot refer to ComparisonResult enum #318

axelandersson opened this issue Nov 15, 2023 · 3 comments
Assignees

Comments

@axelandersson
Copy link

Given the following predicate:

struct Dummy {
    let stringValue: String
}

let predicate = #Predicate<Dummy> {
    $0.stringValue.caseInsensitiveCompare("a") == .orderedSame
}

Compiler complains that: Member access without an explicit base is not allowed in this predicate.

So let's change it to an explicit base:

let predicate = #Predicate<Dummy> {
    $0.stringValue.caseInsensitiveCompare("a") == ComparisonResult.orderedSame
}

Compiler then complains that Key path cannot refer to enum case 'orderedSame'.

Macro expansion shows that it's now trying to build a key path expression instead of using the enum value:

Foundation.Predicate<Dummy>({
    PredicateExpressions.build_Equal(
        lhs: PredicateExpressions.build_caseInsensitiveCompare(
            PredicateExpressions.build_KeyPath(
                root: PredicateExpressions.build_Arg($0),
                keyPath: \.stringValue
            ),
            PredicateExpressions.build_Arg("a")
        ),
        rhs: PredicateExpressions.build_KeyPath(
            root: PredicateExpressions.build_Arg(ComparisonResult),
            keyPath: \.orderedSame
        )
    )
})

A workaround is to put the enum in a variable:

let orderedSame = ComparisonResult.orderedSame
let predicate = #Predicate<Dummy> {
    $0.stringValue.caseInsensitiveCompare("a") == orderedSame
}

Using Swift from Xcode 15.0 (15A240d).

@jmschonfeld
Copy link
Contributor

Thanks for bringing this up! This is a limitation of Predicates currently. Predicate uses KeyPaths to reference properties, but as it stands today Swift does not support KeyPaths for static properties or enum cases. Unfortunately this means that static properties and enum cases cannot be referenced directly in a predicate and the workaround is to do as you have done - create a variable that references the static property or enum case outside of the predicate and then capture it within the closure.

We've also thought about whether we could strictly allow trivial enum cases as a special case within the predicate since their value can't change between predicate evaluations unlike a static property. However, since macros only have syntactic information and not type information, there's unfortunately no way for us to differentiate between a trivial enum case and a static property. So in order to avoid unexpected behavior of static properties (for example, something like Date.now which can change value between predicate invocations) we chose to disallow this set of capabilities within predicates.

Lastly we also looked at whether we can improve the diagnostic here and/or even make a fix it to do this capturing for you. However, again since the macro doesn't have type information, given something like ComparisonResult.orderedSame the macro cannot determine if ComparisonResult is a type with a static property/enum case, or a valid variable named ComparisonResult with an instance member. So for that reason, we're also left with the existing error message you mentioned.

If you have any suggestions, I'm definitely open to ideas if you think of anything clever we can do here! But in the meantime, I think improving this scenario requires changes to the language to provide type information to macros and/or allow keypaths to static properties/enum cases.

@axelandersson
Copy link
Author

@jmschonfeld Thanks for the explanation. The only suggestion I can think of is that since the most common comparison will be checking if the strings compare equal, StringProtocol could provide a isCaseInsensitiveEqual() -> Bool convenience function to avoid having to use the enum at all, but it feels a bit drastic to make this issue drive a workaround API change to something as fundamental as StringProtocol.

@jmschonfeld
Copy link
Contributor

Yeah, I think I'd agree with you that adding is...Equal() functions for all of the possible combinations we support might be a bit drastic given StringProtocol is pretty highly used/fundamental. Technically, we could have the macro special-case each of the ComparisonResult cases to allow them, but I also don't quite like that idea because I think it'd be a bit too jarring when that only works for that enum but doesn't work for any other trivial enum cases, especially user-defined ones

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

2 participants