Fix miscompile when emitting protocol conformance of a runtime-only class #15860
Conversation
slavapestov
force-pushed
the
fix-foreign-class-bug
branch
from
ec60a3e to
a0a4248
Compare
|
@swift-ci Please smoke test |
ClassDecl::isForeign() is true for both CF types and runtime-only classes. In some places, we do not expect to see the latter.
We were emitting foreign type metadata for this case, because of an invalid usage of ClassDecl::isForeign(), which also returns true for runtime-only classes. Fixes <rdar://problem/39117602>.
slavapestov
force-pushed
the
fix-foreign-class-bug
branch
from
a0a4248 to
94409b8
Compare
|
@swift-ci Please smoke test |
|
ClangImporter/explicit-system-framework-path failure is not me. |
|
@swift-ci Please smoke test macOS |
|
... or maybe it is me! Let’s find out. |
| @@ -111,7 +111,7 @@ MetadataLayout &IRGenModule::getMetadataLayout(NominalTypeDecl *decl) { | |||
| auto &entry = MetadataLayouts[decl]; | |||
| if (!entry) { | |||
| if (auto theClass = dyn_cast<ClassDecl>(decl)) { | |||
| if (theClass->isForeign()) | |||
| if (theClass->getForeignClassKind() == ClassDecl::ForeignKind::CFType) | |||
There was a problem hiding this comment.
Yikes, thank you. Perhaps we should do a renaming pass here, because it seems like IRGen is using "foreign class" in places that really mean "CF type". That's how foreign classes started, but with the introduction the Clang's objc_runtime_visible, the meaning of "foreign" in the AST was generalized.
There was a problem hiding this comment.
I don't know that the concept of ForeignKind really holds its weight at all; the capabilities of CF and runtime-only classes overlap somewhat but both diverge in different ways; the Venn diagram doesn't perfectly nest or overlap. IMO We should probably just split the "is CF" and "is objc_runtime_visible" predicates.
Fixes rdar://problem/39117602.