Skip to content

Revert "[SR-12033] [Sema] Do not allow inferring defaultable closure () -> $T for autoclosure arguments result" #36022

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

Merged
merged 1 commit into from
Feb 18, 2021

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Feb 17, 2021

Reverts #35503

@LucianoPAlmeida Unfortunately we have to revert this for now and make it more tailored since these changes caused a source compatibility regression in code like this:

protocol P {
  associatedtype F : Error
}

extension P {
  func test(error: F?) {
    accepts_autoclosure({
      if let failure = error {
        throw failure
      }
    })
  }

  func accepts_autoclosure<T>(_ expression: @autoclosure () throws -> T) {}
}

…`() -> $T` for autoclosure arguments result"
@xedin
Copy link
Contributor Author

xedin commented Feb 17, 2021

@swift-ci please smoke test

@xedin
Copy link
Contributor Author

xedin commented Feb 17, 2021

@swift-ci please smoke test Windows platform

@xedin
Copy link
Contributor Author

xedin commented Feb 17, 2021

@swift-ci please smoke test Linux platform

1 similar comment
@xedin
Copy link
Contributor Author

xedin commented Feb 17, 2021

@swift-ci please smoke test Linux platform

@xedin xedin merged commit 1efcd3f into main Feb 18, 2021
@xedin xedin deleted the revert-35503-SR-12033-autoclosure branch February 18, 2021 00:40
@LucianoPAlmeida
Copy link
Contributor

@LucianoPAlmeida Unfortunately we have to revert this for now and make it more tailored since these changes caused a source compatibility regression

hum, ok sorry for that and thank you for reverting :))
I'll try to take a look on that to see if we can make an adjustment

@xedin
Copy link
Contributor Author

xedin commented Feb 18, 2021

@LucianoPAlmeida No worries! The fix is correct since otherwise the behavior is not obvious to users it's just the source compatibility impact :/ Could you please open a PR to avoid transitive inference of DefaultClosureType removed which used to be a part of this one?

@LucianoPAlmeida
Copy link
Contributor

it's just the source compatibility impact :/

That's unfortunate... do you think its possible to fix this without source compatibility break?

Could you please open a PR to avoid transitive inference of DefaultClosureType removed which used to be a part of this one?

For sure :)

@xedin
Copy link
Contributor Author

xedin commented Feb 19, 2021

That's unfortunate... do you think its possible to fix this without source compatibility break?

I don't think so since there are projects which rely on this behavior. What we can do though is to rank solutions with @autcolosure lower than solutions where parameters accept regular closures e.g.

func test<T>(_: () -> T) {}
func test<T>(_: @autoclosure () -> T) {}

test { }

This example is ambiguous today but it shouldn't be.

@LucianoPAlmeida
Copy link
Contributor

What we can do though is to rank solutions with @autcolosure lower than solutions where parameters accept regular closures

That makes sense!
Do we have a JIRA ticket for that already? If not I can create it just to track this

@xedin
Copy link
Contributor Author

xedin commented Feb 19, 2021

We might, it might be worth a search before creating a new one.

@LucianoPAlmeida
Copy link
Contributor

We might, it might be worth a search before creating a new one.

Right, I'll look it up. Thanks!

@LucianoPAlmeida
Copy link
Contributor

Found two actually that seems to address the same issue SR-2705 and SR-6725

@xedin
Copy link
Contributor Author

xedin commented Feb 19, 2021

SR-2705 is what we want here.

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

Successfully merging this pull request may close these issues.

2 participants