Skip to content

Commit c0b1a05

Browse files
committed
[clang][objc] Fix non-deterministic stub fixit output for missing requirements
When outputting fixits for missing methods, we had two issues * If we found an optional method we could incorrectly hide the fixit for a requiremed method with the same selector from another protocol * The fixit order wasn't deterministic when two protocols declared the same required selector Fix by choosing the best candidate to store when walking methods. rdar://109411920 (cherry picked from commit 635f5a6)
1 parent d2658ca commit c0b1a05

File tree

2 files changed

+43
-1
lines changed

2 files changed

+43
-1
lines changed

clang/lib/Edit/FillInMissingProtocolStubs.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,20 @@ class MethodSet {
145145
if (Availability == AR_NotYetIntroduced || Availability == AR_Unavailable)
146146
continue;
147147
auto &Map = M->isInstanceMethod() ? InstanceMethods : ClassMethods;
148-
Map.insert(std::make_pair(M->getSelector(), MethodInfo(M, P, Priority)));
148+
MethodInfo MI(M, P, Priority);
149+
auto [I, Inserted] = Map.insert(std::make_pair(M->getSelector(), MI));
150+
if (Inserted)
151+
continue;
152+
153+
// Prefer a required method so we do not miss fixits, and the lowest
154+
// priority so the order of fixits will be deterministic.
155+
if (MI.isRequired() != I->second.isRequired()) {
156+
if (MI.isRequired())
157+
I->second = MI;
158+
continue;
159+
}
160+
if (MI.ProtocolPriority < I->second.ProtocolPriority)
161+
I->second = MI;
149162
}
150163
}
151164

clang/test/FixIt/fixit-fill-in-protocol-requirements.m

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,32 @@ @implementation FixitJustAvailable // expected-warning {{class 'FixitJustAvailab
8282
@end
8383
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"- (void)availableMethod { \n <#code#>\n}\n\n- (void)deprecatedMethod { \n <#code#>\n}\n\n"
8484
// AVAILABLE: fix-it:{{.*}}:{[[@LINE-2]]:1-[[@LINE-2]]:1}:"- (void)availableMethod { \n <#code#>\n}\n\n- (void)deprecatedMethod { \n <#code#>\n}\n\n+ (void)notYetAvailableMethod { \n <#code#>\n}\n\n"
85+
86+
@protocol PReq1
87+
-(void)reqZ1;
88+
@end
89+
90+
@protocol PReq2
91+
-(void)reqZ1;
92+
-(void)reqA2;
93+
-(void)reqA1;
94+
@end
95+
96+
// Ensure optional cannot hide required methods with the same selector.
97+
@protocol POpt
98+
@optional
99+
-(void)reqZ1;
100+
-(void)reqA2;
101+
-(void)reqA1;
102+
@end
103+
104+
@interface MultiReqOpt <PReq1, PReq2, POpt>
105+
@end
106+
@implementation MultiReqOpt // expected-warning {{class 'MultiReqOpt' does not conform to protocols 'PReq1' and 'PReq2'}} expected-note {{add stubs}}
107+
@end
108+
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:1-[[@LINE-1]]:1}:"
109+
// Z1 is first due to being from the first listed protocol, PReq1
110+
// CHECK-SAME: - (void)reqZ1
111+
// A1 is before A2 because of secondary sort by name.
112+
// CHECK-SAME: - (void)reqA1
113+
// CHECK-SAME: - (void)reqA2

0 commit comments

Comments
 (0)