[SILGen] Handle arbitrary ObjC-to-Swift bridging for globals #12603
Conversation
|
This looks good at first glance. It should obsolete my fix for dfdd3a7 -- can you try removing it and see if the test still passes? |
|
Actually my fix hits the code path for member VarDecls, but I think we should use the same bridging path component in both cases (the original bug was that NSString globals were bridged, but import-as-member globals were not). |
jrose-apple
force-pushed
the
building-blocks
branch
from
95f6f3f to
5e36a5f
Compare
jrose-apple
changed the title
|
Build failed |
|
Build failed |
|
Uh, hm. I wonder if the Linux failure is a fluke. @swift-ci Please test Linux |
lib/SILGen/SILGenLValue.cpp
Outdated
| @@ -2219,6 +2280,12 @@ static LValue emitLValueForNonMemberVarDecl(SILGenFunction &SGF, | |||
|
|
|||
| if (address.getType().is<ReferenceStorageType>()) | |||
| lv.add<OwnershipComponent>(typeData); | |||
|
|
|||
| AbstractionPattern pattern = SGF.SGM.Types.getAbstractionPattern(var); | |||
| if (pattern.isClangType()) { | |||
There was a problem hiding this comment.
Hang on, should this also check that the types are different? What's the best way to do that? If they happen to be the same we might as well leave the physical component in place.
There was a problem hiding this comment.
You want to check whether the type-of-rvalue matches the target lowered type.
|
Build failed |
jrose-apple
force-pushed
the
building-blocks
branch
from
5e36a5f to
4b3e48c
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
@swift-ci please smoke test macOS |
A more general fix for import-as-member NSStrings than Slava's dfdd3a7. Reverts the SILGen change from that commit in favor of a new PathComponent for lvalue access. In theory this will handle mutable bridged import-as-member globals as well, but we don't have any of those yet.
jrose-apple
force-pushed
the
building-blocks
branch
from
4b3e48c to
d838ebd
Compare
|
Had the wrong type for reverse bridging. @swift-ci Please test |
|
Build failed |
|
Build failed |
|
I've tried several variations and can't get this right. Going to have to shelve it for now. |
|
Filed rdar://problem/35328468 to come back to this later. |
Possibly useful for rdar://problem/34913634, possibly not. Now that it's working in my little test cases I need to go back and examine whether we actually need it, per discussion on #10832. May still be useful in some form for the SIL folks, though.