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

[SR-8704] Cast from Any? to as? Value where Value == Any produces unexpected result in Xcode 10 #51216

Closed
lilyball mannequin opened this issue Sep 5, 2018 · 15 comments
Closed
Assignees
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself

Comments

@lilyball
Copy link
Mannequin

lilyball mannequin commented Sep 5, 2018

Previous ID SR-8704
Radar rdar://problem/40916953
Original Reporter @lilyball
Type Bug
Status Closed
Resolution Done
Environment

Xcode 10.0 (10L221o)
Apple Swift version 4.2 (swiftlang-1000.0.32.1 clang-1000.10.39)
Target: x86_64-apple-darwin17.7.0

Additional Detail from JIRA
Votes 1
Component/s Compiler
Labels Bug
Assignee @hamishknight
Priority Medium

md5: 695e9648f10cd45d05e11cf6b2acc3b6

is duplicated by:

  • SR-8856 Optional downcast results in double optional during cast to generic value

relates to:

  • SR-7975 Fetching a nonexistent entry from a dictionary returns Optional when inferred type is Any
  • SR-4248 Runtime exception casting an Any? nil to a generic optional

Issue Description:

In a generic context, when casting from an Any? value to a generically-bound type using as? Value, if the generic type is equal to Any and the source value is nil, in Xcode 9 the result is nil but in Xcode 10 the result is .some(nil). This is true even when running with -swift-version 4.

Example:

func foo<Value>(_ x: Any?, as type: Value.Type) -> Value? {
    return x as? Value
}

dump(foo(nil, as: Any.self))

In Xcode 9.4.1 this prints

- nil

But in Xcode 10 beta 5 this prints

▿ Optional(nil)
  - some: nil
@belkadan
Copy link
Contributor

belkadan commented Sep 5, 2018

cc @rudkx

@rudkx
Copy link
Member

rudkx commented Sep 5, 2018

The behavior you're seeing is a result of the change for https://bugs.swift.org/browse/SR-4248.

The behavior now matches what you would get if you conditionally cast to the same type in a non-generic context.

It would have probably been best to put this under -swift-version 5, but apparently the implementor and reviewer did not catch the fact that it would change behavior in this way.

@hamishknight
Copy link
Collaborator

Ah yup, this one's my bad. I didn't realise the (now obvious with hindsight) compatibility implications. Would it be worth restoring the old behaviour for Swift 5 under Swift 4 compat mode?

Under the new behaviour, the compiler will assume that a generic placeholder could be a type with an arbitrary level of optionality, and so therefore won't eagerly unwrap the value before performing the cast – it will instead leave any unwrapping to the runtime. And because you're casting an Any? to Any, the runtime will happily box the value in Any and then promote that to Any? as the successful result of the conditional cast. As Mark says, this matches the behaviour of performing the cast in a non-generic context.

To get the desired behaviour for your function, you can either cast to the optional type and then coalesce nil:

func foo<Value>(_ x: Any?, as type: Value.Type) -> Value? {
  return (x as? Value?) ?? nil
}

Or unwrap x before performing the cast – for example, using flatMap:

func foo<Value>(_ x: Any?, as type: Value.Type) -> Value? {
  return x.flatMap { $0 as? Value }
}

@lilyball
Copy link
Mannequin Author

lilyball mannequin commented Sep 6, 2018

Given that this affects Xcode 10 beta, which is Swift 4.2, not Swift 5, I would say the question should be whether old behavior should be restored in Swift 4 mode and new behavior used in Swift 4.2 mode (and I think the answer should be "yes" because this is a rather surprising behavior change to get just by upgrading Xcode 10 while keeping Swift 4 mode).

@hamishknight
Copy link
Collaborator

Unfortunately, I believe the ship has sailed for making such changes to the 4.2 compiler, so the most we can do is restore the behaviour for the Swift 5 compiler under Swift 4 compat mode.

