IRGen: Clean up types of outlined existential buffer operations #27101
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
|
@swift-ci Please smoke test |
|
@swift-ci Please test source compatibility |
compnerd
reviewed
Sep 10, 2019
lib/IRGen/GenExistential.cpp
Outdated
| @@ -1355,6 +1357,36 @@ createErrorExistentialTypeInfo(IRGenModule &IGM, | |||
| refcounting); | |||
| } | |||
|
|
|||
| llvm::Type *TypeConverter::getExistentialType(unsigned numWitnessTables) { | |||
| auto &type = OpaqueExistentialTypes[numWitnessTables]; | |||
There was a problem hiding this comment.
Could you please use the explicit type here. The reference makes this confusing - the reference indicates that the value is non-null, and the type check on the next line doesn't make sense until you realise that the value that you have a reference to is a pointer.
|
@swift-ci please test Windows platform |
The uniquing key for these was just the number of witness tables, but the function itself referenced the specific existential type it was instantiated with. Everything still worked because getOrCreateHelperFunction() would bitcast an existing function to the correct type, and in practice the layout of an existential type only depends on the number of witness tables. However on master-next, other changes were made that stripped off the bitcasts. This would result in assertions or LLVM verifier failures when multiple existential types were used in a single translation unit. Fixes <rdar://problem/54780404>.
slavapestov
force-pushed
the
outlined-existential-operations-fix
branch
from
3495f2d to
bc1aa97
Compare
|
@swift-ci Please smoke test |
|
@swift-ci Please test Windows platform |
1 similar comment
|
@swift-ci Please test Windows platform |
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.
The uniquing key for these was just the number of witness tables,
but the function itself referenced the specific existential type
it was instantiated with.
Everything still worked because getOrCreateHelperFunction() would
bitcast an existing function to the correct type, and in practice
the layout of an existential type only depends on the number of
witness tables.
However on master-next, other changes were made that stripped
off the bitcasts. This would result in assertions or LLVM
verifier failures when multiple existential types were used in
a single translation unit.
Fixes rdar://problem/54780404.