Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Self available to member functions (SE-0068?) #22863

Merged
merged 19 commits into from
Mar 16, 2019

Conversation

johnno1962
Copy link
Contributor

Hi Apple,

A cheeky implementation of SE-0068 that intercepts the error message use of unresolved identifier 'Self'; did you mean 'self'? and makes Self a synonym for type(of: self) when it has no other meaning. Works surprisingly well and only required a change to an error message in one test to have them pass.

Resolves SR-1340.

lib/Sema/TypeCheckConstraints.cpp Outdated Show resolved Hide resolved
lib/Sema/TypeCheckConstraints.cpp Outdated Show resolved Hide resolved
lib/Sema/TypeCheckConstraints.cpp Outdated Show resolved Hide resolved
@slavapestov
Copy link
Contributor

You'll have to get this working in type context as well, eg struct S { func foo() -> Self { ... } }

@johnno1962
Copy link
Contributor Author

johnno1962 commented Feb 25, 2019

Eh? I don’t understand. The error is as before: 'Self' is only available in a protocol or as the result of a method in a class; did you mean 'S'? It works fine for a class:

class S {
  required init() {
  }
  func foo() -> Self {
    return Self.init()
  }
}

The patch leaves what works already alone and adds some new contexts where Self is valid.

@xwu
Copy link
Collaborator

xwu commented Feb 25, 2019

It works fine for a class:

Right, but to implement SE-0068 and resolve SR-1340, it'll need to work for a struct too.

@johnno1962
Copy link
Contributor Author

If works inside the function body for a struct but I don’t understand why there would be a use case for Self as a return type for a struct type. The following works:

struct S2 {
  let x = 99
  let y: S
  init() {
    y = S()
  }
  func foo() -> S2 {
    return Self.init()
  }
}

Is it important that this works for a struct which cannot inherit?

  func foo() -> Self {
    return Self.init()
  }

I don’t see this mentioned in the proposal.

@johnno1962
Copy link
Contributor Author

johnno1962 commented Feb 26, 2019

OK, I can see where it is mentioned in the review decision. Seems pointless to extend Self to return types of member functions of value types to me. We're getting into diminishing returns.

@slavapestov
Copy link
Contributor

@johnno1962 It's not pointless. With generics it allows you to avoid re-stating a complex type.

Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you were calling this a "cheeky" implementation at first but this is starting to look real. I think my main remaining concern is how the code is located inside diagnostic paths. If we find a better place for it to live it would be great.

@jckarter
Copy link
Contributor

This is great! Does this manage to make x as? Self work too?

@johnno1962
Copy link
Contributor Author

johnno1962 commented Feb 27, 2019

x as? Self doesn't work at the moment as it expands to x as? type(of: self). Maybe it will work if we move to using a TypeExpr.

Assertion failed: (!type->hasTypeParameter()), function openUnboundGenericType, file /Volumes/Elements/swift-self/swift/lib/Sema/ConstraintSystem.cpp, line 538.
Stack dump:
0.	Program arguments: /Volumes/Elements/swift-self/build/buildbot_osx/swift-macosx-x86_64/bin/swift -frontend -interpret h.swift -enable-objc-interop -sdk /Applications/Xcode1023.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -O -color-diagnostics -module-name h
1.	While type-checking 'x()' (at h.swift:15:3)
2.	While type-checking statement at [h.swift:15:12 - line:20:3] RangeText="{
    print("\(Self.self).\(#function)")
    Self.y()
    Self.z()
    _ = Self.init(a: 77) as? Self
  "
