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-14943] Fix-it for changing unsafeBitcast to unsafeDowncast should be more narrow or removed #57285

Open
typesanitizer opened this issue Jul 20, 2021 · 5 comments

Comments

@typesanitizer
Copy link

@typesanitizer typesanitizer commented Jul 20, 2021

Previous ID SR-14943
Radar rdar://problem/80820402
Original Reporter @typesanitizer
Type Bug
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, DiagnosticsQoI, StarterBug
Assignee Rajagopalan (JIRA)
Priority Medium

md5: 3d91228cea2823e2a2396d782c8b9edb

Issue Description:

According to the documentation, unsafeBitcast expects that the target type is layout-compatible with the type of the value passed in.

OTOH, unsafeDowncast expects that the type of the value is the target type (there is a _debugPrecondition(x is T)).

Currently, we have a fix-it where we recommend replacing unsafeBitcast with unsafeDowncast. However, strengthening a precondition like that in a fix-it is generally not OK unless we have some reason to believe that x is T will be true.

Here is a code example that can be tested in a playground:

import Foundation
import UIKit@propertyWrapper
struct UnsafeBitCast<Actual: NSObjectProtocol, Mocked: NSObjectProtocol> {
    init(wrappedValue: Actual?, to mocked: Mocked.Type) {
        self.wrappedValue = wrappedValue
    }
    var wrappedValue: Actual?
    var projectedValue: Mocked? {
        wrappedValue.map({ unsafeBitCast($0, to: Mocked.self) }) // fix-it suggests unsafeDowncast
    }
}
​
class MockUIWindow: NSObject {
    @UnsafeBitCast(to: UIScene.self) var mockWindowScene: MockUIScene? = nil
    init(mockWindowScene: MockUIScene? = .init()) {
        self.mockWindowScene = mockWindowScene
    }
    var windowScene: UIScene? {
        $mockWindowScene
    }
}
​
class MockUIScene: NSObject {
    @objc var activationState: UIScene.ActivationState
    init(activationState: UIScene.ActivationState = .foregroundActive) {
        self.activationState = activationState
    }
}
​
let mockWindow = MockUIWindow()
print(mockWindow.windowScene!.activationState == .foregroundActive)

If one applies the fix-it, this code crashes due to the precondition failing. We should either remove the fix-it altogether or only apply it narrowly in the cases where we know for sure that x is T will be true.

@typesanitizer
Copy link
Author

@typesanitizer typesanitizer commented Jul 20, 2021

@swift-ci create

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Sep 19, 2021

Comment by Rajagopalan Gangadharan (JIRA)

Hello theindigamer (JIRA User) Can I work on this if it's still open?

@typesanitizer
Copy link
Author

@typesanitizer typesanitizer commented Sep 20, 2021

Sure, go for it.

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Dec 11, 2021

Comment by Rajagopalan Gangadharan (JIRA)

Hello @typesanitizer sorry for the late PR but here it is - #40523 .

@swift-ci
Copy link
Collaborator

@swift-ci swift-ci commented Jan 4, 2022

Comment by Rajagopalan Gangadharan (JIRA)

@typesanitizer I am facing a problem which I don’t know how to resolve - https://forums.swift.org/t/swift-ios-build-error-due-to-sdk-versions/54373/2

can you help me out please?

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants