Skip to content

Conversation

@jckarter
Copy link
Contributor

@jckarter jckarter commented Apr 6, 2016

Not the superclass's. This can cause mapping types out of context to not
resolve archetypes, because the DeclContexts won't match, e.g:

internal class _IteratorBox<Base: IteratorProtocol> :
_AnyIteratorBoxBase<Base.Element> { internal override func next() ->
Base.Element? }

Here, _IteratorBox's associated type Element for IteratorProtocol is
Base.Element, but the conformance comes from the superclass,
_AnyIteratorBoxBase. This meant that Base.Element couldn't be mapped to
an interface type, which should be:

(dependent-member
(generic-type-parameter depth=0 index=0) member=Element)

The old behavior also broke inherited conformances across serialization boundaries, because subclasses would fail to inherit conformances a deserialized class inherited itself from a base class (SR-1158).

@jckarter
Copy link
Contributor Author

jckarter commented Apr 6, 2016

@DougGregor Does this look reasonable for 2.2? There's something more subtle going on that I don't fully understand, and wouldn't want to mess with in the scope of 2.2. @bitjammer made this change to master as b0d6341 for other reasons, and it also gets deserialization working in the case of SR-1158.

@jckarter
Copy link
Contributor Author

jckarter commented Apr 6, 2016

@swift-ci Please test

@jckarter
Copy link
Contributor Author

jckarter commented Apr 6, 2016

@tkremenek Can we take this for 2.2 (contingent on tests passing and @DougGregor's approval)?

@jckarter jckarter added this to the Swift 2.2.x milestone Apr 6, 2016
@tkremenek
Copy link
Member

@jckarter Yes we can take it, assuming review and testing passes.

@tkremenek tkremenek assigned DougGregor and unassigned tkremenek Apr 6, 2016
@tkremenek
Copy link
Member

Test fails:

Testing: 0 .. 10.. 20
FAIL: Swift :: SILOptimizer/specialize_inherited.sil (1876 of 7874)
******************** TEST 'Swift :: SILOptimizer/specialize_inherited.sil' FAILED ********************
Script:
--
/home/buildnode/jenkins/workspace/swift-PR-Linux@2/buildbot_linux/swift-linux-x86_64/bin/sil-opt -target x86_64-unknown-linux-gnu  -module-cache-path '/tmp/swift-testsuite-clang-module-cacheH7m5_2' -enable-sil-verify-all /home/buildnode/jenkins/workspace/swift-PR-Linux@2/swift/test/SILOptimizer/specialize_inherited.sil -generic-specializer -module-name inherit | FileCheck /home/buildnode/jenkins/workspace/swift-PR-Linux@2/swift/test/SILOptimizer/specialize_inherited.sil
--
Exit Code: 1

Command Output (stderr):
--
/home/buildnode/jenkins/workspace/swift-PR-Linux@2/swift/test/SILOptimizer/specialize_inherited.sil:39:17: error: expected string not found in input
// CHECK-LABEL: @_TTSg5C7inherit8MMStringCS_8MMObjects8HashableS__GSQCS_6MMFont___callee : $@convention(method) (@in MMString, Int, @owned MMStorage<MMString, ImplicitlyUnwrappedOptional<MMFont>>) -> Bool {
                ^
<stdin>:31:1: note: scanning from here
// %0 // user: %4
^
<stdin>:47:23: note: possible intended match here
sil shared [noinline] @_TTSg5C7inherit8MMStringS0_s8HashableS__GSQCS_6MMFont___callee : $@convention(method) (@in MMString, Int, @owned MMStorage<MMString, ImplicitlyUnwrappedOptional<MMFont>>) -> Bool {
                      ^

--

@tkremenek
Copy link
Member

@jckarter Is this just a test case that needs to be updated, or something more?

@tkremenek tkremenek assigned jckarter and unassigned DougGregor Apr 6, 2016
@jckarter
Copy link
Contributor Author

jckarter commented Apr 6, 2016

@tkremenek Yeah, the test case can just be updated. I'll take care of it and start the tests again.

@tkremenek
Copy link
Member

FYI, @DougGregor signed off offline on this change.

Not the superclass's. This can cause mapping types out of context to not
resolve archetypes, because the DeclContexts won't match, e.g:

internal class _IteratorBox<Base: IteratorProtocol> :
_AnyIteratorBoxBase<Base.Element> { internal override func next() ->
Base.Element?  }

Here, _IteratorBox's associated type Element for IteratorProtocol is
Base.Element, but the conformance comes from the superclass,
_AnyIteratorBoxBase. This meant that Base.Element couldn't be mapped to
an interface type, which should be:

(dependent-member
  (generic-type-parameter depth=0 index=0) member=Element)

The old behavior also broke inherited conformances across serialization boundaries, because subclasses would fail to inherit conformances a deserialized class inherited itself from a base class (SR-1158).
@jckarter jckarter force-pushed the protocol-external-grandchild branch from 5afeddc to e6d8787 Compare April 6, 2016 01:32
@jckarter
Copy link
Contributor Author

jckarter commented Apr 6, 2016

@swift-ci Please test

@tkremenek tkremenek merged commit b0867f0 into swiftlang:swift-2.2-branch Apr 6, 2016
@jckarter jckarter deleted the protocol-external-grandchild branch April 6, 2016 02:08
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.

3 participants