Skip to content

Commit fc81208

Browse files
beccadaxbeccadax
authored
Merge pull request #75919 from beccadax/objcimpl-selector-crasher
2 parents b434c9f + a0842b5 commit fc81208

File tree

6 files changed

+45
-40
lines changed

6 files changed

+45
-40
lines changed

‎lib/Sema/TypeCheckAttr.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1612,7 +1612,8 @@ visitObjCImplementationAttr(ObjCImplementationAttr *attr) {
16121612
auto argListRange = getArgListRange(Ctx, attr);
16131613
if (argListRange.isValid()) {
16141614
diag.fixItRemove(argListRange);
1615-
fixDeclarationObjCName(diag, ED, objcLangAttr->getName(),
1615+
fixDeclarationObjCName(diag, ED,
1616+
objcLangAttr->getName().value_or(ObjCSelector()),
16161617
correctSelector);
16171618
}
16181619
objcLangAttr->setName(correctSelector, /*implicit=*/false);

‎lib/Sema/TypeCheckDeclObjC.cpp‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,7 +1749,7 @@ static ObjCSelector inferObjCName(ValueDecl *decl) {
17491749
diag::objc_override_method_selector_mismatch,
17501750
*attr->getName(), overriddenSelector)
17511751
.limitBehavior(behavior));
1752-
fixDeclarationObjCName(diag, decl, attr->getName(),
1752+
fixDeclarationObjCName(diag, decl, attr->getName().value(),
17531753
overriddenSelector);
17541754
}
17551755

@@ -1839,8 +1839,9 @@ static ObjCSelector inferObjCName(ValueDecl *decl) {
18391839
auto diag = decl->diagnose(diag::objc_ambiguous_inference_candidate,
18401840
req, proto, *req->getObjCRuntimeName());
18411841
fixDeclarationObjCName(diag, decl,
1842-
decl->getObjCRuntimeName(/*skipIsObjC=*/true),
1843-
req->getObjCRuntimeName());
1842+
decl->getObjCRuntimeName(/*skipIsObjC=*/true)
1843+
.value_or(ObjCSelector()),
1844+
req->getObjCRuntimeName().value());
18441845
};
18451846
diagnoseCandidate(firstReq);
18461847
diagnoseCandidate(req);
@@ -2151,8 +2152,8 @@ bool swift::fixDeclarationName(InFlightDiagnostic &diag, const ValueDecl *decl,
21512152

21522153
bool swift::fixDeclarationObjCName(InFlightDiagnostic &diag,
21532154
const Decl *decl,
2154-
std::optional<ObjCSelector> nameOpt,
2155-
std::optional<ObjCSelector> targetNameOpt,
2155+
ObjCSelector name,
2156+
ObjCSelector targetName,
21562157
bool ignoreImpliedName) {
21572158
if (decl->isImplicit())
21582159
return false;
@@ -2164,9 +2165,6 @@ bool swift::fixDeclarationObjCName(InFlightDiagnostic &diag,
21642165
return false;
21652166
}
21662167

2167-
auto name = *nameOpt;
2168-
auto targetName = *targetNameOpt;
2169-
21702168
// Dig out the existing '@objc' attribute on the witness. We don't care
21712169
// about implicit ones because they don't have useful source location
21722170
// information.
@@ -2213,7 +2211,7 @@ bool swift::fixDeclarationObjCName(InFlightDiagnostic &diag,
22132211

22142212
// If the names of the witness and requirement differ, we need to
22152213
// specify the name.
2216-
if (name != targetName || ignoreImpliedName) {
2214+
if (targetName && (name != targetName || ignoreImpliedName)) {
22172215
out << "(";
22182216
out << targetName;
22192217
out << ")";
@@ -2908,8 +2906,10 @@ bool swift::diagnoseObjCUnsatisfiedOptReqConflicts(SourceFile &sf) {
29082906
// Fix the '@objc' attribute, if needed.
29092907
if (!bestConflict->canInferObjCFromRequirement(req))
29102908
fixDeclarationObjCName(diag, bestConflict,
2911-
bestConflict->getObjCRuntimeName(),
2912-
req->getObjCRuntimeName(),
2909+
bestConflict->getObjCRuntimeName()
2910+
.value_or(ObjCSelector()),
2911+
req->getObjCRuntimeName()
2912+
.value_or(ObjCSelector()),
29132913
/*ignoreImpliedName=*/true);
29142914
}
29152915

@@ -3278,12 +3278,12 @@ class ObjCImplementationChecker {
32783278
return ObjCSelector();
32793279
}
32803280

3281-
static std::optional<ObjCSelector> getObjCName(ValueDecl *VD) {
3281+
static ObjCSelector getObjCName(ValueDecl *VD) {
32823282
if (!VD->getCDeclName().empty()) {
32833283
auto ident = VD->getASTContext().getIdentifier(VD->getCDeclName());
32843284
return ObjCSelector(VD->getASTContext(), 0, { ident });
32853285
}
3286-
return VD->getObjCRuntimeName();
3286+
return VD->getObjCRuntimeName().value_or(ObjCSelector());
32873287
}
32883288

32893289
public:
@@ -3428,8 +3428,8 @@ class ObjCImplementationChecker {
34283428
for (auto req : matchedRequirements.matches) {
34293429
auto diag =
34303430
diagnose(cand, diag::objc_implementation_one_matched_requirement,
3431-
req, *getObjCName(req), shouldOfferFix,
3432-
getObjCName(req)->getString(scratch));
3431+
req, getObjCName(req), shouldOfferFix,
3432+
getObjCName(req).getString(scratch));
34333433
if (shouldOfferFix) {
34343434
fixDeclarationObjCName(diag, cand, getObjCName(cand),
34353435
getObjCName(req), /*ignoreImpliedName=*/true);
@@ -3469,14 +3469,14 @@ class ObjCImplementationChecker {
34693469
auto ext =
34703470
cast<ExtensionDecl>(reqIDC->getImplementationContext());
34713471
diagnose(ext, diag::objc_implementation_multiple_matching_candidates,
3472-
req, *getObjCName(req));
3472+
req, getObjCName(req));
34733473

34743474
for (auto cand : cands.matches) {
34753475
bool shouldOfferFix = !unmatchedCandidates[cand];
34763476
auto diag =
34773477
diagnose(cand, diag::objc_implementation_candidate_impl_here,
34783478
cand, shouldOfferFix,
3479-
getObjCName(req)->getString(scratch));
3479+
getObjCName(req).getString(scratch));
34803480

34813481
if (shouldOfferFix) {
34823482
fixDeclarationObjCName(diag, cand, getObjCName(cand),
@@ -3651,7 +3651,7 @@ class ObjCImplementationChecker {
36513651

36523652
void diagnoseOutcome(MatchOutcome outcome, ValueDecl *req, ValueDecl *cand,
36533653
ObjCSelector explicitObjCName) {
3654-
auto reqObjCName = *getObjCName(req);
3654+
auto reqObjCName = getObjCName(req);
36553655

36563656
switch (outcome) {
36573657
case MatchOutcome::NoRelationship:
@@ -3668,7 +3668,7 @@ class ObjCImplementationChecker {
36683668
case MatchOutcome::WrongImplicitObjCName:
36693669
case MatchOutcome::WrongExplicitObjCName: {
36703670
auto diag = diagnose(cand, diag::objc_implementation_wrong_objc_name,
3671-
*getObjCName(cand), cand, reqObjCName);
3671+
getObjCName(cand), cand, reqObjCName);
36723672
fixDeclarationObjCName(diag, cand, explicitObjCName, reqObjCName);
36733673
return;
36743674
}

‎lib/Sema/TypeCheckDeclPrimary.cpp‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,29 +2045,33 @@ static void applyAccessNote(ValueDecl *VD, const AccessNote &note,
20452045
});
20462046

20472047
if (note.ObjCName) {
2048+
auto newName = note.ObjCName.value();
2049+
2050+
// addOrRemoveAttr above guarantees there's an ObjCAttr on this decl.
20482051
auto attr = VD->getAttrs().getAttribute<ObjCAttr>();
20492052
assert(attr && "ObjCName set, but ObjCAttr not true or did not apply???");
20502053

20512054
if (!attr->hasName()) {
2052-
auto oldName = attr->getName();
2053-
attr->setName(*note.ObjCName, true);
2055+
// There was already an @objc attribute with no selector. Set it.
2056+
attr->setName(newName, true);
20542057

20552058
if (!ctx.LangOpts.shouldRemarkOnAccessNoteSuccess())
20562059
return;
20572060

20582061
VD->diagnose(diag::attr_objc_name_changed_by_access_note,
2059-
notes.Reason, VD->getDescriptiveKind(), *note.ObjCName);
2062+
notes.Reason, VD->getDescriptiveKind(), newName);
20602063

20612064
auto fixIt =
20622065
VD->diagnose(diag::fixit_attr_objc_name_changed_by_access_note);
2063-
fixDeclarationObjCName(fixIt, VD, oldName, *note.ObjCName);
2066+
fixDeclarationObjCName(fixIt, VD, ObjCSelector(), newName);
20642067
}
2065-
else if (attr->getName() != *note.ObjCName) {
2068+
else if (attr->getName() != newName) {
2069+
// There was already an @objc
20662070
auto behavior = ctx.LangOpts.getAccessNoteFailureLimit();
20672071

20682072
VD->diagnose(diag::attr_objc_name_conflicts_with_access_note,
2069-
notes.Reason, VD->getDescriptiveKind(), *attr->getName(),
2070-
*note.ObjCName)
2073+
notes.Reason, VD->getDescriptiveKind(),
2074+
attr->getName().value(), newName)
20712075
.highlight(attr->getRangeWithAt())
20722076
.limitBehavior(behavior);
20732077
}

‎lib/Sema/TypeCheckObjC.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,7 @@ bool fixDeclarationName(InFlightDiagnostic &diag, const ValueDecl *decl,
213213
/// For properties, the selector should be a zero-parameter selector of the
214214
/// given property's name.
215215
bool fixDeclarationObjCName(InFlightDiagnostic &diag, const Decl *decl,
216-
std::optional<ObjCSelector> nameOpt,
217-
std::optional<ObjCSelector> targetNameOpt,
216+
ObjCSelector name, ObjCSelector targetName,
218217
bool ignoreImpliedName = false);
219218

220219
} // end namespace swift

‎lib/Sema/TypeCheckProtocol.cpp‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2842,8 +2842,10 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
28422842
// Also fix the Objective-C name, if needed.
28432843
if (!match.Witness->canInferObjCFromRequirement(req))
28442844
fixDeclarationObjCName(diag, match.Witness,
2845-
match.Witness->getObjCRuntimeName(),
2846-
req->getObjCRuntimeName());
2845+
match.Witness->getObjCRuntimeName()
2846+
.value_or(ObjCSelector()),
2847+
req->getObjCRuntimeName()
2848+
.value_or(ObjCSelector()));
28472849
break;
28482850
}
28492851

@@ -5066,8 +5068,8 @@ void ConformanceChecker::resolveValueWitnesses() {
50665068
if (!witness->canInferObjCFromRequirement(requirement)) {
50675069
fixDeclarationObjCName(
50685070
fixItDiag.value(), witness,
5069-
witness->getObjCRuntimeName(),
5070-
requirement->getObjCRuntimeName());
5071+
witness->getObjCRuntimeName().value_or(ObjCSelector()),
5072+
requirement->getObjCRuntimeName().value_or(ObjCSelector()));
50715073
}
50725074
} else if (isa<VarDecl>(witness)) {
50735075
std::optional<InFlightDiagnostic> fixItDiag = C.Diags.diagnose(
@@ -5086,8 +5088,8 @@ void ConformanceChecker::resolveValueWitnesses() {
50865088
if (!witness->canInferObjCFromRequirement(requirement)) {
50875089
fixDeclarationObjCName(
50885090
fixItDiag.value(), witness,
5089-
witness->getObjCRuntimeName(),
5090-
requirement->getObjCRuntimeName());
5091+
witness->getObjCRuntimeName().value_or(ObjCSelector()),
5092+
requirement->getObjCRuntimeName().value_or(ObjCSelector()));
50915093
}
50925094
} else if (isa<SubscriptDecl>(witness)) {
50935095
std::optional<InFlightDiagnostic> fixItDiag = C.Diags.diagnose(
@@ -5785,8 +5787,10 @@ static void diagnosePotentialWitness(NormalProtocolConformance *conformance,
57855787
witness->diagnose(diag::optional_req_nonobjc_near_match_add_objc);
57865788
if (!witness->canInferObjCFromRequirement(req))
57875789
fixDeclarationObjCName(diag, witness,
5788-
witness->getObjCRuntimeName(),
5789-
req->getObjCRuntimeName());
5790+
witness->getObjCRuntimeName()
5791+
.value_or(ObjCSelector()),
5792+
req->getObjCRuntimeName()
5793+
.value_or(ObjCSelector()));
57905794
} else {
57915795
diagnoseMatch(conformance->getDeclContext()->getParentModule(),
57925796
conformance, req, match);

‎test/decl/ext/objc_implementation.swift‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
// RUN: %target-typecheck-verify-swift -import-objc-header %S/Inputs/objc_implementation.h -enable-experimental-feature ObjCImplementation -enable-experimental-feature CImplementation -target %target-stable-abi-triple
22
// REQUIRES: objc_interop
33

4-
// Temporarily disabled rdar://128683206
5-
// REQUIRES: rdar128683206
6-
74
protocol EmptySwiftProto {}
85

96
@objc @implementation extension ObjCClass: EmptySwiftProto, EmptyObjCProto {

0 commit comments

Comments
 (0)