Devirtualizer: fix a crash due to a not supported bitcast of ABI compatible types #74574
Conversation
|
@swift-ci test |
|
|
||
| llvm::errs() << "Source type: " << srcTy << "\n"; | ||
| llvm::errs() << "Destination type: " << destTy << "\n"; | ||
| llvm_unreachable("Unknown combination of types for casting"); | ||
| } | ||
|
|
||
| namespace { | ||
| class TypeDependentVisitor : public CanTypeVisitor<TypeDependentVisitor, bool> { |
There was a problem hiding this comment.
Can you put this in a shared utility instead of copy and pasting it from IRGen?
There was a problem hiding this comment.
This utility is for a different purpose and also behaves substantially different. It's not a copy-paste from IRGen.
What could be done, though is the extract the visit logic for stored properties and enum cases into a base class visitor, like ValuePropertyTypeVisitor from which both utilities can derive.
But I'd like to do that in a follow-up PR to minimize the risk for cherry-picking this PR to 6.0.
|
Actually I don't understand how this can ever happen, and the test case is quite large so it's not clear to me what's going on. Maybe there's a bug somewhere else? |
This comes from whole-module reasoning of protocol conformances. and after devirtualizing the The |
Oh, is this the getSoleConformingType() thing in ProtocolConformanceAnalysis.h?
what if I call |
You can't, because you cannot construct an |
I see! In that case, do you actually need to check that the types are layout-compatible? Is it just an assertion at that point or can it actually be the case that they are not compatible? |
Exactly (the else of the if directly runs into an unreachable). But I think it's an important assert. Therefore I added the utility |
Maybe. But I'm not sure if those unsafe bitcast instructions can also stem from unsafe user code, where there is no (provable) layout compatibility on SIL level between the involved types. |
|
@swift-ci test |
…ut is dependent on its generic parameters
This is usually the case. Some examples, where they layout is _not_ dependent:
```
struct S<T> {
var x: Int // no members which depend on T
}
struct S<T> {
var c: SomeClass<T> // a class reference does not depend on the layout of the class
}
```
…atible types
When devirtualizing witness method calls, it can happen that we need a cast between ABI compatible return types.
We were missing supporting type casts between nominal types which are ABI compatible.
This comes from whole-module reasoning of protocol conformances.
If a protocol only has a single conformance where the associated type (`ID`) is some concrete type (e.g. `Int`), then the devirtualizer knows that `p.get()` can only return an `Int`:
```
public struct X2<ID> {
let p: any P2<ID>
public func testit(i: ID, x: ID) -> S2<ID> {
return p.get(x: x)
}
}
```
and after devirtualizing the `get` function, its result must be cast from `Int` to `ID`.
The `layoutIsTypeDependent` utility is basically only used here to assert that this cast can only happen between layout compatible types.
rdar://129004015
eeckstein
force-pushed
the
fix-devirtualizer
branch
from
3fa686b to
864c143
Compare
|
@swift-ci test |
When devirtualizing witness method calls, it can happen that we need a cast between ABI compatible return types.
We were missing supporting type casts between nominal types which are ABI compatible.
This comes from whole-module reasoning of protocol conformances.
If a protocol only has a single conformance where the associated type (
ID) is some concrete type (e.g.Int), then the devirtualizer knows thatp.get()can only return anInt:and after devirtualizing the
getfunction, its result must be cast fromInttoID.The
layoutIsTypeDependentutility is basically only used here to assert that this cast can only happen between layout compatible types.rdar://129004015