Skip to content

Commit

Permalink
Merge pull request swiftlang#30097 from slavapestov/sr-75-super-method
Browse files Browse the repository at this point in the history 
Sema: Diagnose unbound method references on 'super.'
  • Loading branch information
@slavapestov
slavapestov authored Feb 28, 2020
2 parents 631555a + 019452f commit ea9806d
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 94 deletions.
6 changes: 4 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3285,14 +3285,16 @@ ERROR(partial_application_of_function_invalid,none,
"partial application of %select{"
"'mutating' method|"
"'super.init' initializer chain|"
"'self.init' initializer delegation"
"'self.init' initializer delegation|"
"'super' instance method with metatype base"
"}0 is not allowed",
(unsigned))
WARNING(partial_application_of_function_invalid_swift4,none,
"partial application of %select{"
"'mutating' method|"
"'super.init' initializer chain|"
"'self.init' initializer delegation"
"'self.init' initializer delegation|"
"'super' instance method with metatype base"
"}0 is not allowed; calling the function has undefined behavior and will "
"be an error in future Swift versions",
(unsigned))
Expand Down
2 changes: 2 additions & 0 deletions lib/Sema/CSDiagnostics.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3756,6 +3756,8 @@ bool PartialApplicationFailure::diagnoseAsError() {
anchor, ConstraintLocator::ConstructorMember))) {
kind = anchor->getBase()->isSuperExpr() ? RefKind::SuperInit
: RefKind::SelfInit;
} else if (anchor->getBase()->isSuperExpr()) {
kind = RefKind::SuperMethod;
}

auto diagnostic = CompatibilityWarning
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/CSDiagnostics.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,7 @@ class PartialApplicationFailure final : public FailureDiagnostic {
MutatingMethod,
SuperInit,
SelfInit,
SuperMethod,
};

bool CompatibilityWarning;
Expand Down
174 changes: 88 additions & 86 deletions lib/Sema/ConstraintSystem.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1812,71 +1812,71 @@ static std::pair<Type, Type> getTypeOfReferenceWithSpecialTypeCheckingSemantics(
llvm_unreachable("Unhandled DeclTypeCheckingSemantics in switch.");
}

/// \returns true if given declaration is an instance method marked as
/// `mutating`, false otherwise.
bool isMutatingMethod(const ValueDecl *decl) {
if (!(decl->isInstanceMember() && isa<FuncDecl>(decl)))
return false;
return cast<FuncDecl>(decl)->isMutating();
}

static bool shouldCheckForPartialApplication(ConstraintSystem &cs,
const ValueDecl *decl,
ConstraintLocator *locator) {
auto *anchor = locator->getAnchor();
if (!(anchor && isa<UnresolvedDotExpr>(anchor)))
return false;

// If this is a reference to instance method marked as 'mutating'
// it should be checked for invalid partial application.
if (isMutatingMethod(decl))
return true;

// Another unsupported partial application is related
// to constructor delegation via `self.init` or `super.init`.

if (!isa<ConstructorDecl>(decl))
return false;

auto *UDE = cast<UnresolvedDotExpr>(anchor);
// This is `super.init`
if (UDE->getBase()->isSuperExpr())
return true;

// Or this might be `self.init`.
if (auto *DRE = dyn_cast<DeclRefExpr>(UDE->getBase())) {
if (auto *baseDecl = DRE->getDecl())
return baseDecl->getBaseName() == cs.getASTContext().Id_self;
}

return false;
}

