Author: Valeriy Savchenko Date: 2021-03-17T11:12:55+03:00 New Revision: c86dacd1a4489572721dec3135506d31da96c679
URL: https://github.com/llvm/llvm-project/commit/c86dacd1a4489572721dec3135506d31da96c679 DIFF: https://github.com/llvm/llvm-project/commit/c86dacd1a4489572721dec3135506d31da96c679.diff LOG: [-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 Added: Modified: clang/lib/Analysis/CalledOnceCheck.cpp clang/test/SemaObjC/warn-called-once.m Removed: ################################################################################ diff --git a/clang/lib/Analysis/CalledOnceCheck.cpp b/clang/lib/Analysis/CalledOnceCheck.cpp index 2438c50d7e4e..d24e0b500564 100644 --- a/clang/lib/Analysis/CalledOnceCheck.cpp +++ b/clang/lib/Analysis/CalledOnceCheck.cpp @@ -867,16 +867,14 @@ class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> { // Let's check if any of the call arguments is a point of interest. for (const auto &Argument : llvm::enumerate(Arguments)) { if (auto Index = getIndexOfExpression(Argument.value())) { - ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(*Index); - if (shouldBeCalledOnce(CallOrMessage, Argument.index())) { // If the corresponding parameter is marked as 'called_once' we should // consider it as a call. processCallFor(*Index, CallOrMessage); - } else if (CurrentParamStatus.getKind() == ParameterStatus::NotCalled) { + } else { // Otherwise, we mark this parameter as escaped, which can be // interpreted both as called or not called depending on the context. - CurrentParamStatus = ParameterStatus::Escaped; + processEscapeFor(*Index); } // Otherwise, let's keep the state as it is. } @@ -910,6 +908,16 @@ class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> { } } + /// Process escape of the parameter with the given index + void processEscapeFor(unsigned Index) { + ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(Index); + + // Escape overrides whatever error we think happened. + if (CurrentParamStatus.isErrorStatus()) { + CurrentParamStatus = ParameterStatus::Escaped; + } + } + void findAndReportNotCalledBranches(const CFGBlock *Parent, unsigned Index, bool IsEscape = false) { for (const CFGBlock *Succ : Parent->succs()) { @@ -1365,11 +1373,7 @@ class CalledOnceChecker : public ConstStmtVisitor<CalledOnceChecker> { /// Check given parameter that was discovered to escape. void checkEscapee(const ParmVarDecl &Parameter) { if (auto Index = getIndex(Parameter)) { - ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(*Index); - - if (CurrentParamStatus.getKind() == ParameterStatus::NotCalled) { - CurrentParamStatus = ParameterStatus::Escaped; - } + processEscapeFor(*Index); } } diff --git a/clang/test/SemaObjC/warn-called-once.m b/clang/test/SemaObjC/warn-called-once.m index 3d846deca921..7d0679035238 100644 --- a/clang/test/SemaObjC/warn-called-once.m +++ b/clang/test/SemaObjC/warn-called-once.m @@ -1130,4 +1130,32 @@ - (void)test_nil_suppression_3:(int)cond1 } } +- (void)test_escape_before_branch:(int)cond + withCompletion:(void (^)(void))handler { + if (cond) { + filler(); + } + + void (^copiedHandler)(void) = ^{ + handler(); + }; + + if (cond) { + // no-warning + handler(); + } else { + copiedHandler(); + } +} + +- (void)test_escape_after_branch:(int)cond + withCompletion:(void (^)(void))handler { + if (cond) { + // no-warning + handler(); + } + + escape(handler); +} + @end _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits