From a2d1ef67e468c83a4671bfd00f6f3b1b30a684a6 Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Wed, 21 Sep 2022 21:19:32 -0400 Subject: [PATCH 01/12] Issue-60730: Addressing the situation where we were saying that (any P)? does not conform to P, instead we should offer the Force Optional error messaging --- include/swift/Option/Options.td | 3 +++ lib/Sema/CSSimplify.cpp | 17 +++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td index cac5d8b1a8fcd..62452245e1f9b 100644 --- a/include/swift/Option/Options.td +++ b/include/swift/Option/Options.td @@ -241,6 +241,9 @@ def tools_directory : Separate<["-"], "tools-directory">, def D : JoinedOrSeparate<["-"], "D">, Flags<[FrontendOption]>, HelpText<"Marks a conditional compilation flag as true">; + +def e : JoinedOrSeparate<["-"], "e">, Flags<[NewDriverOnlyOption]>, + HelpText<"Executes a line of code provided on the command line">; def F : JoinedOrSeparate<["-"], "F">, Flags<[FrontendOption, ArgumentIsPath, SwiftAPIExtractOption, SwiftSymbolGraphExtractOption, SwiftAPIDigesterOption]>, diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 2570aaab4f9ee..fc921dcb328d9 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -3635,9 +3635,22 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2, // This would make sure that arguments with incorrect // conformances are not prioritized over general argument // mismatches. - if (recordFix(fix, /*impact=*/2)) + auto impact = 2; + if (type1->isOptional()) { + auto unwrappedType = type1->getOptionalObjectType(); + auto matchTypeResult = + matchTypes(unwrappedType, type2, ConstraintKind::Conversion, + subflags, locator); + if (matchTypeResult.isSuccess()) { + // Impact here is a 1 because there is only one failure before + // we need to force type1 to be + impact = 1; + fix = ForceOptional::create(*this, type1, proto, + getConstraintLocator(locator)); + } + } + if (recordFix(fix, /*impact=*/impact)) return getTypeMatchFailure(locator); - break; } From 62220993311531cfd691a46c00d53ec97259337b Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Wed, 21 Sep 2022 22:16:39 -0400 Subject: [PATCH 02/12] Issue-60730: Removing assigned of AllowArgumentMismatch fix that would get thrown away if we fall into ForceOptional fix --- lib/Sema/CSSimplify.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index fc921dcb328d9..998500fa252f0 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -3626,18 +3626,16 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2, if (last.is()) { auto proto = protoDecl->getDeclaredInterfaceType(); - auto *fix = AllowArgumentMismatch::create( - *this, type1, proto, getConstraintLocator(anchor, path)); - + ConstraintFix *fix; // Impact is 2 here because there are two failures // 1 - missing conformance and 2 - incorrect argument type. // // This would make sure that arguments with incorrect // conformances are not prioritized over general argument // mismatches. - auto impact = 2; + unsigned impact; if (type1->isOptional()) { - auto unwrappedType = type1->getOptionalObjectType(); + auto unwrappedType = type1->lookThroughAllOptionalTypes(); auto matchTypeResult = matchTypes(unwrappedType, type2, ConstraintKind::Conversion, subflags, locator); @@ -3649,6 +3647,11 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2, getConstraintLocator(locator)); } } + if (!fix) { + fix = AllowArgumentMismatch::create( + *this, type1, proto, getConstraintLocator(anchor, path)); + impact = 2; + } if (recordFix(fix, /*impact=*/impact)) return getTypeMatchFailure(locator); break; From 81c3b3c703baf0a6133eba0cc72c34446eb8c034 Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Wed, 21 Sep 2022 22:17:44 -0400 Subject: [PATCH 03/12] Issue-60730: Removing file that was accidentally changed --- include/swift/Option/Options.td | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/swift/Option/Options.td b/include/swift/Option/Options.td index 62452245e1f9b..cac5d8b1a8fcd 100644 --- a/include/swift/Option/Options.td +++ b/include/swift/Option/Options.td @@ -241,9 +241,6 @@ def tools_directory : Separate<["-"], "tools-directory">, def D : JoinedOrSeparate<["-"], "D">, Flags<[FrontendOption]>, HelpText<"Marks a conditional compilation flag as true">; - -def e : JoinedOrSeparate<["-"], "e">, Flags<[NewDriverOnlyOption]>, - HelpText<"Executes a line of code provided on the command line">; def F : JoinedOrSeparate<["-"], "F">, Flags<[FrontendOption, ArgumentIsPath, SwiftAPIExtractOption, SwiftSymbolGraphExtractOption, SwiftAPIDigesterOption]>, From 41cbed84ce0d1541c15eb3af4b76375ea642efbe Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Fri, 23 Sep 2022 22:05:06 -0400 Subject: [PATCH 04/12] Issue-60730: Updating to use simplifyConformsToConstraint - and to clean up code per comments --- lib/Sema/CSSimplify.cpp | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 998500fa252f0..9f61373802c6d 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -3626,33 +3626,36 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2, if (last.is()) { auto proto = protoDecl->getDeclaredInterfaceType(); - ConstraintFix *fix; // Impact is 2 here because there are two failures // 1 - missing conformance and 2 - incorrect argument type. // // This would make sure that arguments with incorrect // conformances are not prioritized over general argument // mismatches. - unsigned impact; if (type1->isOptional()) { auto unwrappedType = type1->lookThroughAllOptionalTypes(); - auto matchTypeResult = - matchTypes(unwrappedType, type2, ConstraintKind::Conversion, - subflags, locator); - if (matchTypeResult.isSuccess()) { - // Impact here is a 1 because there is only one failure before - // we need to force type1 to be - impact = 1; - fix = ForceOptional::create(*this, type1, proto, - getConstraintLocator(locator)); + auto result = simplifyConformsToConstraint( + unwrappedType, type2, ConstraintKind::ConformsTo, + getConstraintLocator(locator), + TypeMatchFlags::TMF_ApplyingFix); + if (result == SolutionKind::Solved) { + auto matchTypeResult = + matchTypes(unwrappedType, type2, ConstraintKind::Conversion, + subflags, locator); + if (matchTypeResult.isSuccess()) { + // Impact here is a 1 because there is only one failure before + // we need to force type1 to be + auto fix = ForceOptional::create( + *this, type1, proto, getConstraintLocator(locator)); + if (recordFix(fix, /*impact=*/1)) + return getTypeMatchFailure(locator); + break; + } } } - if (!fix) { - fix = AllowArgumentMismatch::create( - *this, type1, proto, getConstraintLocator(anchor, path)); - impact = 2; - } - if (recordFix(fix, /*impact=*/impact)) + auto fix = AllowArgumentMismatch::create( + *this, type1, proto, getConstraintLocator(anchor, path)); + if (recordFix(fix, /*impact=*/2)) return getTypeMatchFailure(locator); break; } From 54d99aa3d05f290d0ee23915c67d33c8444e738a Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Wed, 28 Sep 2022 19:48:27 -0400 Subject: [PATCH 05/12] Issue-60730: Updating to pass protocoDecl instead of type2 to simplifyConformsToConstraint --- lib/Sema/CSSimplify.cpp | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 9f61373802c6d..9cab18ca391b8 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -3634,27 +3634,18 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2, // mismatches. if (type1->isOptional()) { auto unwrappedType = type1->lookThroughAllOptionalTypes(); - auto result = simplifyConformsToConstraint( - unwrappedType, type2, ConstraintKind::ConformsTo, - getConstraintLocator(locator), - TypeMatchFlags::TMF_ApplyingFix); + auto result = simplifyConformsToConstraint(unwrappedType, protoDecl, + kind, locator, subflags | TMF_ApplyingFix); if (result == SolutionKind::Solved) { - auto matchTypeResult = - matchTypes(unwrappedType, type2, ConstraintKind::Conversion, - subflags, locator); - if (matchTypeResult.isSuccess()) { - // Impact here is a 1 because there is only one failure before - // we need to force type1 to be - auto fix = ForceOptional::create( - *this, type1, proto, getConstraintLocator(locator)); - if (recordFix(fix, /*impact=*/1)) - return getTypeMatchFailure(locator); - break; - } + auto fix = ForceOptional::create( + *this, type1, proto, getConstraintLocator(locator)); + if (recordFix(fix, /*impact=*/1)) + return getTypeMatchFailure(locator); + break; } } auto fix = AllowArgumentMismatch::create( - *this, type1, proto, getConstraintLocator(anchor, path)); + *this, type1, proto, getConstraintLocator(anchor, path)); if (recordFix(fix, /*impact=*/2)) return getTypeMatchFailure(locator); break; From c34e5c279a4c47231b45e6ece4d5851c4374b892 Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Wed, 28 Sep 2022 19:48:27 -0400 Subject: [PATCH 06/12] Issue-60730: Updating to pass protocoDecl instead of type2 to simplifyConformsToConstraint --- lib/Sema/CSSimplify.cpp | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 9f61373802c6d..4f87ebc16e6b5 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -3635,22 +3635,14 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2, if (type1->isOptional()) { auto unwrappedType = type1->lookThroughAllOptionalTypes(); auto result = simplifyConformsToConstraint( - unwrappedType, type2, ConstraintKind::ConformsTo, - getConstraintLocator(locator), - TypeMatchFlags::TMF_ApplyingFix); + unwrappedType, protoDecl, kind, locator, + subflags | TMF_ApplyingFix); if (result == SolutionKind::Solved) { - auto matchTypeResult = - matchTypes(unwrappedType, type2, ConstraintKind::Conversion, - subflags, locator); - if (matchTypeResult.isSuccess()) { - // Impact here is a 1 because there is only one failure before - // we need to force type1 to be - auto fix = ForceOptional::create( - *this, type1, proto, getConstraintLocator(locator)); - if (recordFix(fix, /*impact=*/1)) - return getTypeMatchFailure(locator); - break; - } + auto fix = ForceOptional::create(*this, type1, proto, + getConstraintLocator(locator)); + if (recordFix(fix, /*impact=*/1)) + return getTypeMatchFailure(locator); + break; } } auto fix = AllowArgumentMismatch::create( From 66c2284e7db8913fa290f26410e447de39788459 Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Wed, 28 Sep 2022 19:52:17 -0400 Subject: [PATCH 07/12] Issue-60730: Updating to not pass 1 to recordFix, where that is the default value --- lib/Sema/CSSimplify.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 98534a4e84170..a3a6c98d9ad81 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -3640,7 +3640,7 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2, if (result == SolutionKind::Solved) { auto fix = ForceOptional::create(*this, type1, proto, getConstraintLocator(locator)); - if (recordFix(fix, /*impact=*/1)) + if (recordFix(fix)) return getTypeMatchFailure(locator); break; } From f4c2027d0794dec7aea54dc1f45d10745d2fc756 Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Mon, 3 Oct 2022 22:51:24 -0400 Subject: [PATCH 08/12] Issue-60730: Adding in test to validate that the fix works as expected --- test/Constraints/optional.swift | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/Constraints/optional.swift b/test/Constraints/optional.swift index 8f59203edf978..d958530f23ab4 100644 --- a/test/Constraints/optional.swift +++ b/test/Constraints/optional.swift @@ -565,3 +565,18 @@ func testFunctionContainerMethodCall(container: FunctionContainer?) { // expected-note@-3 {{coalesce}} // expected-note@-4 {{force-unwrap}} } + +// Test for Issue: 60730 (https://github.com/apple/swift/issues/60730) +// Rdar: rdar://94037733 +func issue60730Test() { + struct S: P {} + func takesP(_: any P) {} + func passOptional(value: (any P)?) { + takesP(value) + // Add full error message + // Don't include the error message yet - run it and see the test fail first, and go from there. + // expected-error@-1 {{value of optional type '(any P)?' must be unwrapped to a value of type 'any P'}} + // expected-note@-3 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} + // expected-note@-4 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} + } +} \ No newline at end of file From d4e2cb7cc498139d4ed1da29801f4040b807e379 Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Tue, 4 Oct 2022 18:40:14 -0400 Subject: [PATCH 09/12] Issue-60730: Cleaning up test based on PR Comments --- test/Constraints/optional.swift | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/Constraints/optional.swift b/test/Constraints/optional.swift index d958530f23ab4..6b46b08b8f703 100644 --- a/test/Constraints/optional.swift +++ b/test/Constraints/optional.swift @@ -566,15 +566,13 @@ func testFunctionContainerMethodCall(container: FunctionContainer?) { // expected-note@-4 {{force-unwrap}} } -// Test for Issue: 60730 (https://github.com/apple/swift/issues/60730) -// Rdar: rdar://94037733 -func issue60730Test() { +// Test for https://github.com/apple/swift/issues/60730 +// rdar://94037733 +do { struct S: P {} func takesP(_: any P) {} func passOptional(value: (any P)?) { takesP(value) - // Add full error message - // Don't include the error message yet - run it and see the test fail first, and go from there. // expected-error@-1 {{value of optional type '(any P)?' must be unwrapped to a value of type 'any P'}} // expected-note@-3 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} // expected-note@-4 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} From 4e9215296a31a2a142a75784605b3a70e65d6fb1 Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Wed, 5 Oct 2022 15:39:53 -0400 Subject: [PATCH 10/12] Issue-60730: Correcting typo in test --- test/Constraints/optional.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Constraints/optional.swift b/test/Constraints/optional.swift index 6b46b08b8f703..b3f3c8a0b9df3 100644 --- a/test/Constraints/optional.swift +++ b/test/Constraints/optional.swift @@ -574,7 +574,7 @@ do { func passOptional(value: (any P)?) { takesP(value) // expected-error@-1 {{value of optional type '(any P)?' must be unwrapped to a value of type 'any P'}} - // expected-note@-3 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} - // expected-note@-4 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} + // expected-note@-2 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} + // expected-note@-3 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} } } \ No newline at end of file From 9f9770ec1992d5cccb4a3c2f3c1e453c9480b5e0 Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Fri, 7 Oct 2022 15:02:02 -0400 Subject: [PATCH 11/12] Issue-60730: Adding additional test cases for layers of optionals and for non-conforming argument --- test/Constraints/optional.swift | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/Constraints/optional.swift b/test/Constraints/optional.swift index b3f3c8a0b9df3..ae39289bae1c6 100644 --- a/test/Constraints/optional.swift +++ b/test/Constraints/optional.swift @@ -577,4 +577,14 @@ do { // expected-note@-2 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} // expected-note@-3 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} } + func passLayersOfOptional(value: (any P)??) { + takesP(value) + // expected-error@-1 {{value of optional type '(any P)??' must be unwrapped to a value of type '(any P)?}} + // expected-note@-2 {{coalesce using '??' to provide a default when the optional value contains 'nil'}} + // expected-note@-3 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} + } + func passNonConformingValue(value: (any BinaryInteger)?){ + takesP(value) + // expected-error@-1 {{argument type '(any BinaryInteger)?' does not conform to expected type 'P'}} + } } \ No newline at end of file From 6811590a9374e988870c62234cdf5aedb230808c Mon Sep 17 00:00:00 2001 From: "DylanPerry5@gmail.com" Date: Fri, 7 Oct 2022 17:11:55 -0400 Subject: [PATCH 12/12] Issue-60730: Updating test to include fixme(diagnostics) for multilpe layers of force unwrap --- test/Constraints/optional.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Constraints/optional.swift b/test/Constraints/optional.swift index ae39289bae1c6..afa08adffc692 100644 --- a/test/Constraints/optional.swift +++ b/test/Constraints/optional.swift @@ -578,6 +578,7 @@ do { // expected-note@-3 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}} } func passLayersOfOptional(value: (any P)??) { + // FIXME(diagnostics): Consider recording multiple ForceUnwrap fixes based on number of optionals takesP(value) // expected-error@-1 {{value of optional type '(any P)??' must be unwrapped to a value of type '(any P)?}} // expected-note@-2 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}