@belkadan
Copy link
Contributor

belkadan commented Sep 6, 2018

It's a compatibility regression, so I'm not sure how Ted and co would feel about taking it for a 4.2.1 release.

@hamishknight
Copy link
Collaborator

@belkadan That would be great if we could! Would it be possible to also consider #18488 & #18952 for such a release?

@belkadan
Copy link
Contributor

belkadan commented Sep 6, 2018

It depends how risky you and your reviewers think the changes are. If you think it's worth doing, you can follow the same process as before to get changes into the 4.2 branch, and Ted will either approve or deny the change.

(See https://forums.swift.org/t/swift-4-2-in-final-convergence-swift-4-2-branch-open-for-post-4-2-0-changes/15128.)

@hamishknight
Copy link
Collaborator

Opened a PR to limit the new casting behaviour to Swift 5 mode: #19217

@swift-ci
Copy link
Collaborator

Comment by Evgeny Kaz (JIRA)

@hamishknight But what the philosophy of Optionals in swift says about it? Is this new behaviour right or not? Shall we rely on it?

@lilyball
Copy link
Mannequin Author

lilyball mannequin commented Sep 15, 2018

I think the new behavior is perfectly reasonable, it's just the fact that it's a potentially-breaking change that wasn't good.

@swift-ci
Copy link
Collaborator

Comment by Evgeny Kaz (JIRA)

Eridius (JIRA User) I feel like it should not be "reasonable" - it is kinda crucial for the language concept of optional and nil coalescing. It should be described very clearly in the documentation. Look an the next code:

import UIKit

protocol Factory {
    
    associatedtype ViewController: UIViewController
    
    associatedtype Context

    func build(with context: Context) -> ViewController

}

struct StringFactory: Factory {

    func build(with context: String?) -> UINavigationController {
        return UINavigationController(nibName: nil, bundle: nil)
    }

}

struct FactoryBox<F: Factory> {
    
    let factory: F
    
    init(_ factory: F) {
        self.factory = factory
    }
    
    func build(with context: Any?) -> UIViewController? {
        guard let typedContext = context as? F.Context else {
            print("Context is not supported")
            return nil
        }
        return factory.build(with: typedContext)
    }
}

StringFactory().build(with: nil) // Returns UINavigationController in both XCode 9 and 10
FactoryBox(StringFactory()).build(with: nil) // Returns nil in XCode 9 printing "Context is not supported", returns UINavigationController in XCode 10

As for me - the new behaviour is reasonable, but on other hand a construction like `guard let b = a as? Whatever else ...` becomes a bit confusing. What exactly is b?

@hamishknight
Copy link
Collaborator

Resolved by #19217 which preserves the old behaviour in Swift 5 under Swift 4/4.2 compat mode.

I have also opened #19562 to hopefully get this into 4.2.1.

@norio-nomura
Copy link
Contributor

I confirmed that the behavior described in this issue has been changed on Swift 5 with -swift-version 4.2

$ pbpaste
func foo<Value>(_ x: Any?, as type: Value.Type) -> Value? {
    return x as? Value
}

$ pbpaste|xcrun --toolchain org.swift.42320190228a swift -
▿ Optional(nil)
  - some: nil
$ pbpaste|xcrun --toolchain org.swift.5020190306a swift -
▿ Optional(nil)
  - some: nil
$ pbpaste|xcrun --toolchain org.swift.5020190306a swift -swift-version 4.2 -
- nil

But is it correct behavior that using `-swift-version 4.2` makes Swift 5 to incompatible with Swift 4.2?

@hamishknight
Copy link
Collaborator

@norio-nomura That is correct, albeit unfortunate. It's worth noting that there would be still be incompatibility between Swift 4.2 and Swift 5 with `-swift-version 4` even if we preserved the behavioural change in `-swift-version 4.2`.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself
Projects
None yet
Development

No branches or pull requests

5 participants