From 628f5cbc2cabcb6045196f25d335cf494fc089f1 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 13 Nov 2023 22:30:26 -0800 Subject: [PATCH 1/2] [Typed throws] Support overrides that are contravariant in the thrown error --- include/swift/AST/DiagnosticsSema.def | 3 ++ include/swift/AST/ExtInfo.h | 2 +- lib/AST/Type.cpp | 3 +- lib/Sema/TypeCheckDeclOverride.cpp | 46 ++++++++++++++-- test/SILGen/typed_throws.swift | 20 +++++++ test/decl/class/typed_throws_override.swift | 58 +++++++++++++++++++++ 6 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 test/decl/class/typed_throws_override.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index f16c86ded5acf..e45f317063cc4 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -3366,6 +3366,9 @@ ERROR(override_class_declaration_in_extension,none, ERROR(override_with_more_effects,none, "cannot override non-%1 %0 with %1 %0", (DescriptiveDeclKind, StringRef)) +ERROR(override_typed_throws,none, + "%0 that throws %1 cannot override one that throws %2", + (DescriptiveDeclKind, Type, Type)) ERROR(override_throws_objc,none, "overriding a throwing @objc %select{method|initializer}0 with " "a non-throwing %select{method|initializer}0 is not supported", (bool)) diff --git a/include/swift/AST/ExtInfo.h b/include/swift/AST/ExtInfo.h index 472a9f546fe6a..c896543ea964c 100644 --- a/include/swift/AST/ExtInfo.h +++ b/include/swift/AST/ExtInfo.h @@ -544,7 +544,7 @@ class ASTExtInfoBuilder { return bits == other.bits && (useClangTypes ? (clangTypeInfo == other.clangTypeInfo) : true) && globalActor.getPointer() == other.globalActor.getPointer() && - thrownError.getPointer() == thrownError.getPointer(); + thrownError.getPointer() == other.thrownError.getPointer(); } constexpr std::tuple diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index f033ca00b7238..3d84b41f7af7e 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -3421,7 +3421,8 @@ static bool matchesFunctionType(CanAnyFunctionType fn1, CanAnyFunctionType fn2, if (matchMode.contains(TypeMatchFlags::AllowOverride)) { // Removing 'throwing' is ABI-compatible for synchronous functions, but // not for async ones. - if (ext2.isThrowing() && + if (ext2.isThrowing() && !ext1.isThrowing() && + ext2.getThrownError().isNull() && !(ext2.isAsync() && matchMode.contains(TypeMatchFlags::AllowABICompatible))) { ext1 = ext1.withThrows(true, ext2.getThrownError()); diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index 4db76bd3a9804..e63234e8a927b 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -17,6 +17,7 @@ #include "TypeCheckAvailability.h" #include "TypeCheckConcurrency.h" #include "TypeCheckDecl.h" +#include "TypeCheckEffects.h" #include "TypeCheckObjC.h" #include "TypeChecker.h" #include "swift/AST/ASTVisitor.h" @@ -2036,16 +2037,53 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) { diags.diagnose(base, diag::overridden_here); } } - // If the overriding declaration is 'throws' but the base is not, - // complain. Do the same for 'async' + + // Check effects. if (auto overrideFn = dyn_cast(override)) { - if (overrideFn->hasThrows() && - !cast(base)->hasThrows()) { + // Determine the thrown errors in the base and override declarations. + auto baseFn = cast(base); + Type overrideThrownError = + overrideFn->getEffectiveThrownErrorType().value_or(ctx.getNeverType()); + Type baseThrownError = + baseFn->getEffectiveThrownErrorType().value_or(ctx.getNeverType()); + + if (baseThrownError->hasTypeParameter()) { + auto subs = SubstitutionMap::getOverrideSubstitutions(base, override); + baseThrownError = baseThrownError.subst(subs); + baseThrownError = overrideFn->mapTypeIntoContext(baseThrownError); + } + + overrideThrownError = overrideFn->mapTypeIntoContext(overrideThrownError); + + // Check for a subtyping relationship. + switch (compareThrownErrorsForSubtyping( + overrideThrownError, baseThrownError, overrideFn)) { + case ThrownErrorSubtyping::DropsThrows: diags.diagnose(override, diag::override_with_more_effects, override->getDescriptiveKind(), "throwing"); diags.diagnose(base, diag::overridden_here); + break; + + case ThrownErrorSubtyping::Mismatch: + diags.diagnose(override, diag::override_typed_throws, + override->getDescriptiveKind(), overrideThrownError, + baseThrownError); + diags.diagnose(base, diag::overridden_here); + break; + + case ThrownErrorSubtyping::ExactMatch: + case ThrownErrorSubtyping::Subtype: + // Proper subtyping. + break; + + case ThrownErrorSubtyping::Dependent: + // Only in already ill-formed code. + assert(ctx.Diags.hadAnyError()); + break; } + // If the override is 'async' but the base declaration is not, we have a + // problem. if (overrideFn->hasAsync() && !cast(base)->hasAsync()) { diags.diagnose(override, diag::override_with_more_effects, diff --git a/test/SILGen/typed_throws.swift b/test/SILGen/typed_throws.swift index 6170e5112ab5b..78e0f107d0a71 100644 --- a/test/SILGen/typed_throws.swift +++ b/test/SILGen/typed_throws.swift @@ -126,4 +126,24 @@ open class MyClass { // CHECK-NEXT: throw_addr public init(body: () throws(E) -> Void) throws(E) { } + + func f() throws { } +} + +// CHECK-LABEL: sil_vtable MySubclass { +// CHECK-NEXT: #MyClass.init!allocator: (MyClass.Type) -> (() throws(E) -> ()) throws(E) -> MyClass : @$s12typed_throws10MySubclassC4bodyACyyxYKXE_txYKcs5ErrorRzlufC [override] +// CHECK-NEXT: #MyClass.f: (MyClass) -> () throws -> () : @$s12typed_throws10MySubclassC1fyyAA0C5ErrorOYKFAA0C5ClassCADyyKFTV [override] +// CHECK-NEXT: #MySubclass.f: (MySubclass) -> () throws(MyError) -> () : @$s12typed_throws10MySubclassC1fyyAA0C5ErrorOYKF + +class MySubclass: MyClass { + override func f() throws(MyError) { } +} + +// CHECK-LABEL: sil_vtable MySubsubclass +// CHECK-NEXT: #MyClass.init!allocator: (MyClass.Type) -> (() throws(E) -> ()) throws(E) -> MyClass : @$s12typed_throws13MySubsubclassC4bodyACyyxYKXE_txYKcs5ErrorRzlufC [override] +// CHECK-NEXT: #MyClass.f: (MyClass) -> () throws -> () : @$s12typed_throws13MySubsubclassC1fyyF [override] +// CHECK-NEXT: #MySubclass.f: (MySubclass) -> () throws(MyError) -> () : @$s12typed_throws13MySubsubclassC1fyyF [override] +// CHECK-NEXT: #MySubsubclass.f: (MySubsubclass) -> () -> () : @$s12typed_throws13MySubsubclassC1fyyF +class MySubsubclass: MySubclass { + override func f() { } } diff --git a/test/decl/class/typed_throws_override.swift b/test/decl/class/typed_throws_override.swift new file mode 100644 index 0000000000000..68d060c5025d0 --- /dev/null +++ b/test/decl/class/typed_throws_override.swift @@ -0,0 +1,58 @@ +// RUN: %target-typecheck-verify-swift -parse-as-library -enable-experimental-feature TypedThrows + +enum MyError: Error { +case failed +} + +enum HomeworkError: Error { +case dogAteIt +} + +class SuperError: Error { } +class SubError: SuperError { } + +class Super { + func f() throws { } + func g() throws(MyError) { } + func h() throws(HomeworkError) { } // expected-note{{overridden declaration is here}} + func i() throws(SuperError) { } + + var propF: Int { + get throws { 5 } + } + + var propG: Int { + get throws(MyError) { 5 } + } + + var propH: Int { + get throws(HomeworkError) { 5 } // expected-note{{overridden declaration is here}} + } + + var propI: Int { + get throws(SuperError) { 5 } + } +} + +class Sub: Super { + override func f() throws(MyError) { } // okay to make type more specific + override func g() { } // okay to be non-throwing + override func h() throws(MyError) { } // expected-error{{instance method that throws 'MyError' cannot override one that throws 'HomeworkError'}} + override func i() throws(SubError) { } // okay to have a subtype + + override var propF: Int { + get throws(MyError) { 5 } // okay to make type more specific + } + + override var propG: Int { + get { 5 } // okay to be non-throwing + } + + override var propH: Int { + get throws(MyError) { 5 } // expected-error{{getter that throws 'MyError' cannot override one that throws 'HomeworkError'}} + } + + override var propI: Int { + get throws(SubError) { 5 } // okay to make type more specific + } +} From d4e1558dd9082cda2e5de389323759e1267f3fbb Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 15 Nov 2023 22:36:06 -0800 Subject: [PATCH 2/2] Deal with tests that don't import the Swift standard library --- lib/Sema/TypeCheckDeclOverride.cpp | 5 +++-- lib/Sema/TypeCheckEffects.cpp | 7 +++++++ test/IRGen/protocol_synthesized.swift | 3 +++ test/SIL/OwnershipVerifier/definite_init.sil | 3 +++ 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index e63234e8a927b..7cb1c728c88f4 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -2047,13 +2047,14 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) { Type baseThrownError = baseFn->getEffectiveThrownErrorType().value_or(ctx.getNeverType()); - if (baseThrownError->hasTypeParameter()) { + if (baseThrownError && baseThrownError->hasTypeParameter()) { auto subs = SubstitutionMap::getOverrideSubstitutions(base, override); baseThrownError = baseThrownError.subst(subs); baseThrownError = overrideFn->mapTypeIntoContext(baseThrownError); } - overrideThrownError = overrideFn->mapTypeIntoContext(overrideThrownError); + if (overrideThrownError) + overrideThrownError = overrideFn->mapTypeIntoContext(overrideThrownError); // Check for a subtyping relationship. switch (compareThrownErrorsForSubtyping( diff --git a/lib/Sema/TypeCheckEffects.cpp b/lib/Sema/TypeCheckEffects.cpp index 3a7d00bc7c7b0..bce6616453579 100644 --- a/lib/Sema/TypeCheckEffects.cpp +++ b/lib/Sema/TypeCheckEffects.cpp @@ -3595,6 +3595,13 @@ ThrownErrorSubtyping swift::compareThrownErrorsForSubtyping( Type subThrownError, Type superThrownError, DeclContext *dc ) { + // Deal with NULL errors. This should only occur when there is no standard + // library. + if (!subThrownError || !superThrownError) { + assert(!dc->getASTContext().getStdlibModule() && "NULL thrown error type"); + return ThrownErrorSubtyping::ExactMatch; + } + // Easy case: exact match. if (superThrownError->isEqual(subThrownError)) return ThrownErrorSubtyping::ExactMatch; diff --git a/test/IRGen/protocol_synthesized.swift b/test/IRGen/protocol_synthesized.swift index f21979914709e..6fdb1955a7595 100644 --- a/test/IRGen/protocol_synthesized.swift +++ b/test/IRGen/protocol_synthesized.swift @@ -10,6 +10,9 @@ import SynthesizedProtocol +enum Never { } +protocol Error { } + // CHECK: @"$sSo5Flagsas9OptionSetSCMc" = linkonce_odr hidden constant { i32, i32, i32, i32, i16, i16, i32, i32 } { i32 {{(trunc \(i64 )?}}sub ({{i(32|64)}} ptrtoint (ptr @"$ss9OptionSetMp" to {{i(32|64)}}), {{i(32|64)}} ptrtoint (ptr @"$sSo5Flagsas9OptionSetSCMc" to {{i(32|64)}})){{( to i32\))?}}, i32 {{(trunc \(i64 )?}}sub ({{i(32|64)}} ptrtoint (ptr @"$sSo5FlagsaMn" to {{i(32|64)}}), {{i(32|64)}} ptrtoint (ptr getelementptr inbounds ({ i32, i32, i32, i32, i16, i16, i32, i32 }, ptr @"$sSo5Flagsas9OptionSetSCMc", i32 0, i32 1) to {{i(32|64)}})){{( to i32\))?}}, i32 {{(trunc \(i64 )?}}sub ({{i(32|64)}} ptrtoint (ptr @"$sSo5Flagsas9OptionSetSCWP" to {{i(32|64)}}), {{i(32|64)}} ptrtoint (ptr getelementptr inbounds ({ i32, i32, i32, i32, i16, i16, i32, i32 }, ptr @"$sSo5Flagsas9OptionSetSCMc", i32 0, i32 2) to {{i(32|64)}})){{( to i32\))?}}, i32 131200, i16 3, i16 1, i32 {{(trunc \(i64 )?}}sub ({{i(32|64)}} ptrtoint (ptr @"$sSo5Flagsas9OptionSetSCWI" to {{i(32|64)}}), {{i(32|64)}} ptrtoint (ptr getelementptr inbounds ({ i32, i32, i32, i32, i16, i16, i32, i32 }, ptr @"$sSo5Flagsas9OptionSetSCMc", i32 0, i32 6) to {{i(32|64)}})){{( to i32\))?}}, i32 {{(trunc \(i64 )?}}sub ({{i(32|64)}} ptrtoint (ptr @"$sSo5Flagsas9OptionSetSCMcMK" to {{i(32|64)}}), {{i(32|64)}} ptrtoint (ptr getelementptr inbounds ({ i32, i32, i32, i32, i16, i16, i32, i32 }, ptr @"$sSo5Flagsas9OptionSetSCMc", i32 0, i32 7) to {{i(32|64)}})) {{(to i32\) )?}}}, section "{{[^"]*}}"{{(, comdat)?}},{{.*}} align 4 // Triggers the inclusion of the relevant ProtocolConformanceDescriptor diff --git a/test/SIL/OwnershipVerifier/definite_init.sil b/test/SIL/OwnershipVerifier/definite_init.sil index e2a25d56e1836..f557e5e30b68d 100644 --- a/test/SIL/OwnershipVerifier/definite_init.sil +++ b/test/SIL/OwnershipVerifier/definite_init.sil @@ -5,6 +5,9 @@ sil_stage raw import Builtin +enum Never { } +protocol Error { } + enum Optional { case some(T) case none