AST: Remove legacy GSB-based GenericSignature query implementation #40812
Conversation
slavapestov
force-pushed
the
remove-gsb-queries
branch
from
8311b7d to
62ea3e5
Compare
slavapestov
force-pushed
the
remove-gsb-queries
branch
from
62ea3e5 to
3a92d2f
Compare
|
@swift-ci Please smoke test |
|
@slavapestov there are several bugs with RequirementMachine that make me unable to run a lot of basic code with generics. I have to stick with the Xcode 13.2.1 Swift 5.5.2 toolchain just to get this to work. I tried the recent Swift 5.6 toolchain, and it isn't usable either. Is it possible to disable RequirementMachine so that I can proceed with using code that compiled just fine previously on Swift 5.5, while benefiting from lower-level AutoDiff bug fixes in the newer toolchains? I tried passing in The error below was on the January 11, 2022 Swift 5.6 toolchain. Have these kinds of errors been fixed in the newer ones? If not, could you re-introduce the ability to disable RequirementMachine in Swift 5.6 for clients whose code will now fail to compile? |
|
@philipturner - In pretty much every case we've encountered so far, RequirementMachine has only exposed pre-existing problems and allowed us to fix them at the source (see SR-15530 as one example). I think it's inappropriate to ask for this to be turned off when the problems most likely lie elsewhere. We should instead isolate and fix the real issues here. |
@BradLarson this code produces an error caused exclusively by RequirementMachine: protocol DifferentiableCollection
where INVALID_1 == DifferentiableCollectionView<ElementTangentCollection> {
associatedtype ElementTangentCollection: DifferentiableCollection
where ElementTangentCollection.INVALID_2 == INVALID_3
}
extension DifferentiableCollection {
typealias DifferentiableView = DifferentiableCollectionView<Self>
}
struct DifferentiableCollectionView<Base: DifferentiableCollection> {}That's what I meant when I said "unable to run a lot of basic code with generics". This instance has nothing to do with AutoDiff, yet it's blocking further progress on my end. Full link to crash reproducer: |
|
I’m just hoping that I can build S4TF on the 5.6 release toolchain using my Differentiation package. A massive amount of crashes are gone when RequirementMachine is disabled. Otherwise, I’ll be stuck on 5.5.2 for Swift-Colab and iOS until WWDC 2022. |
|
@philipturner I didn't remove anything in the Swift 5.6 branch. The
|
|
About the other assertion:
Passing |
|
Thanks for the explanation you gave me. What about the code I cited in the comment to Brad Larson? I just reproduced it on Swift main and it still crashes. It seems to be an incorrect assert. Would you mind helping me file a test for it and the crash you just explained? I just narrowed down the bug you explained, and it indeed is caused by AutoDiff. It won't happen unless I add |
|
I tried your test case on main with the feature flags which turn off the GenericSignatureBuilder entirely, using the RequirementMachine for all generic signature computations: protocol DifferentiableCollection
where INVALID_1 == DifferentiableCollectionView<ElementTangentCollection> {
associatedtype ElementTangentCollection: DifferentiableCollection
where ElementTangentCollection.INVALID_2 == INVALID_3
}
extension DifferentiableCollection {
typealias DifferentiableView = DifferentiableCollectionView<Self>
}
struct DifferentiableCollectionView<Base: DifferentiableCollection> {}The large number of 'circular reference' diagnostics is unfortunate but that's an old problem. |
|
I looked at public protocol Differentiable {
associatedtype TangentVector: Differentiable
where TangentVector.TangentVector == TangentVector
}
public protocol DifferentiableCollection: Collection, Differentiable
where
Element: Differentiable,
TangentVector == ElementTangentCollection.DifferentiableView
{
associatedtype DifferentiableView: DifferentiableCollectionViewProtocol
associatedtype ElementTangentCollection: DifferentiableCollection
}
public protocol DifferentiableCollectionViewProtocol: DifferentiableCollection {
associatedtype Base: DifferentiableCollection
}This crashes in an assert build of 5.5 and 5.6 with this assertion: and on main it's slightly different but it's the same error: The problem in all cases is the GenericSignatureBuilder incorrectly minimizing same-type requirements. The test case works if I pass |
|
I just got it to compile successfully when using your build flags. So I don't consider it a compiler crasher anymore. I actually just came across a 5th crasher when copying and pasting my existing "release toolchain workaround" and compiling it. I'm isolating that bug right now. |
|
The errors you got were the same two ones I kept seeing in Xcode when I switched between dev 01/09 and 5.6 01/11. |
|
I added Still, could you isolate it further and report it? I don't have enough experience to do so effectively myself. Going back through the crash logs, the original crash has 54 lines, while the final one has 33. That means there's going to be a 6th crash I have to report soon. In the meantime, I still have one of the 54-line stack traces in the terminal: |
|
|
|
CC: @BradLarson |
|
@slavapestov I misunderstood which crash you were referring to (I thought it was 5). On line 28, there is a function that refers to AutoDiff. So both crashes 5 and 6 should be discussed by someone with expertise on AutoDiff. To anyone who wishes to address my concerns, please do look at my comments in #41094. Both crashers call into For crash 6, I had conformed the Update: I isolated crashes 5 and 6 further. They seem to be the same crash, just slightly modified to get different effects. |
|
Is the following a valid set of generic requirements? |
|
@philipturner No, |
|
So the following would be correct then? The crash message always says this for crash 6: |
|
It could mean that |
This should only happen if |
|
@slavapestov I got something very similar to It passes regardless of whether RequirementMachine is on. I got the same stack trace on both the January 9 developer nighty and Swift main. I also got the same crash with a different stack trace using the Xcode 5.5.2 toolchain and my Differentiation package. This all just adds to the confusion, because crash_4 was fixed by enabling RequirementMachine. Meanwhile, crash_7 was active when crash_4 didn't exist yet. |
|
This crash is unrelated to the requirement machine. It trips in AutoDiff code over an unconditional runtime cast on this line, because it expects |
|
Is it possible to just solve by checking if that's an |
|
Sure, go ahead!
We may want to use either |
|
@slavapestov crash_6 in my repository seems to be wierdly related to RequirementMachine. It now runs successfully when you pass in no flags, but crashes when you pass in: This contrasts with crashes 1 and 4, which crash normally but run fine with the flags above. This crashing is one of the multiple bugs blocking my collection differentiation PR. I caught it while testing whether #41174 (which fixed crash_5) fixed crash_6. Crashes 5 and 6 were closely related, so I thought they had the same source. |
The RequirementMachine-based queries seem to work well these days, and in fact, we're not even testing the old implementation anymore. Only a handful of tests pass
-requirement-machine=offor-requirement-machine=verify. There are of course countless bugs in the old implementation, and a few tests will already fail with it I imagine. The old code will remain in therelease/5.6branch, but I'm going to rip it out onmainsince it has outlived its purpose.