/// Try to identify and fix failures related to partial function application
/// e.g. partial application of `init` or 'mutating' instance methods.
static std::pair<bool, unsigned>
isInvalidPartialApplication(ConstraintSystem &cs, const ValueDecl *member,
isInvalidPartialApplication(ConstraintSystem &cs,
const AbstractFunctionDecl *member,
ConstraintLocator *locator) {
if (!shouldCheckForPartialApplication(cs, member, locator))
return {false, 0};
auto *UDE = dyn_cast_or_null<UnresolvedDotExpr>(locator->getAnchor());
if (UDE == nullptr)
return {false,0};

auto anchor = cast<UnresolvedDotExpr>(locator->getAnchor());
// If this choice is a partial application of `init` or
// `mutating` instance method we should report that it's not allowed.
auto baseTy =
cs.simplifyType(cs.getType(anchor->getBase()))->getWithoutSpecifierType();
cs.simplifyType(cs.getType(UDE->getBase()))->getWithoutSpecifierType();

auto isInvalidIfPartiallyApplied = [&]() {
if (auto *FD = dyn_cast<FuncDecl>(member)) {
// 'mutating' instance methods cannot be partially applied.
if (FD->isMutating())
return true;

// Instance methods cannot be referenced on 'super' from a static
// context.
if (UDE->getBase()->isSuperExpr() &&
baseTy->is<MetatypeType>() &&
!FD->isStatic())
return true;
}

// Partial applications are not allowed only for constructor
// delegation, reference on the metatype is considered acceptable.
if (baseTy->is<MetatypeType>() && isa<ConstructorDecl>(member))
return {false, 0};
// Another unsupported partial application is related
// to constructor delegation via 'self.init' or 'super.init'.
//
// Note that you can also write 'self.init' or 'super.init'
// inside a static context -- since 'self' is a metatype there
// it doesn't have the special delegation meaning that it does
// in the body of a constructor.
if (isa<ConstructorDecl>(member) && !baseTy->is<MetatypeType>()) {
// Check for a `super.init` delegation...
if (UDE->getBase()->isSuperExpr())
return true;

// ... and `self.init` delegation. Note that in a static context,
// `self.init` is just an ordinary partial application; it's OK
// because there's no associated instance for delegation.
if (auto *DRE = dyn_cast<DeclRefExpr>(UDE->getBase())) {
if (auto *baseDecl = DRE->getDecl()) {
if (baseDecl->getBaseName() == cs.getASTContext().Id_self)
return true;
}
}
}

return false;
};

if (!isInvalidIfPartiallyApplied())
return {false,0};

// If base is a metatype it would be ignored (unless this is an initializer
// call), but if it is some other type it means that we have a single
// application level already.
unsigned level = baseTy->is<MetatypeType>() ? 0 : 1;
if (auto *call = dyn_cast_or_null<CallExpr>(cs.getParentExpr(anchor))) {
level += dyn_cast_or_null<CallExpr>(cs.getParentExpr(call)) ? 2 : 1;
unsigned level = 0;
if (!baseTy->is<MetatypeType>())
level++;

if (auto *call = dyn_cast_or_null<CallExpr>(cs.getParentExpr(UDE))) {
level += 1;
}

return {true, level};
Expand Down Expand Up @@ -2357,39 +2357,41 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
}
}

// Check whether applying this overload would result in invalid
// partial function application e.g. partial application of
// mutating method or initializer.

// This check is supposed to be performed without
// `shouldAttemptFixes` because name lookup can't
// detect that particular partial application is
// invalid, so it has to return all of the candidates.

bool isInvalidPartialApply;
unsigned level;

std::tie(isInvalidPartialApply, level) =
isInvalidPartialApplication(*this, decl, locator);

if (isInvalidPartialApply) {
// No application at all e.g. `Foo.bar`.
if (level == 0) {
// Swift 4 and earlier failed to diagnose a reference to a mutating
// method without any applications at all, which would get
// miscompiled into a function with undefined behavior. Warn for
// source compatibility.
bool isWarning = !getASTContext().isSwiftVersionAtLeast(5);
(void)recordFix(
AllowInvalidPartialApplication::create(isWarning, *this, locator));
} else if (level == 1) {
// `Self` parameter is applied, e.g. `foo.bar` or `Foo.bar(&foo)`
(void)recordFix(AllowInvalidPartialApplication::create(
/*isWarning=*/false, *this, locator));
}
if (auto *afd = dyn_cast<AbstractFunctionDecl>(decl)) {
// Check whether applying this overload would result in invalid
// partial function application e.g. partial application of
// mutating method or initializer.

// This check is supposed to be performed without
// `shouldAttemptFixes` because name lookup can't
// detect that particular partial application is
// invalid, so it has to return all of the candidates.

bool isInvalidPartialApply;
unsigned level;

std::tie(isInvalidPartialApply, level) =
isInvalidPartialApplication(*this, afd, locator);

if (isInvalidPartialApply) {
// No application at all e.g. `Foo.bar`.
if (level == 0) {
// Swift 4 and earlier failed to diagnose a reference to a mutating
// method without any applications at all, which would get
// miscompiled into a function with undefined behavior. Warn for
// source compatibility.
bool isWarning = !getASTContext().isSwiftVersionAtLeast(5);
(void)recordFix(
AllowInvalidPartialApplication::create(isWarning, *this, locator));
} else if (level == 1) {
// `Self` parameter is applied, e.g. `foo.bar` or `Foo.bar(&foo)`
(void)recordFix(AllowInvalidPartialApplication::create(
/*isWarning=*/false, *this, locator));
}

// Otherwise both `Self` and arguments are applied,
// e.g. `foo.bar()` or `Foo.bar(&foo)()`, and there is nothing to do.
// Otherwise both `Self` and arguments are applied,
// e.g. `foo.bar()` or `Foo.bar(&foo)()`, and there is nothing to do.
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/Constraints/mutating_members.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ func bar() -> Foo.Type { fatalError() }

_ = bar().boom // expected-error{{partial application of 'mutating' method}}
_ = bar().boom(&y) // expected-error{{partial application of 'mutating' method}}
_ = bar().boom(&y)() // ok
_ = bar().boom(&y)() // expected-error{{partial application of 'mutating' method}}

func foo(_ foo: Foo.Type) {
_ = foo.boom // expected-error{{partial application of 'mutating' method}}
_ = foo.boom(&y) // expected-error{{partial application of 'mutating' method}}
_ = foo.boom(&y)() // ok
_ = foo.boom(&y)() // expected-error{{partial application of 'mutating' method}}
}
4 changes: 2 additions & 2 deletions test/Constraints/mutating_members_compat.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ func bar() -> Foo.Type { fatalError() }

_ = bar().boom // expected-warning{{partial application of 'mutating' method}}
_ = bar().boom(&y) // expected-error{{partial application of 'mutating' method}}
_ = bar().boom(&y)() // ok
_ = bar().boom(&y)() // expected-error{{partial application of 'mutating' method}}

func foo(_ foo: Foo.Type) {
_ = foo.boom // expected-warning{{partial application of 'mutating' method}}
_ = foo.boom(&y) // expected-error{{partial application of 'mutating' method}}
_ = foo.boom(&y)() // ok
_ = foo.boom(&y)() // expected-error{{partial application of 'mutating' method}}
}
7 changes: 5 additions & 2 deletions test/NameBinding/name_lookup.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,16 +292,19 @@ class ThisDerived1 : ThisBase1 {
super.baseInstanceVar = 42 // expected-error {{member 'baseInstanceVar' cannot be used on type 'ThisBase1'}}
super.baseProp = 42 // expected-error {{member 'baseProp' cannot be used on type 'ThisBase1'}}
super.baseFunc0() // expected-error {{instance member 'baseFunc0' cannot be used on type 'ThisBase1'}}
super.baseFunc0(ThisBase1())()
// expected-error@-1 {{partial application of 'super' instance method with metatype base is not allowed}}
super.baseFunc0(ThisBase1())() // expected-error {{partial application of 'super' instance method with metatype base is not allowed}}
super.baseFunc1(42) // expected-error {{instance member 'baseFunc1' cannot be used on type 'ThisBase1'}}
super.baseFunc1(ThisBase1())(42)
// expected-error@-1 {{partial application of 'super' instance method with metatype base is not allowed}}
super.baseFunc1(ThisBase1())(42) // expected-error {{partial application of 'super' instance method with metatype base is not allowed}}
super[0] = 42.0 // expected-error {{instance member 'subscript' cannot be used on type 'ThisBase1'}}
super.baseStaticVar = 42
super.baseStaticProp = 42
super.baseStaticFunc0()

super.baseExtProp = 42 // expected-error {{member 'baseExtProp' cannot be used on type 'ThisBase1'}}
super.baseExtFunc0() // expected-error {{instance member 'baseExtFunc0' cannot be used on type 'ThisBase1'}}
// expected-error@-1 {{partial application of 'super' instance method with metatype base is not allowed}}
super.baseExtStaticVar = 42 // expected-error {{instance member 'baseExtStaticVar' cannot be used on type 'ThisBase1'}}
super.baseExtStaticProp = 42 // expected-error {{member 'baseExtStaticProp' cannot be used on type 'ThisBase1'}}
super.baseExtStaticFunc0()
Expand Down

0 comments on commit ea9806d

Please sign in to comment.