3.	While type-checking expression at [h.swift:19:5 - line:19:30] RangeText="_ = Self.init(a: 77) as? "
0  swift                    0x000000010bbef3c5 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
1  swift                    0x000000010bbee665 llvm::sys::RunSignalHandlers() + 85
2  swift                    0x000000010bbef9b8 SignalHandler(int) + 264
3  libsystem_platform.dylib 0x00007fff54501f5a _sigtramp + 26
4  libsystem_platform.dylib 0x00007ffee7e331a8 _sigtramp + 2475889256
5  libsystem_c.dylib        0x00007fff5429f1ae abort + 127
6  libsystem_c.dylib        0x00007fff542671ac basename_r + 0
7  swift                    0x000000010880e0c8 swift::constraints::ConstraintSystem::openUnboundGenericType(swift::Type, swift::constraints::ConstraintLocatorBuilder) + 168
8  swift                    0x00000001087952a8 swift::ASTVisitor<(anonymous namespace)::ConstraintGenerator, swift::Type, void, void, void, void, void>::visit(swift::Expr*) + 920
9  swift                    0x0000000108794c35 (anonymous namespace)::ConstraintWalker::walkToExprPost(swift::Expr*) + 389
10 swift                    0x0000000108c21e3f swift::ASTVisitor<(anonymous namespace)::Traversal, swift::Expr*, swift::Stmt*, bool, swift::Pattern*, bool, void>::visit(swift::Expr*) + 6751
11 swift                    0x0000000108c1fd94 swift::Expr::walk(swift::ASTWalker&) + 84
12 swift                    0x0000000108790f12 swift::constraints::ConstraintSystem::generateConstraints(swift::Expr*) + 418
13 swift                    0x00000001087c716b swift::constraints::ConstraintSystem::solveImpl(swift::Expr*&, swift::Type, swift::ExprTypeCheckListener*, llvm::SmallVectorImpl<swift::constraints::Solution>&, swift::FreeTypeVariableBinding) + 475
14 swift                    0x00000001087c6d3f swift::constraints::ConstraintSystem::solve(swift::Expr*&, swift::Type, swift::ExprTypeCheckListener*, llvm::SmallVectorImpl<swift::constraints::Solution>&, swift::FreeTypeVariableBinding) + 31
15 swift                    0x000000010887dd9c swift::TypeChecker::typeCheckExpressionImpl(swift::Expr*&, swift::DeclContext*, swift::TypeLoc, swift::ContextualTypePurpose, swift::OptionSet<swift::TypeCheckExprFlags, unsigned int>, swift::ExprTypeCheckListener&, swift::constraints::ConstraintSystem*) + 908
16 swift                    0x000000010887d9ff swift::TypeChecker::typeCheckExpression(swift::Expr*&, swift::DeclContext*, swift::TypeLoc, swift::ContextualTypePurpose, swift::OptionSet<swift::TypeCheckExprFlags, unsigned int>, swift::ExprTypeCheckListener*, swift::constraints::ConstraintSystem*) + 63
17 swift                    0x000000010891322b swift::ASTVisitor<(anonymous namespace)::StmtChecker, void, swift::Stmt*, void, void, void, void>::visit(swift::Stmt*) + 587
18 swift                    0x00000001089123f8 bool (anonymous namespace)::StmtChecker::typeCheckStmt<swift::BraceStmt>(swift::BraceStmt*&) + 136
19 swift                    0x0000000108910f68 swift::TypeChecker::typeCheckFunctionBodyUntil(swift::FuncDecl*, swift::SourceLoc) + 264
20 swift                    0x0000000108911abf swift::TypeChecker::typeCheckAbstractFunctionBody(swift::AbstractFunctionDecl*) + 271
21 swift                    0x0000000108932b53 typeCheckFunctionsAndExternalDecls(swift::SourceFile&, swift::TypeChecker&) + 323
22 swift                    0x0000000108933789 swift::performTypeChecking(swift::SourceFile&, swift::TopLevelContext&, swift::OptionSet<swift::TypeCheckingFlags, unsigned int>, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int) + 1065
23 swift                    0x0000000108068d66 swift::CompilerInstance::parseAndTypeCheckMainFileUpTo(swift::SourceFile::ASTStage_t, swift::PersistentParserState&, swift::DelayedParsingCallbacks*, swift::OptionSet<swift::TypeCheckingFlags, unsigned int>) + 502

@jckarter
Copy link
Contributor

Thanks for trying. This is great progress nonetheless!

@johnno1962
Copy link
Contributor Author

Wrapping up for the day. I can get TypeExpr working but I'm not seeing overloaded methods being called when they are messaged on Self so I've given up on using a TypeExpr for now.

//        Type SelfType = DC->getInnermostTypeContext()->getSelfInterfaceType();
//        if (ClassType::classof(SelfType.getPointer()))
//          SelfType = DynamicSelfType::get(SelfType, Context);
//        return new (Context) TypeExpr(TypeLoc(new (Context)
//                FixedTypeRepr(DC//->getInnermostTypeContext()
//                              ->mapTypeIntoContext(SelfType), Loc)));

With Joe's x as? Self it works even using a DynamicTypeExpr provided Self is not generic. You get a similar result when you try something like let a: Self = Self.init(). The code that fails in both cases is:

