Merge pull request #17653 from jckarter/no-unapplied-mutating-method-references-4.2
[4.2] Sema: Diagnose completely unapplied references to mutating methods.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
index 47addcd..e1b37d9 100644
--- a/include/swift/AST/DiagnosticsSema.def
+++ b/include/swift/AST/DiagnosticsSema.def
@@ -2702,12 +2702,19 @@
// 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 5a6af26..a5cb986 100644
--- a/lib/Sema/MiscDiagnostics.cpp
+++ b/lib/Sema/MiscDiagnostics.cpp
@@ -86,14 +86,18 @@
// Selector for the partial_application_of_function_invalid diagnostic
// message.
struct PartialApplication {
- unsigned level : 29;
enum : unsigned {
- Function,
MutatingMethod,
SuperInit,
SelfInit,
};
- unsigned kind : 3;
+ enum : unsigned {
+ Error,
+ CompatibilityWarning,
+ };
+ unsigned compatibilityWarning: 1;
+ unsigned kind : 2;
+ unsigned level : 29;
};
// Partial applications of functions that are not permitted. This is
@@ -104,49 +108,18 @@
~DiagnoseWalker() override {
for (auto &unapplied : InvalidPartialApplications) {
unsigned kind = unapplied.second.kind;
- TC.diagnose(unapplied.first->getLoc(),
- diag::partial_application_of_function_invalid,
- kind);
+ 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);
+ }
}
}
- /// 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<OtherConstructorDeclRefExpr>(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, {1, kind} });
- return;
- }
-
- auto fnDeclRef = dyn_cast<DeclRefExpr>(fnExpr);
- if (!fnDeclRef)
- return;
-
- auto fn = dyn_cast<FuncDecl>(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, {fn->getNumParameterLists(), 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<ApplyExpr>(E)) {
@@ -157,8 +130,18 @@
fnExpr = dotSyntaxExpr->getRHS();
// Check to see if this is a potentially unsupported partial
- // application.
- recordUnsupportedPartialApply(AE, fnExpr);
+ // application of a constructor delegation.
+ if (isa<OtherConstructorDeclRefExpr>(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.
@@ -172,11 +155,36 @@
InvalidPartialApplications.erase(foundApplication);
if (level > 1) {
// We have remaining argument clauses.
- InvalidPartialApplications.insert({ AE, {level - 1, kind} });
+ // 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<DeclRefExpr>(E);
+ if (!fnDeclRef)
+ return;
+ auto fn = dyn_cast<FuncDecl>(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 1805106..6af2274 100644
--- a/test/Compatibility/members.swift
+++ b/test/Compatibility/members.swift
@@ -7,5 +7,4 @@
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 752e33a..4192a11 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 @@
_ = 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 0000000..dea386b
--- /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 0000000..e29fd34
--- /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}}
+ }
+}
+
