From b14c00d521e6637a9378e5e2cdb98fa144ba8315 Mon Sep 17 00:00:00 2001 From: Becca Royal-Gordon Date: Wed, 31 May 2023 17:49:25 -0700 Subject: [PATCH] Support required inits in @objcImpl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Allow `required init`s in @objcImpl extensions of a class’s main body • Validate that the presence or absence of a `required` modifier matches the imported header declaration. Fixes rdar://110016760. --- include/swift/AST/DiagnosticsSema.def | 5 +++++ lib/Sema/TypeCheckAttr.cpp | 2 +- lib/Sema/TypeCheckDeclObjC.cpp | 21 +++++++++++++++++++ test/decl/ext/Inputs/objc_implementation.h | 11 +++++++++- test/decl/ext/objc_implementation.swift | 15 +++++++++++++ .../ext/objc_implementation_conflicts.swift | 18 ++++++++++++++++ 6 files changed, 70 insertions(+), 2 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 4944736c3962a..b96f78106d942 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -1663,6 +1663,11 @@ ERROR(objc_implementation_type_mismatch,none, "header", (DescriptiveDeclKind, ValueDecl *, Type, Type)) +ERROR(objc_implementation_required_attr_mismatch,none, + "%0 %1 %select{should not|should}2 be 'required' to match %0 declared by " + "the header", + (DescriptiveDeclKind, ValueDecl *, bool)) + ERROR(objc_implementation_wrong_objc_name,none, "selector %0 for %1 %2 not found in header; did you mean %3?", (ObjCSelector, DescriptiveDeclKind, ValueDecl *, ObjCSelector)) diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 93ca4513b7c60..edec9c35fc4cb 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -2717,7 +2717,7 @@ void AttributeChecker::visitRequiredAttr(RequiredAttr *attr) { // The constructor must be declared within the class itself. // FIXME: Allow an SDK overlay to add a required initializer to a class // defined in Objective-C - if (!isa(ctor->getDeclContext()) && + if (!isa(ctor->getDeclContext()->getImplementedObjCContext()) && !isObjCClassExtensionInOverlay(ctor->getDeclContext())) { diagnose(ctor, diag::required_initializer_in_extension, parentTy) .highlight(attr->getLocation()); diff --git a/lib/Sema/TypeCheckDeclObjC.cpp b/lib/Sema/TypeCheckDeclObjC.cpp index 09501281c6aaa..d4dc2e171f32c 100644 --- a/lib/Sema/TypeCheckDeclObjC.cpp +++ b/lib/Sema/TypeCheckDeclObjC.cpp @@ -3015,6 +3015,7 @@ class ObjCImplementationChecker { WrongDeclKind, WrongType, WrongWritability, + WrongRequiredAttr, Match, MatchWithExplicitObjCName, @@ -3312,6 +3313,10 @@ class ObjCImplementationChecker { !cast(cand)->isSettable(nullptr)) return MatchOutcome::WrongWritability; + if (auto reqCtor = dyn_cast(req)) + if (reqCtor->isRequired() != cast(cand)->isRequired()) + return MatchOutcome::WrongRequiredAttr; + // If we got here, everything matched. But at what quality? if (explicitObjCName) return MatchOutcome::MatchWithExplicitObjCName; @@ -3399,6 +3404,22 @@ class ObjCImplementationChecker { diagnose(cand, diag::objc_implementation_must_be_settable, cand->getDescriptiveKind(), cand, req->getDescriptiveKind()); return; + + case MatchOutcome::WrongRequiredAttr: { + bool shouldBeRequired = cast(req)->isRequired(); + + auto diag = + diagnose(cand, diag::objc_implementation_required_attr_mismatch, + cand->getDescriptiveKind(), cand, shouldBeRequired); + + if (shouldBeRequired) + diag.fixItInsert(cand->getAttributeInsertionLoc(/*forModifier=*/true), + "required "); + else + diag.fixItRemove(cand->getAttrs().getAttribute() + ->getLocation()); + return; + } } llvm_unreachable("Unknown MatchOutcome"); diff --git a/test/decl/ext/Inputs/objc_implementation.h b/test/decl/ext/Inputs/objc_implementation.h index f6fc4856df29f..dac9bf6ad6ebe 100644 --- a/test/decl/ext/Inputs/objc_implementation.h +++ b/test/decl/ext/Inputs/objc_implementation.h @@ -12,7 +12,16 @@ @end -@interface ObjCClass : ObjCBaseClass +@protocol ObjCProto + +- (instancetype)initFromProtocol1:(int)param; +- (instancetype)initFromProtocol2:(int)param; + +@end + +@interface ObjCClass : ObjCBaseClass + +- (instancetype)initNotFromProtocol:(int)param; - (void)methodFromHeader1:(int)param; - (void)methodFromHeader2:(int)param; diff --git a/test/decl/ext/objc_implementation.swift b/test/decl/ext/objc_implementation.swift index ba0cba881b161..9c27e5a502a55 100644 --- a/test/decl/ext/objc_implementation.swift +++ b/test/decl/ext/objc_implementation.swift @@ -145,6 +145,21 @@ // OK } + @objc(initFromProtocol1:) + required public init?(fromProtocol1: CInt) { + // OK + } + + @objc(initFromProtocol2:) + public init?(fromProtocol2: CInt) { + // expected-error@-1 {{initializer 'init(fromProtocol2:)' should be 'required' to match initializer declared by the header}} {{3-3=required }} + } + + @objc(initNotFromProtocol:) + required public init?(notFromProtocol: CInt) { + // expected-error@-1 {{initializer 'init(notFromProtocol:)' should not be 'required' to match initializer declared by the header}} {{3-12=}} + } + class func classMethod1(_: CInt) { // OK } diff --git a/test/decl/ext/objc_implementation_conflicts.swift b/test/decl/ext/objc_implementation_conflicts.swift index 3fbd480d8c78c..c17348e6f47f3 100644 --- a/test/decl/ext/objc_implementation_conflicts.swift +++ b/test/decl/ext/objc_implementation_conflicts.swift @@ -146,6 +146,24 @@ import objc_implementation_private super.init(fromSuperclass2: v) } + @objc(initFromProtocol1:) + required public init?(fromProtocol1 v: CInt) { + // OK + super.init(fromSuperclass: v) + } + + @objc(initFromProtocol2:) + required public init?(fromProtocol2 v: CInt) { + // OK + super.init(fromSuperclass: v) + } + + @objc(initNotFromProtocol:) + public init?(notFromProtocol v: CInt) { + // OK + super.init(fromSuperclass: v) + } + class func classMethod1(_: CInt) {} class func classMethod2(_: CInt) {} class func classMethod3(_: CInt) {}