Skip to content

Commit 9bb71ff

Browse files
committed
Add @objcImpl vtable fallback diagnostic
Nothing in an `@_objcImplementation` block—including Swift-only declarations—should ever require a vtable entry. Diagnose any such members. The diagnostic emitted here is intended to be a fallback that will be used when a vtable entry is needed but we don’t know the specific reason. A future commit will add a more specific diagnostic for Swift-only non-convenience inits.
1 parent 231de1a commit 9bb71ff

File tree

4 files changed

+45
-4
lines changed

4 files changed

+45
-4
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1841,6 +1841,11 @@ NOTE(objc_implementation_requirement_here,none,
18411841
"%kind0 declared in header here",
18421842
(ValueDecl *))
18431843

1844+
ERROR(objc_implementation_member_requires_vtable, none,
1845+
"%kind0 is not valid in an '@_objcImplementation' extension because it "
1846+
"is an overridable Swift-only %kindonly0",
1847+
(const ValueDecl *))
1848+
18441849
ERROR(objc_implementation_multiple_matching_requirements,none,
18451850
"%kind0 could match several different members declared in the header",
18461851
(ValueDecl *))

lib/Sema/TypeCheckDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1173,7 +1173,7 @@ NeedsNewVTableEntryRequest::evaluate(Evaluator &evaluator,
11731173
AbstractFunctionDecl *decl) const {
11741174
auto *dc = decl->getDeclContext();
11751175

1176-
if (!isa<ClassDecl>(dc))
1176+
if (!isa<ClassDecl>(dc->getImplementedObjCContext()))
11771177
return false;
11781178

11791179
// Destructors always use a fixed vtable entry.

lib/Sema/TypeCheckDeclObjC.cpp

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2958,6 +2958,36 @@ class ObjCImplementationChecker {
29582958
return nullptr;
29592959
}
29602960

2961+
void diagnoseVTableUse(ValueDecl *member) {
2962+
if (auto storage = dyn_cast<AbstractStorageDecl>(member)) {
2963+
for (auto accessor : storage->getAllAccessors()) {
2964+
diagnoseVTableUse(accessor);
2965+
}
2966+
return;
2967+
}
2968+
2969+
auto afd = dyn_cast_or_null<AbstractFunctionDecl>(member);
2970+
if (!afd || !afd->needsNewVTableEntry())
2971+
return;
2972+
2973+
// Don't diagnose if we already diagnosed an unrelated ObjC interop issue,
2974+
// like an un-representable type. If there's an `@objc` attribute on the
2975+
// member, this will be indicated by its `isInvalid()` bit; otherwise we'll
2976+
// use the enclosing extension's `@_objcImplementation` attribute.
2977+
DeclAttribute *attr = afd->getAttrs()
2978+
.getAttribute<ObjCAttr>(/*AllowInvalid=*/true);
2979+
if (!attr)
2980+
attr = member->getDeclContext()->getAsDecl()->getAttrs()
2981+
.getAttribute<ObjCImplementationAttr>(/*AllowInvalid=*/true);
2982+
assert(attr && "expected @_objcImplementation on context of member checked "
2983+
"by ObjCImplementationChecker");
2984+
if (attr->isInvalid())
2985+
return;
2986+
2987+
// Emit a vague fallback diagnostic.
2988+
diagnose(afd, diag::objc_implementation_member_requires_vtable, afd);
2989+
}
2990+
29612991
void addRequirements(IterableDeclContext *idc) {
29622992
assert(idc->getDecl()->hasClangNode());
29632993
for (Decl *_member : idc->getMembers()) {
@@ -2998,8 +3028,12 @@ class ObjCImplementationChecker {
29983028

29993029
// Skip non-member implementations.
30003030
// FIXME: Should we consider them if they were only rejected for privacy?
3001-
if (!member->isObjCMemberImplementation())
3031+
if (!member->isObjCMemberImplementation()) {
3032+
// No member of an `@_objcImplementation` extension should need a vtable
3033+
// entry.
3034+
diagnoseVTableUse(member);
30023035
continue;
3036+
}
30033037

30043038
// `getExplicitObjCName()` is O(N) and would otherwise be used repeatedly
30053039
// in `matchRequirementsAtThreshold()`, so just precompute it.
@@ -3383,6 +3417,8 @@ class ObjCImplementationChecker {
33833417
case MatchOutcome::Match:
33843418
case MatchOutcome::MatchWithExplicitObjCName:
33853419
// Successful outcomes!
3420+
// If this member will require a vtable entry, diagnose that now.
3421+
diagnoseVTableUse(cand);
33863422
return;
33873423

33883424
case MatchOutcome::WrongImplicitObjCName:

test/decl/ext/objc_implementation.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,11 @@ protocol EmptySwiftProto {}
203203
}
204204

205205
@nonobjc public init(notFromHeader4: CInt) {
206-
// FIXME: Should be an error; requires a vtable entry
206+
// expected-warning@-1 {{initializer 'init(notFromHeader4:)' is not valid in an '@_objcImplementation' extension because it is an overridable Swift-only initializer}}
207207
}
208208

209209
@nonobjc public required init(notFromHeader5: CInt) {
210-
// FIXME: Should be an error; requires a vtable entry
210+
// expected-warning@-1 {{initializer 'init(notFromHeader5:)' is not valid in an '@_objcImplementation' extension because it is an overridable Swift-only initializer}}
211211
}
212212

213213
@nonobjc public convenience init(notFromHeader6: CInt) {

0 commit comments

Comments
 (0)