Skip to content

[5.9] Re-merge #63668: Improve @objcImplementation member checking #64790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,10 @@ class ExtensionDecl final : public GenericContext, public Decl,
/// \c isObjCImplementation() returns \c true.
Optional<Identifier> getCategoryNameForObjCImplementation() const;

/// If this extension represents an imported Objective-C category, returns the
/// category's name. Otherwise returns the empty identifier.
Identifier getObjCCategoryName() const;

// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) {
return D->getKind() == DeclKind::Extension;
Expand Down
48 changes: 45 additions & 3 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1602,14 +1602,56 @@ ERROR(member_of_objc_implementation_not_objc_or_final,none,
"%0 %1 does not match any %0 declared in the headers for %2; did you use "
"the %0's Swift name?",
(DescriptiveDeclKind, ValueDecl *, ValueDecl *))
NOTE(fixit_add_objc_for_objc_implementation,none,
"add '@objc' to define an Objective-C-compatible %0 not declared in "
"the header",
NOTE(fixit_add_private_for_objc_implementation,none,
"add 'private' or 'fileprivate' to define an Objective-C-compatible %0 "
"not declared in the header",
(DescriptiveDeclKind))
NOTE(fixit_add_final_for_objc_implementation,none,
"add 'final' to define a Swift %0 that cannot be overridden",
(DescriptiveDeclKind))

ERROR(objc_implementation_wrong_category,none,
"%0 %1 should be implemented in extension for "
"%select{main class interface|category %2}2, not "
"%select{main class interface|category %3}3",
(DescriptiveDeclKind, ValueDecl *, Identifier, Identifier))

ERROR(objc_implementation_wrong_objc_name,none,
"selector %0 for %1 %2 not found in header; did you mean %3?",
(ObjCSelector, DescriptiveDeclKind, ValueDecl *, ObjCSelector))

ERROR(objc_implementation_wrong_swift_name,none,
"selector %0 used in header by an %1 with a different name; did you "
"mean %2?",
(ObjCSelector, DescriptiveDeclKind, DeclName))

ERROR(objc_implementation_missing_impl,none,
"extension for %select{main class interface|category %0}0 should "
"provide implementation for %1 %2",
(Identifier, DescriptiveDeclKind, ValueDecl *))

ERROR(objc_implementation_class_or_instance_mismatch,none,
"%0 %1 does not match %2 declared in header",
(DescriptiveDeclKind, ValueDecl *, DescriptiveDeclKind))

ERROR(objc_implementation_multiple_matching_candidates,none,
"found multiple implementations that could match %0 %1 with selector %2",
(DescriptiveDeclKind, ValueDecl *, ObjCSelector))
NOTE(objc_implementation_candidate_impl_here,none,
"%0 %1 is a potential match%select{|; insert '@objc(%3)' to use it}2",
(DescriptiveDeclKind, ValueDecl *, bool, StringRef))
NOTE(objc_implementation_requirement_here,none,
"%0 %1 declared in header here",
(DescriptiveDeclKind, ValueDecl *))

ERROR(objc_implementation_multiple_matching_requirements,none,
"%0 %1 could match several different members declared in the header",
(DescriptiveDeclKind, ValueDecl *))
NOTE(objc_implementation_one_matched_requirement,none,
"%0 %1 (with selector %2) is a potential match%select{|; insert "
"'@objc(%4)' to use it}3",
(DescriptiveDeclKind, ValueDecl *, ObjCSelector, bool, StringRef))

ERROR(cdecl_not_at_top_level,none,
"@_cdecl can only be applied to global functions", ())
ERROR(cdecl_empty_name,none,
Expand Down
22 changes: 22 additions & 0 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -4166,6 +4166,28 @@ class LocalDiscriminatorsRequest
bool isCached() const { return true; }
};

/// Checks that all of a class's \c \@objcImplementation extensions provide
/// complete and correct implementations for their corresponding interfaces.
/// This is done on all of a class's implementations at once to improve diagnostics.
class TypeCheckObjCImplementationRequest
: public SimpleRequest<TypeCheckObjCImplementationRequest,
evaluator::SideEffect(ExtensionDecl *),
RequestFlags::Cached> {
public:
using SimpleRequest::SimpleRequest;

private:
friend SimpleRequest;

// Evaluation.
evaluator::SideEffect
evaluate(Evaluator &evaluator, ExtensionDecl *ED) const;

public:
// Separate caching.
bool isCached() const { return true; }
};

void simple_display(llvm::raw_ostream &out, ASTNode node);
void simple_display(llvm::raw_ostream &out, Type value);
void simple_display(llvm::raw_ostream &out, const TypeRepr *TyR);
Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -466,3 +466,6 @@ SWIFT_REQUEST(TypeChecker, GetRuntimeDiscoverableAttributes,
SWIFT_REQUEST(TypeChecker, LocalDiscriminatorsRequest,
unsigned(DeclContext *),
Cached, NoLocationInfo)
SWIFT_REQUEST(TypeChecker, TypeCheckObjCImplementationRequest,
unsigned(ExtensionDecl *),
Cached, NoLocationInfo)
28 changes: 22 additions & 6 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ static bool contributesToParentTypeStorage(const AbstractStorageDecl *ASD) {
return !ND->isResilient() && ASD->hasStorage() && !ASD->isStatic();
}

static bool isInObjCImpl(const ValueDecl *VD) {
auto *ED = dyn_cast<ExtensionDecl>(VD->getDeclContext());
return ED && ED->isObjCImplementation();
}

PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
bool preferTypeRepr,
bool printFullConvention,
Expand Down Expand Up @@ -208,9 +213,9 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
if (!options.PrintSPIs && D->isSPI())
return false;

// Skip anything that isn't 'public' or '@usableFromInline' or has a
// _specialize attribute with a targetFunction parameter.
if (auto *VD = dyn_cast<ValueDecl>(D)) {
// Skip anything that isn't 'public' or '@usableFromInline' or has a
// _specialize attribute with a targetFunction parameter.
if (!isPublicOrUsableFromInline(VD) &&
!isPrespecilizationDeclWithTarget(VD)) {
// We do want to print private stored properties, without their
Expand All @@ -221,6 +226,13 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,

return false;
}

// Skip member implementations and @objc overrides in @objcImpl
// extensions.
if (VD->isObjCMemberImplementation()
|| (isInObjCImpl(VD) && VD->getOverriddenDecl() && VD->isObjC())) {
return false;
}
}

// Skip extensions that extend things we wouldn't print.
Expand Down Expand Up @@ -312,6 +324,7 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
DAK_AccessControl,
DAK_SetterAccess,
DAK_Lazy,
DAK_ObjCImplementation,
DAK_StaticInitializeObjCMetadata,
DAK_RestatedObjCConformance,
DAK_NonSendable,
Expand Down Expand Up @@ -1125,8 +1138,10 @@ void PrintAST::printAttributes(const Decl *D) {

if (auto vd = dyn_cast<VarDecl>(D)) {
// Don't print @_hasInitialValue if we're printing an initializer
// expression or if the storage is resilient.
if (vd->isInitExposedToClients() || vd->isResilient())
// expression, if the storage is resilient, or if it's in an
// @objcImplementation extension (where final properties should appear
// computed).
if (vd->isInitExposedToClients() || vd->isResilient() || isInObjCImpl(vd))
Options.ExcludeAttrList.push_back(DAK_HasInitialValue);

if (!Options.PrintForSIL) {
Expand Down Expand Up @@ -2105,8 +2120,9 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) {
// Don't print accessors for trivially stored properties...
if (impl.isSimpleStored()) {
// ...unless we're printing for SIL, which expects a { get set? } on
// trivial properties
if (Options.PrintForSIL) {
// trivial properties, or in an @objcImpl extension, which treats
// final stored properties as computed.
if (Options.PrintForSIL || isInObjCImpl(ASD)) {
Printer << " { get " << (impl.supportsMutation() ? "set }" : "}");
}
// ...or you're private/internal(set), at which point we'll print
Expand Down
4 changes: 3 additions & 1 deletion lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5361,8 +5361,10 @@ synthesizeEmptyFunctionBody(AbstractFunctionDecl *afd, void *context) {

DestructorDecl *
GetDestructorRequest::evaluate(Evaluator &evaluator, ClassDecl *CD) const {
auto dc = CD->getImplementationContext();

auto &ctx = CD->getASTContext();
auto *DD = new (ctx) DestructorDecl(CD->getLoc(), CD);
auto *DD = new (ctx) DestructorDecl(CD->getLoc(), dc->getAsGenericContext());

DD->setImplicit();

Expand Down
2 changes: 1 addition & 1 deletion lib/AST/NameLookupRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ Optional<DestructorDecl *> GetDestructorRequest::getCachedResult() const {

void GetDestructorRequest::cacheResult(DestructorDecl *value) const {
auto *classDecl = std::get<0>(getStorage());
classDecl->addMember(value);
classDecl->getImplementationContext()->addMember(value);
}

//----------------------------------------------------------------------------//
Expand Down
10 changes: 5 additions & 5 deletions lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5183,20 +5183,20 @@ findImplsGivenInterface(ClassDecl *classDecl, Identifier categoryName) {
return impls;
}

static Identifier getCategoryName(ExtensionDecl *ext) {
Identifier ExtensionDecl::getObjCCategoryName() const {
// Could it be an imported category?
if (!ext || !ext->hasClangNode())
if (!hasClangNode())
// Nope, not imported.
return Identifier();

auto category = dyn_cast<clang::ObjCCategoryDecl>(ext->getClangDecl());
auto category = dyn_cast<clang::ObjCCategoryDecl>(getClangDecl());
if (!category)
// Nope, not a category.
return Identifier();

// We'll look for an implementation with this category name.
auto clangCategoryName = category->getName();
return ext->getASTContext().getIdentifier(clangCategoryName);
return getASTContext().getIdentifier(clangCategoryName);
}

static IterableDeclContext *
Expand Down Expand Up @@ -5246,7 +5246,7 @@ findContextInterfaceAndImplementation(DeclContext *dc) {

// Is this an imported category? If so, extract its name so we can look for
// implementations of that category.
categoryName = getCategoryName(ext);
categoryName = ext->getObjCCategoryName();
if (categoryName.empty())
return {};

Expand Down
32 changes: 32 additions & 0 deletions lib/Sema/CodeSynthesis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,29 @@ static bool canInheritDesignatedInits(Evaluator &eval, ClassDecl *decl) {
areAllStoredPropertiesDefaultInitializable(eval, decl);
}

static ValueDecl *findImplementedObjCDecl(ValueDecl *VD) {
// If VD has an ObjC name...
if (auto vdSelector = VD->getObjCRuntimeName()) {
// and it's in an extension...
if (auto implED = dyn_cast<ExtensionDecl>(VD->getDeclContext())) {
// and that extension is the @objcImplementation of a class's main body...
if (auto interfaceCD =
dyn_cast_or_null<ClassDecl>(implED->getImplementedObjCDecl())) {
// Find the initializer in the class's main body that matches VD.
for (auto interfaceVD : interfaceCD->getAllMembers()) {
if (auto interfaceCtor = dyn_cast<ConstructorDecl>(interfaceVD)) {
if (vdSelector == interfaceCtor->getObjCRuntimeName()) {
return interfaceCtor;
}
}
}
}
}
}

return VD;
}

static void collectNonOveriddenSuperclassInits(
ClassDecl *subclass, SmallVectorImpl<ConstructorDecl *> &results) {
auto *superclassDecl = subclass->getSuperclassDecl();
Expand Down Expand Up @@ -968,6 +991,15 @@ static void collectNonOveriddenSuperclassInits(
subOptions, lookupResults);

for (auto decl : lookupResults) {
// HACK: If an @objcImplementation extension declares an initializer, its
// interface usually also has a declaration. We need the interface decl for
// access control computations, but the name lookup returns the
// implementation decl because it's in the Swift module. Go find the
// matching interface decl.
// (Note that this is necessary for both newly-declared inits and overrides,
// even implicit ones.)
decl = findImplementedObjCDecl(decl);

auto superclassCtor = cast<ConstructorDecl>(decl);

// Skip invalid superclass initializers.
Expand Down
Loading