AST: Add workaround for incorrect mangling of conditional conformances with pack requirements #77463
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s with pack requirements I added commit 7eecf97 a while ago to fix a newly-added assertion failure that came up, however this had the inadvertent side effect of changing symbol mangling and ASTPrinter behavior. The problem in both instances was that we would incorrectly return certain requirements as unsatisfied when really they are satisfied. There is nothing to fix in the ASTPrinter, because printing redundant requirements does not change the generic signature of the extension; they are simply dropped. I added a test to exercise the new behavior showing that the requirements are dropped. As for the mangler, the fix introduced an ABI break, because the symbol name of a conformance descriptor includes its conditional requirements, so we must preserve the redundant requirements forever. The IRGen test has some examples of manglings that are incorrect but must be preserved. I'm plumbing down a flag to isRequirementSatified() to preserve compatibility with the old behavior where we would mangle these redundant requirements. No other callers should pass this flag, except for the mangler. Fixes rdar://139089004.
slavapestov
force-pushed
the
fix-rdar139089004
branch
from
fe638f1 to
69e3aaf
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test Linux |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I added commit 7eecf97 a while ago to fix a newly-added assertion failure that came up, however this had the inadvertent side effect of changing symbol mangling and ASTPrinter behavior.
The problem in both instances was that we would incorrectly return certain requirements as unsatisfied when really they are satisfied.
There is nothing to fix in the ASTPrinter, because printing redundant requirements does not change the generic signature of the extension; they are simply dropped. I added a test to exercise the new behavior showing that the requirements are dropped.
As for the mangler, the fix introduced an ABI break, because the symbol name of a conformance descriptor includes its conditional requirements, so we must preserve the redundant requirements forever.
I'm plumbing down a flag to isRequirementSatified() to preserve compatibility with the old behavior where we would mangle these redundant requirements. No other callers should pass this flag, except for the mangler.
Fixes rdar://139089004.