Enable dynamic replacement with library evolution

[slavapestov]

Fixes rdar://problem/58457716.

[slavapestov]

@aschwaighofer This was the only actual change I had to make. Are you aware of anything that I might have missed?

[slavapestov]

@swift-ci Please test

[aschwaighofer]

The way we choose which accessors get marked dynamically replaceable is not right. JohnM and Jrose explained to me that we should use the opaque accessors instead of whatever I choose to replace.

[slavapestov]

@aschwaighofer Do you have an example that we miscompile today?

@rjmccall Do you remember what the issue was?

[aschwaighofer]

I don't have an example. But the accessors that are printed in the swiftinterface file need to be the ones that are marked dynamic, I think.

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - c0ad045d8ce9cfac4e6f0ca725cc757fbcd93eb9

[aschwaighofer]

Here is the comment from the commit 1980c4a that disabled dynamic in library evolution mode:

Disallow `dynamic` on native Swift declarations under library-evolution mode
    
    The current ABI of dynamic is not what we want for certain declarations.
    
    e.g
    
    dynamic var x : Int {
      willset {
      }
    }
    
    Is exposed in the .swiftinterface file as
    
    dynamic var x : Int {
      get {}
      set {}
    }
    
    However, the actual implementation is marking the willSet only dynamic.
    
    var x : Int {
      dynamic willSet() {}
      get {}
      set {}
    }
    
    To be able to rely on `dynamic` in the future we disable dynamic (for
    non-@objc) under library-evolution for now until we have made sure that
    the ABI is proper.
    
    rdar://51678074

[aschwaighofer]

I am not sure that we actually print all opaque accessors in the swift interface file.
I feel like _modify accessor is also part of the set of opaque accessors?

[slavapestov]

@aschwaighofer I don't understand why, though. There's no resilience boundary between the instrumented library and the dynamic replacement stub.

[slavapestov]

@swift-ci Please smoke test

[slavapestov]

I'm going to merge this PR since it fixes the class method linkage issue. I'll keep poking at the accessor thing to try to come up with an example where the current scheme fails.

[aschwaighofer]

I don't understand why, though. There's no resilience boundary between the instrumented library and the dynamic replacement stub.

This might be the case how it is being used today. But dynamic is a language feature like any other. So a resilient dynamic foo should be resilient? It should be possible to resiliently replace it.
I can't think of a reason why it can't be so.
You might be limited in what you do in the replacement but it should be possible?

If we ship the compiler with 'invalid' interface files we can't rely on them without some sort of versioning in the future (or so was the argument brought up in the radar).

I am not 100% sure anymore but the example above I think also is a distillation of a problem I ran into at some point. The mismatch was causing a problem somehow: I can see this causing problems when the compilation of the replacement would use the .swiftinterface file. It would try to replace the wrong entry points.

[aschwaighofer]

It might be worth running the resilience tests with -enable-implicit-dynamic enabled and to see that does not cause issues.

I ran the all tests like this at one point (hack the compiler to have enable-implicit-dynamic enabled is probably the easiest way to do this).

[aschwaighofer]

The mismatch was causing a problem somehow: I can see this causing problems when the compilation of the replacement would use the .swiftinterface file. It would try to replace the wrong entry points.

This might no longer reproduce today because this could have only reproduced when we were (erroneously) preferring .swiftinterface over .swiftmodule files?

[compnerd]

@slavapestov - this seems to have caused a regression: https://ci-external.swift.org/job/swift-PR-windows/1596/consoleText