Skip to content

Commit c86dacd

Browse files
[-Wcalled-once-parameter] Let escapes overwrite MaybeCalled states
This commit makes escapes symmetrical, meaning that having escape before and after the branching, where parameter is not called on one of the paths, will have the same effect. Differential Revision: https://reviews.llvm.org/D98622
1 parent 42d653d commit c86dacd

File tree

2 files changed

+41
-9
lines changed

2 files changed

+41
-9
lines changed

clang/lib/Analysis/CalledOnceCheck.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -867,16 +867,14 @@ class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> {
867867
// Let's check if any of the call arguments is a point of interest.
868868
for (const auto &Argument : llvm::enumerate(Arguments)) {
869869
if (auto Index = getIndexOfExpression(Argument.value())) {
870-
ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(*Index);
871-
872870
if (shouldBeCalledOnce(CallOrMessage, Argument.index())) {
873871
// If the corresponding parameter is marked as 'called_once' we should
874872
// consider it as a call.
875873
processCallFor(*Index, CallOrMessage);
876-
} else if (CurrentParamStatus.getKind() == ParameterStatus::NotCalled) {
874+
} else {
877875
// Otherwise, we mark this parameter as escaped, which can be
878876
// interpreted both as called or not called depending on the context.
879-
CurrentParamStatus = ParameterStatus::Escaped;
877+
processEscapeFor(*Index);
880878
}
881879
// Otherwise, let's keep the state as it is.
882880
}
@@ -910,6 +908,16 @@ class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> {
910908
}
911909
}
912910

911+
/// Process escape of the parameter with the given index
912+
void processEscapeFor(unsigned Index) {
913+
ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(Index);
914+
915+
// Escape overrides whatever error we think happened.
916+
if (CurrentParamStatus.isErrorStatus()) {
917+
CurrentParamStatus = ParameterStatus::Escaped;
918+
}
919+
}
920+
913921
void findAndReportNotCalledBranches(const CFGBlock *Parent, unsigned Index,
914922
bool IsEscape = false) {
915923
for (const CFGBlock *Succ : Parent->succs()) {
@@ -1365,11 +1373,7 @@ class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> {
13651373
/// Check given parameter that was discovered to escape.
13661374
void checkEscapee(const ParmVarDecl &Parameter) {
13671375
if (auto Index = getIndex(Parameter)) {
1368-
ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(*Index);
1369-
1370-
if (CurrentParamStatus.getKind() == ParameterStatus::NotCalled) {
1371-
CurrentParamStatus = ParameterStatus::Escaped;
1372-
}
1376+
processEscapeFor(*Index);
13731377
}
13741378
}
13751379

clang/test/SemaObjC/warn-called-once.m

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,4 +1130,32 @@ - (void)test_nil_suppression_3:(int)cond1
11301130
}
11311131
}
11321132

1133+
- (void)test_escape_before_branch:(int)cond
1134+
withCompletion:(void (^)(void))handler {
1135+
if (cond) {
1136+
filler();
1137+
}
1138+
1139+
void (^copiedHandler)(void) = ^{
1140+
handler();
1141+
};
1142+
1143+
if (cond) {
1144+
// no-warning
1145+
handler();
1146+
} else {
1147+
copiedHandler();
1148+
}
1149+
}
1150+
1151+
- (void)test_escape_after_branch:(int)cond
1152+
withCompletion:(void (^)(void))handler {
1153+
if (cond) {
1154+
// no-warning
1155+
handler();
1156+
}
1157+
1158+
escape(handler);
1159+
}
1160+
11331161
@end

0 commit comments

Comments
 (0)