[CSApply] Don't attempt covariant result type erasure when parameters use dynamic Self.

[hborla]

Otherwise, the following code crashes, because buildMemberRef attempts covariant result type erasure of Final.useSelf but there is no Self in the result type:

final class Final {
  static func useSelf(_ body: (Self) -> ()) {}
}
func testNoErasure(_ body: (Final) -> ()) {
  return Final.useSelf(body)
}

This code used to build successfully in Swift 5.5, and started crashing in Swift 5.6. This change adjusts the condition for performing covariant result type erasure to only the case where the member is a FuncDecl with a use of dynamic Self in the result type. Member references to VarDecls are handled before this case, and I think buildMemberRef only needs to do covariant result type erasure for FuncDecls after that point.

Resolves: rdar://89099887

[hborla]

@swift-ci please smoke test

[hborla]

@swift-ci please test source compatibility

[slavapestov]

Yeah, we need to remove replaceCovariantResultType() entirely at some point. The existing logic won't work if you return something like (Self, Self) either. But this is OK for now. Can you also add a SILGen test to test/SILGen/dynamic_self.swift just in case?

[hborla]

The existing logic won't work if you return something like (Self, Self) either.

I thought about this too, but we're saved by the restriction that Self can't be nested other than inside an optional:

error: covariant 'Self' or 'Self?' can only appear at the top level of method result type
  static func useSelf() -> (Self, Self) { fatalError() }
              ^

Can you also add a SILGen test to test/SILGen/dynamic_self.swift just in case?

Will do 👍

[slavapestov]

I thought about this too, but we're saved by the restriction that Self can't be nested other than inside an optional:

Right, I meant one day we'll want to lift the restriction though, and at that point we'll need to rethink this part of CSApply. I know @AnthonyLatsis has spent some time looking into issues with dynamic Self before, maybe he's interested in taking this on.

[hborla]

@swift-ci please smoke test

[AnthonyLatsis]

I know @AnthonyLatsis has spent some time looking into issues with dynamic Self before, maybe he's interested in taking this on.

There's so much stuff like this that I would like to take on that I keep forgetting the older ideas (heh), but yes, this has been on my radar for quite a while.

[AnthonyLatsis]

I thought about this too, but we're saved by the restriction that Self can't be nested other than inside an optional

Not exactly:

class A {
  static func useSelf(_: () -> Self) {} // OK
}

Would still error out or crash later on if you use the method though.