diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index c6d2e966e9e9d..5ba3c94ac5e8e 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -2799,12 +2799,19 @@ NOTE(ambiguous_because_of_trailing_closure,none, // Partial application of foreign functions not supported ERROR(partial_application_of_function_invalid,none, "partial application of %select{" - "function with 'inout' parameters|" "'mutating' method|" "'super.init' initializer chain|" "'self.init' initializer delegation" "}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" + "}0 is not allowed; calling the function has undefined behavior and will " + "be an error in future Swift versions", + (unsigned)) ERROR(self_assignment_var,none, "assigning a variable to itself", ()) diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index dab83ec0cd657..0b7dbab58fe4e 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -87,13 +87,16 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E, // message. struct PartialApplication { enum : unsigned { - Function, MutatingMethod, SuperInit, SelfInit, }; - // 'kind' before 'level' is better for code gen. - unsigned kind : 3; + enum : unsigned { + Error, + CompatibilityWarning, + }; + unsigned compatibilityWarning: 1; + unsigned kind : 2; unsigned level : 29; }; @@ -105,49 +108,18 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E, ~DiagnoseWalker() override { for (auto &unapplied : InvalidPartialApplications) { unsigned kind = unapplied.second.kind; - TC.diagnose(unapplied.first->getLoc(), - diag::partial_application_of_function_invalid, - kind); - } - } - - /// If this is an application of a function that cannot be partially - /// applied, arrange for us to check that it gets fully applied. - void recordUnsupportedPartialApply(ApplyExpr *expr, Expr *fnExpr) { - - if (isa(fnExpr)) { - auto kind = expr->getArg()->isSuperExpr() - ? PartialApplication::SuperInit - : PartialApplication::SelfInit; - - // Partial applications of delegated initializers aren't allowed, and - // don't really make sense to begin with. - InvalidPartialApplications.insert({ expr, {kind, 1} }); - return; - } - - auto fnDeclRef = dyn_cast(fnExpr); - if (!fnDeclRef) - return; - - auto fn = dyn_cast(fnDeclRef->getDecl()); - if (!fn) - return; - - unsigned kind = - fn->isInstanceMember() ? PartialApplication::MutatingMethod - : PartialApplication::Function; - - // Functions with inout parameters cannot be partially applied. - if (!expr->getArg()->getType()->isMaterializable()) { - // We need to apply all argument clauses. - InvalidPartialApplications.insert({ - fnExpr, {kind, fn->getNumParameterLists()} - }); + if (unapplied.second.compatibilityWarning) { + TC.diagnose(unapplied.first->getLoc(), + diag::partial_application_of_function_invalid_swift4, + kind); + } else { + TC.diagnose(unapplied.first->getLoc(), + diag::partial_application_of_function_invalid, + kind); + } } } - /// This method is called in post-order over the AST to validate that /// methods are fully applied when they can't support partial application. void checkInvalidPartialApplication(Expr *E) { if (auto AE = dyn_cast(E)) { @@ -158,8 +130,18 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E, fnExpr = dotSyntaxExpr->getRHS(); // Check to see if this is a potentially unsupported partial - // application. - recordUnsupportedPartialApply(AE, fnExpr); + // application of a constructor delegation. + if (isa(fnExpr)) { + auto kind = AE->getArg()->isSuperExpr() + ? PartialApplication::SuperInit + : PartialApplication::SelfInit; + + // Partial applications of delegated initializers aren't allowed, and + // don't really make sense to begin with. + InvalidPartialApplications.insert( + {E, {PartialApplication::Error, kind, 1}}); + return; + } // If this is adding a level to an active partial application, advance // it to the next level. @@ -173,11 +155,36 @@ static void diagSyntacticUseRestrictions(TypeChecker &TC, const Expr *E, InvalidPartialApplications.erase(foundApplication); if (level > 1) { // We have remaining argument clauses. - InvalidPartialApplications.insert({ AE, {kind, level - 1} }); + // Partial applications were always diagnosed in Swift 4 and before, + // so there's no need to preserve the compatibility warning bit. + InvalidPartialApplications.insert( + {AE, {PartialApplication::Error, kind, level - 1}}); } return; } + + /// If this is a reference to a mutating method, it cannot be partially + /// applied or even referenced without full application, so arrange for + /// us to check that it gets fully applied. + auto fnDeclRef = dyn_cast(E); + if (!fnDeclRef) + return; + + auto fn = dyn_cast(fnDeclRef->getDecl()); + if (!fn || !fn->isInstanceMember() || !fn->isMutating()) + return; + // 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. + auto errorBehavior = TC.Context.LangOpts.isSwiftVersionAtLeast(5) + ? PartialApplication::Error + : PartialApplication::CompatibilityWarning; + + InvalidPartialApplications.insert( + {fnDeclRef, {errorBehavior, + PartialApplication::MutatingMethod, + fn->getNumParameterLists()}}); } // Not interested in going outside a basic expression. diff --git a/test/Compatibility/members.swift b/test/Compatibility/members.swift index 180510688da80..6af22745a6550 100644 --- a/test/Compatibility/members.swift +++ b/test/Compatibility/members.swift @@ -7,5 +7,4 @@ struct X { func g0(_: (inout X) -> (Float) -> ()) {} -// This becomes an error in Swift 4 mode -- probably a bug -g0(X.f1) +g0(X.f1) // expected-warning{{partial application of 'mutating' method}} diff --git a/test/Constraints/members.swift b/test/Constraints/members.swift index 752e33a77a267..4192a11f67470 100644 --- a/test/Constraints/members.swift +++ b/test/Constraints/members.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -swift-version 4 +// RUN: %target-typecheck-verify-swift -swift-version 5 //// // Members of structs @@ -28,7 +28,7 @@ func g0(_: (inout X) -> (Float) -> ()) {} _ = x.f0(i) x.f0(i).f1(i) -g0(X.f1) +g0(X.f1) // expected-error{{partial application of 'mutating' method}} _ = x.f0(x.f2(1)) _ = x.f0(1).f2(i) diff --git a/test/Constraints/mutating_members.swift b/test/Constraints/mutating_members.swift new file mode 100644 index 0000000000000..dea386ba18afa --- /dev/null +++ b/test/Constraints/mutating_members.swift @@ -0,0 +1,22 @@ +// RUN: %target-swift-frontend -typecheck -verify -swift-version 5 %s + +struct Foo { + mutating func boom() {} +} + +let x = Foo.boom // expected-error{{partial application of 'mutating' method}} +var y = Foo() +let z0 = y.boom // expected-error{{partial application of 'mutating' method}} +let z1 = Foo.boom(&y) // expected-error{{partial application of 'mutating' method}} + +func fromLocalContext() -> (inout Foo) -> () -> () { + return Foo.boom // expected-error{{partial application of 'mutating' method}} +} +func fromLocalContext2(x: inout Foo, y: Bool) -> () -> () { + if y { + return x.boom // expected-error{{partial application of 'mutating' method}} + } else { + return Foo.boom(&x) // expected-error{{partial application of 'mutating' method}} + } +} + diff --git a/test/Constraints/mutating_members_compat.swift b/test/Constraints/mutating_members_compat.swift new file mode 100644 index 0000000000000..e29fd3423ae01 --- /dev/null +++ b/test/Constraints/mutating_members_compat.swift @@ -0,0 +1,22 @@ +// RUN: %target-swift-frontend -typecheck -verify -swift-version 4 %s + +struct Foo { + mutating func boom() {} +} + +let x = Foo.boom // expected-warning{{partial application of 'mutating' method}} +var y = Foo() +let z0 = y.boom // expected-error{{partial application of 'mutating' method}} +let z1 = Foo.boom(&y) // expected-error{{partial application of 'mutating' method}} + +func fromLocalContext() -> (inout Foo) -> () -> () { + return Foo.boom // expected-warning{{partial application of 'mutating' method}} +} +func fromLocalContext2(x: inout Foo, y: Bool) -> () -> () { + if y { + return x.boom // expected-error{{partial application of 'mutating' method}} + } else { + return Foo.boom(&x) // expected-error{{partial application of 'mutating' method}} + } +} +