Skip to content

Revert " Remove extensions on ImplicitlyUnwrappedOptional from the stdlib." #13967

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 17, 2018
Merged

Revert " Remove extensions on ImplicitlyUnwrappedOptional from the stdlib." #13967

merged 3 commits into from
Jan 17, 2018

Conversation

rudkx
Copy link
Contributor

@rudkx rudkx commented Jan 16, 2018 •
edited
Loading

Partially reverts #12713

It looks like this probably broke conditional casts from IUOs to NSObject.

@rudkx
Copy link
Contributor Author

rudkx commented Jan 16, 2018

@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Jan 16, 2018 •
edited
Loading

@DougGregor Any ideas about this build failure?

I hit it locally as well and changing the result type to ImplicitlyUnwrappedOptional<Wrapped> didn't seem to help.

Perhaps an associated type inference or conformance checking problem?

@rudkx
Copy link
Contributor Author

rudkx commented Jan 16, 2018

@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Jan 16, 2018

I'm working on new test cases for ImplicitlyUnwrappedOptional<T> and Optional<T> bridging and will push them here and re-run before merging.

@rudkx
Copy link
Contributor Author

rudkx commented Jan 16, 2018

I've also opened a PR for 4.1: #13976

The difference is that the PR for master also has to back out a change to conformance checking which broke reintroducing these extensions.

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

timeout on linux

@rudkx rudkx force-pushed the revert-12713-remove-extensions-of-IUO branch from c84853b to 48108a7 Compare January 17, 2018 00:41
@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

@swift-ci Please smoke test Linux platform

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

@shahmishal The linux machine here appears to have an old version of CMake so the configure failed.

@shahmishal
Copy link
Member

@rudkx Looking into it.

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

@swift-ci Please test OS X platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - f3bc520

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

@swift-ci Please test source compatibility

rudkx and others added 3 commits January 16, 2018 23:12
@rudkx
Restore ImplicitlyUnwrappedOptional extension conforming to _Objectiv…
be6d83c 
…eCBridgeable.

Partial revert of 26f6a75.

Removing this breaks bridging these values to Objective C. Once IUOs
are removed from the type system and ImplicitlyUnwrappedOptional<T> is
removed from the library, we'll be strictly using Optional<T> which
has this conformance as well.

Fixes: rdar://problem/36477954, https://bugs.swift.org/browse/SR-6764
@rudkx
Revert "IUO: Update protocol conformance checking to check IUO attr o…
c773493 
…n decl."

This reverts commit ec5cebd.

The way it's implemented is problematic when we still have IUOs in the
type system and library.

We end up thinking there are mismatches between requirements and
potential witnesses when there are not.
@rudkx
Add runtime tests for bridging optionals and IUOs.
41056ae
@rudkx rudkx force-pushed the revert-12713-remove-extensions-of-IUO branch from 48108a7 to 41056ae Compare January 17, 2018 07:13
@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

@swift-ci Please test source compatibility

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

Another Linux timeout

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

@swift-ci Please smoke test Linux

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

@swift-ci Please test source compatibility

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

@swift-ci Please smoke test Linux platform

@rudkx
Copy link
Contributor Author

rudkx commented Jan 17, 2018

There was another timeout on linux. I've seen this several times now across a few different PR.

DougGregor
DougGregor approved these changes Jan 17, 2018
View reviewed changes
Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@rudkx rudkx merged commit 5339a14 into master Jan 17, 2018
@rudkx rudkx deleted the revert-12713-remove-extensions-of-IUO branch January 17, 2018 19:22
@rudkx rudkx mentioned this pull request Jan 16, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DougGregor DougGregor DougGregor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rudkx @shahmishal @swift-ci @DougGregor