[Sema/CSApply] Make sure that the member type references created for a type(of: x).A expression are visible to the SourceEntityWalker
#29585
Conversation
|
@swift-ci Please test |
|
Build failed |
|
@swift-ci test linux platform |
lib/Sema/CSApply.cpp
Outdated
| TypeExpr::createImplicitHack(memberLoc.getBaseNameLoc(), | ||
| refType, context); | ||
| auto ref = TypeExpr::createForDecl(memberLoc, TD, cs.DC, /*isImplicit=*/false); | ||
| ref->setType(MetatypeType::get(refType, context)); |
There was a problem hiding this comment.
Setting type explicitly here shouldn't be necessary, type from the cache would be set for each expression at the end of solution application.
There was a problem hiding this comment.
Also it's weird that ref gets a metatype but cache entry gets its instance type.
There was a problem hiding this comment.
This follows somewhat what TypeExpr::createImplicitHack was doing here.
There was a problem hiding this comment.
Hm, that sounds like a bug we introduced while porting everything stuff to use cache. I'd suggest you just use cs.setType(ref, MetatypeType::get(refType, context)); here instead.
There was a problem hiding this comment.
I'd suggest you just use
cs.setType(ref, MetatypeType::get(refType, context));here instead.
Unfortunately when I do that it causes test/Serialization/nested-type-with-overlay.swift to crash with:
TYPE MISMATCH IN ARGUMENT 0 OF APPLY AT expression at [/Users/argiris/proj/opensource/swift/src/swift/test/Serialization/Inputs/nested-type-with-overlay/overlay.swift:10:32 - line:10:64] RangeText="Base.NestedAndShadowed(dummy: ()"
argument value: %5 = metatype $@thin Base.NestedAndShadowed.Type.Type
parameter type: $@thin Base.NestedAndShadowed.Type
Stack dump:
0. Program arguments: /Users/argiris/proj/opensource/swift/nbuild/Release+Asserts/bin/swiftc -frontend -target x86_64-apple-macosx10.12 -module-cache-path /Users/argiris/proj/opensource/swift/nbuild/Release+Asserts/swift-test-results/x86_64-apple-macosx10.12/clang-module-cache -sdk /System/Volumes/Data/XcodeVer/Current/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.16.sdk -swift-version 4 -ignore-module-source-info -typo-correction-limit 10 -emit-module -o /Users/argiris/proj/opensource/swift/nbuild/Release+Asserts/tools/swift/test-macosx-x86_64/Serialization/Output/nested-type-with-overlay.swift.tmp/HasOverlay.swiftmodule /Users/argiris/proj/opensource/swift/src/swift/test/Serialization/Inputs/nested-type-with-overlay/overlay.swift -I /Users/argiris/proj/opensource/swift/src/swift/test/Serialization/Inputs/nested-type-with-overlay -module-name HasOverlay
1. Swift version 5.2-dev (Swift d8283a9ebe)
2. While evaluating request GenerateSILRequest(SIL Generation for module HasOverlay)
0 swiftc 0x000000010880c998 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
1 swiftc 0x000000010880b838 llvm::sys::RunSignalHandlers() + 248
2 swiftc 0x000000010880cf97 SignalHandler(int) + 295
3 libsystem_platform.dylib 0x00007fff67efa42d _sigtramp + 29
4 libsystem_platform.dylib 0x00007fdd2a01e800 _sigtramp + 3255976944
5 libsystem_c.dylib 0x00007fff67dcfa1c abort + 120
6 swiftc 0x000000010895cc5f emitRawApply(swift::Lowering::SILGenFunction&, swift::SILLocation, swift::Lowering::ManagedValue, swift::SubstitutionMap, llvm::ArrayRef<swift::Lowering::ManagedValue>, swift::CanTypeWrapper<swift::SILFunctionType>, swift::Lowering::ApplyOptions, llvm::ArrayRef<swift::SILValue>, llvm::SmallVectorImpl<swift::SILValue>&) (.cold.10) + 223
7 swiftc 0x0000000104d6dd90 emitRawApply(swift::Lowering::SILGenFunction&, swift::SILLocation, swift::Lowering::ManagedValue, swift::SubstitutionMap, llvm::ArrayRef<swift::Lowering::ManagedValue>, swift::CanTypeWrapper<swift::SILFunctionType>, swift::Lowering::ApplyOptions, llvm::ArrayRef<swift::SILValue>, llvm::SmallVectorImpl<swift::SILValue>&) + 2768
8 swiftc 0x0000000104d6e469 swift::Lowering::SILGenFunction::emitApply(std::__1::unique_ptr<swift::Lowering::ResultPlan, std::__1::default_delete<swift::Lowering::ResultPlan> >&&, swift::Lowering::ArgumentScope&&, swift::SILLocation, swift::Lowering::ManagedValue, swift::SubstitutionMap, llvm::ArrayRef<swift::Lowering::ManagedValue>, swift::Lowering::CalleeTypeInfo const&, swift::Lowering::ApplyOptions, swift::Lowering::SGFContext) + 1065
And If I remove setting the type explicitly via ref->setType(MetatypeType::get(refType, context)); then theTypeExpr node in the AST has different (non-metatype) type; visible if I dump the AST.
There was a problem hiding this comment.
I think the explicit setting to MetatypeType on the TypeExpr ref was probably unintentional effect of calling TypeExpr::createImplicitHack.
In the test case the line _ = X.A.self doesn't emit a TypeExpr as MetatypeType, so having MetatypeType for _ = type(of: x).A.self is actually inconsistent.
If I just remove the explicit type setting then all tests pass.
There was a problem hiding this comment.
Actually, setting the type explicitly seems to only affect printed typereprs (and makes them consistent) the AST Expr types are unchanged.
It looks to me that setting the type explicitly can be dropped.
There was a problem hiding this comment.
+1 on not setting type explicitly. The problem I have is with TypeExpr getting a non-metatype type assigned from the cache. Calling getType on TypeExpr should always produce a metatype.
There was a problem hiding this comment.
What I wonder is whether in this particular case refType is already a metatype and that's why it was just set directly to type expr, but it seems to contradict what TypeExpr::createImplicitHack was expecting to receive...
There was a problem hiding this comment.
@xedin is right that refType should already be a metatype there, because it is the type of the value. When you use a type as a value (Foo.Bar.self) you get a metatype. We could assert here. The old code was just wrong because it would double-wrap the metatype (and I guess for some reason, everything almost worked, at least until @akyrtzi hit this problem...)
…a `type(of: x).A` expression are visible to the `SourceEntityWalker` For code like ``` _ = type(of: x).A.self ``` the `A` type reference is written explicitely by the user, but the AST created using `TypeExpr::createImplicitHack` is hiding such a reference, making the reference invisible to semantic functionality like 'rename'. rdar://56885871
akyrtzi
force-pushed
the
fix-missing-reference
branch
from
d8283a9 to
6052288
Compare
|
@swift-ci Please test |
|
@swift-ci test linux platform |
|
@slavapestov Since you have approved - why is it okay to set a type in the cache different from what is expected by |
For code like
the
Atype reference is written explicitely by the user, but the AST created usingTypeExpr::createImplicitHackis hiding such a reference,making the reference invisible to semantic functionality like 'rename'.
rdar://56885871