swift was compiled with optimization - stepping may behave oddly; variables may not be available.
frame #4: 0x0000000100a480c8 swift`swift::constraints::ConstraintSystem::openUnboundGenericType(this=<unavailable>, type=<unavailable>, locator=ConstraintLocatorBuilder @ 0x00007ffeefbf9560) at ConstraintSystem.cpp:538:3 [opt]
   535
   536 	Type ConstraintSystem::openUnboundGenericType(
   537 	    Type type, ConstraintLocatorBuilder locator) {
-> 538 	  assert(!type->hasTypeParameter());
   539
   540 	  checkNestedTypeConstraints(*this, type, locator);
   541
(lldb)
frame #5: 0x00000001009cf2a8 swift`swift::ASTVisitor<(anonymous namespace)::ConstraintGenerator, swift::Type, void, void, void, void, void>::visit(swift::Expr*) [inlined] (anonymous namespace)::ConstraintGenerator::visitConditionalCheckedCastExpr(this=0x00007ffeefbf9968, expr=0x00000001098ddcf8) at CSGen.cpp:2762:24 [opt]
   2759	        return nullptr;
   2760
   2761	      // Open the type we're casting to.
-> 2762	      auto toType = CS.openUnboundGenericType(expr->getCastTypeLoc().getType(),
   2763	                                              CS.getConstraintLocator(expr));
   2764	      CS.setType(expr->getCastTypeLoc(), toType);
   2765

Which implies the type has this property:

    /// This type expression contains a GenericTypeParamType.
    HasTypeParameter     = 0x04,

Fixable or can we only improve the error message?

@slavapestov
Copy link
Contributor

The problem with DynamicSelfType should be fixable. I'll take a look at it.

@johnno1962
Copy link
Contributor Author

johnno1962 commented Mar 1, 2019

Hi @slavapestov, managed to get a TypeExpr working including overrides with the following code:

        Type SelfType = DC->getInnermostTypeContext()->getSelfInterfaceType();
        if (SelfType->is<ClassType>() || SelfType->is<BoundGenericClassType>())
          SelfType = DynamicSelfType::get(SelfType, Context);
        SelfType = DC->mapTypeIntoContext(SelfType);
        return new (Context) TypeExpr(TypeLoc(new (Context)
                FixedTypeRepr(SelfType, Loc), SelfType));

Is SelfType->is<ClassType>() || SelfType->is<BoundGenericClassType>() exhaustive?

@johnno1962 johnno1962 force-pushed the Self-fallback branch 3 times, most recently from 143b7f9 to 82989fb Compare March 1, 2019 18:13
@slavapestov
Copy link
Contributor

@johnno1962 Rather than testing the type of the DeclContext you can more directly check if its a class or class extension by calling getSelfClassDecl() on the DeclContext.

@johnno1962
Copy link
Contributor Author

Thanks, I've updated the tests in the last commit. This has to be fairly close now.

@johnno1962 johnno1962 changed the title Make Self available to instance member functions (SE-0068?) Make Self available to member functions (SE-0068?) Mar 14, 2019
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
benrimmington and others added 2 commits March 14, 2019 22:55
Co-Authored-By: johnno1962 <github@johnholdsworth.com>
@johnno1962
Copy link
Contributor Author

johnno1962 commented Mar 14, 2019

Over to you @slavapestov. Thanks for your help! Beware, there is a merge commit d3e2550 on CHANGELOG.md which may complicate cherry picking to 5.1

@slavapestov
Copy link
Contributor

I think this is good enough to merge. We can iterate on the details later (the local function case is probably going to be rare anyway).

@slavapestov
Copy link
Contributor

@swift-ci Please test

@slavapestov
Copy link
Contributor

@swift-ci Please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 988e565

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d91769fba45e9d7e52f82ed910715b9f26c53e28

@slavapestov slavapestov merged commit dbe99d7 into swiftlang:master Mar 16, 2019
@slavapestov
Copy link
Contributor

Thank you for your contribution!

@slavapestov
Copy link
Contributor

Before I forget -- can you submit a PR to the swift-evolution repo marking this proposal as implemented, and file a JIRA to implement the remaining case with local functions?

@benrimmington
Copy link
Contributor

benrimmington commented Mar 16, 2019

@johnno1962 I've updated the proposal status to Implemented (Swift 5.1)

@johnno1962 johnno1962 deleted the Self-fallback branch March 16, 2019 07:39
@johnno1962
Copy link
Contributor Author

There is some code mentioned on the forum that doesn't compile:

protocol Container : AnyObject {
  var coordinator: Coordinator<Self> { get }
}

class Base<T> where T : Container {
  unowned let interface: T

  init(interface: T) {
    self.interface = interface
  }
}

final class Coordinator<T> :
  Base<T> where T : Container {}

class BaseContainer : Container {
  private(set) lazy var coordinator = Coordinator<Self>(interface: self)
}
h.swift:144:13: error: type 'BaseContainer' does not conform to protocol 'Container'
final class BaseContainer : Container {
            ^
h.swift:145:25: note: candidate has non-matching type 'Coordinator<Self>'
  private(set) lazy var coordinator = Coordinator<Self>(interface: self as! Self)
                        ^
h.swift:130:7: note: protocol requires property 'coordinator' with type 'Coordinator<BaseContainer>'; do you want to add a stub?
  var coordinator: Coordinator<Self> { get }
      ^

@johnno1962
Copy link
Contributor Author

Can I leave it to you to arrange the cherry-pick to swift-5.1-branch?

@johnno1962
Copy link
Contributor Author

johnno1962 commented Mar 16, 2019

Local functions should work though I think dc->getParent()->isLocalContext() doesn't work.

@benrimmington
Copy link
Contributor

@johnno1962 There's no need to cherry-pick. The final branching for swift-5.1-branch is March 18.

@johnno1962
Copy link
Contributor Author

Just in time 🙂

@hamishknight
Copy link
Contributor

Another case that would be nice to eventually support is the use of Self in a covariant position of a class method's parameter, e.g:

class C {
  func foo(_ fn: (Self) -> Void) {}
}

Filed SR-10121 to track that :)

@xwu
Copy link
Collaborator

xwu commented Mar 16, 2019

class BaseContainer : Container {
  private(set) lazy var coordinator = Coordinator<Self>(interface: self)
}
h.swift:144:13: error: type 'BaseContainer' does not conform to protocol 'Container'
final class BaseContainer : Container {
            ^

@johnno1962, are you missing a final keyword in your example? The diagnostic text suggests so.

I agree that if BaseContainer is final then this should work; if it is not, I don't see how it can be made to work (since conformances in Swift are inherited by subclasses, a protocol requirement for var coordinator: Coordinator<Self> would require coordinator to have type Coordinator<BaseContainer> in order for the base class to conform and simultaneously to have type Coordinator<DerivedContainer> in order for the derived class to conform, which is impossible).

@johnno1962
Copy link
Contributor Author

I tried with a final as you can see but it didn’t fix it. There will be corner cases to iron out.

@johnno1962 johnno1962 restored the Self-fallback branch March 17, 2019 01:50
@johnno1962
Copy link
Contributor Author

johnno1962 commented Mar 21, 2019

Another snippet of code that is not supported:

class A1 {
    required init() {}
    func copy() -> Self {
        let copy = Self.init()
        return copy
    }

    var copied: Self {
        let copy = Self.init()
        return copy
    }
}

class B: A1 {
    override func copy() -> Self {
        let copy = super.copy() as! Self // supported
        return copy
    }
//    override var copied: Self {
//        let copy = super.copied as! Self // unsupported
//        return copy
//    }
}

print(B().copy())

Trying to override copied gives: Property 'copied' with type 'Self' cannot override a property with type 'Self'. Without the override, trying to use copied instead of copy() gives:

Assertion failed: (hasSelfMetadataParam() && "This method can only be called if the " "SILFunction has a self-metadata parameter"), function getSelfMetadataArgument, file /Users/buildnode/jenkins/workspace/oss-swift-package-osx/swift/include/swift/SIL/SILFunction.h, line 955.
0  swift                    0x000000010bcf0da5 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
1  swift                    0x000000010bcf0065 llvm::sys::RunSignalHandlers() + 85
2  swift                    0x000000010bcf1388 SignalHandler(int) + 264
3  libsystem_platform.dylib 0x00007fff7a096f5a _sigtramp + 26
4  libsystem_platform.dylib 0x00007ffe00000000 _sigtramp + 2247528640
5  libsystem_c.dylib        0x00007fff79e341ae abort + 127
6  libsystem_c.dylib        0x00007fff79dfc1ac basename_r + 0
7  swift                    0x0000000108ae0624 swift::SILFunction::getSelfMetadataArgument() const + 100
8  swift                    0x0000000108ad25e7 collectTypeDependentOperands(llvm::SmallVectorImpl<swift::SILValue>&, swift::SILOpenedArchetypesState&, swift::SILFunction&, swift::CanType, swift::SubstitutionMap) + 663
9  swift                    0x0000000108adb6ee swift::InitExistentialAddrInst::create(swift::SILDebugLocation, swift::SILValue, swift::CanType, swift::SILType, llvm::ArrayRef<swift::ProtocolConformanceRef>, swift::SILFunction*, swift::SILOpenedArchetypesState&) + 94
10 swift                    0x00000001085d0afb swift::Lowering::SILGenBuilder::createInitExistentialAddr(swift::SILLocation, swift::SILValue, swift::CanType, swift::SILType, llvm::ArrayRef<swift::ProtocolConformanceRef>) + 267
11 swift                    0x00000001085ecdd8 (anonymous namespace)::ExistentialInitialization::getAddressForInPlaceInitialization(swift::Lowering::SILGenFunction&, swift::SILLocation) + 632
12 swift                    0x00000001085f1bd1 swift::Lowering::SingleBufferInitialization::copyOrInitValueInto(swift::Lowering::SILGenFunction&, swift::SILLocation, swift::Lowering::ManagedValue, bool) + 49
13 swift                    0x0000000108589b09 (anonymous namespace)::ScalarResultPlan::finish(swift::Lowering::SILGenFunction&, swift::SILLocation, swift::CanType, llvm::ArrayRef<swift::Lowering::ManagedValue>&) + 857
14 swift                    0x00000001085a3357 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) + 1495

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants