diff --git a/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h b/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h new file mode 100644 --- /dev/null +++ b/clang/include/clang/Analysis/Analyses/CalledOnceCheck.h @@ -0,0 +1,113 @@ +//===- CalledOnceCheck.h - Check 'called once' parameters -------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines a check for function-like parameters that should be +// called exactly one time. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_CALLEDONCECHECK_H +#define LLVM_CLANG_ANALYSIS_ANALYSES_CALLEDONCECHECK_H + +namespace clang { + +class AnalysisDeclContext; +class CFG; +class Decl; +class DeclContext; +class Expr; +class ParmVarDecl; +class Stmt; + +/// Classification of situations when parameter is not called on every path. +/// \enum IfThen -- then branch of the if statement has no call. +/// \enum IfElse -- else branch of the if statement has no call. +/// \enum Switch -- one of the switch cases doesn't have a call. +/// \enum SwitchSkipped -- there is no call if none of the cases appies. +/// \enum LoopEntered -- no call when the loop is entered. +/// \enum LoopSkipped -- no call when the loop is not entered. +/// \enum FallbackReason -- fallback case when we were not able to figure out +/// the reason. +enum class NeverCalledReason { + IfThen, + IfElse, + Switch, + SwitchSkipped, + LoopEntered, + LoopSkipped, + FallbackReason, + LARGEST_VALUE = FallbackReason +}; + +class CalledOnceCheckHandler { +public: + CalledOnceCheckHandler() = default; + virtual ~CalledOnceCheckHandler() = default; + + /// Called when parameter is called twice. + /// \param Parameter -- parameter that should be called once. + /// \param Call -- call to report the warning. + /// \param PrevCall -- previous call. + /// \param IsCompletionHandler -- true, if parameter is a completion handler. + /// \param Poised -- true, if the second call is guaranteed to happen after + /// the first call. + virtual void handleDoubleCall(const ParmVarDecl *Parameter, const Expr *Call, + const Expr *PrevCall, bool IsCompletionHandler, + bool Poised) {} + + /// Called when parameter is not called at all. + /// \param Parameter -- parameter that should be called once. + /// \param IsCompletionHandler -- true, if parameter is a completion handler. + virtual void handleNeverCalled(const ParmVarDecl *Parameter, + bool IsCompletionHandler) {} + + /// Called when captured parameter is not called at all. + /// \param Parameter -- parameter that should be called once. + /// \param Where -- declaration that captures \p Parameter + /// \param IsCompletionHandler -- true, if parameter is a completion handler. + virtual void handleCapturedNeverCalled(const ParmVarDecl *Parameter, + const Decl *Where, + bool IsCompletionHandler) {} + + /// Called when parameter is not called on one of the paths. + /// Usually we try to find a statement that is the least common ancestor of + /// the path containing the call and not containing the call. This helps us + /// to pinpoint a bad path for the user. + /// \param Parameter -- parameter that should be called once. + /// \param Where -- the least common ancestor statement. + /// \param Reason -- a reason describing the path without a call. + /// \param IsCalledDirectly -- true, if parameter actually gets called on + /// the other path. It is opposed to be used in some other way (added to some + /// collection, passed as a parameter, etc.). + /// \param IsCompletionHandler -- true, if parameter is a completion handler. + virtual void handleNeverCalled(const ParmVarDecl *Parameter, + const Stmt *Where, NeverCalledReason Reason, + bool IsCalledDirectly, + bool IsCompletionHandler) {} +}; + +/// Check given CFG for 'called once' parameter violations. +/// +/// It traverses the function and tracks how such parameters are used. +/// It detects two main violations: +/// * parameter is called twice +/// * parameter is not called +/// +/// \param FunctionCFG -- CFG of the function to analyze. +/// \param AC -- context. +/// \param Handler -- a handler for found violations. +/// \param CheckConventionalParameters -- true, if we want to check parameters +/// not explicitly marked as 'called once', but having the same requirements +/// according to conventions. +void checkCalledOnceParameters(const CFG &FunctionCFG, AnalysisDeclContext &AC, + CalledOnceCheckHandler &Handler, + bool CheckConventionalParameters); + +} // end namespace clang + +#endif /* LLVM_CLANG_ANALYSIS_ANALYSES_CALLEDONCECHECK_H */ diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1780,6 +1780,13 @@ let Documentation = [ReturnsNonNullDocs]; } +def CalledOnce : Attr { + let Spellings = [Clang<"called_once">]; + let Subjects = SubjectList<[ParmVar]>; + let LangOpts = [ObjC]; + let Documentation = [CalledOnceDocs]; +} + // pass_object_size(N) indicates that the parameter should have // __builtin_object_size with Type=N evaluated on the parameter at the callsite. def PassObjectSize : InheritableParamAttr { diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -5083,6 +5083,59 @@ }]; } +def CalledOnceDocs : Documentation { + let Category = DocCatVariable; + let Content = [{ +The ``called_once`` attribute specifies that the annotated function or method +parameter is invoked exactly once on all execution paths. It only applies +to parameters with function-like types, i.e. function pointers or blocks. This +concept is particularly useful for asynchronous programs. + +Clang implements a check for ``called_once`` parameters, +``-Wcalled-once-parameter``. It is on by default and finds the following +violations: + +* Parameter is not called at all. + +* Parameter is called more than once. + +* Parameter is not called on one of the execution paths. + +In the latter case, Clang pinpoints the path where parameter is not invoked +by showing the control-flow statement where the path diverges. + +.. code-block:: objc + + void fooWithCallback(void (^callback)(void) __attribute__((called_once))) { + if (somePredicate()) { + ... + callback(); + } esle { + callback(); // OK: callback is called on every path + } + } + + void barWithCallback(void (^callback)(void) __attribute__((called_once))) { + if (somePredicate()) { + ... + callback(); // note: previous call is here + } + callback(); // warning: callback is called twice + } + + void foobarWithCallback(void (^callback)(void) __attribute__((called_once))) { + if (somePredicate()) { // warning: callback is not called when condition is false + ... + callback(); + } + } + +This attribute is useful for API developers who want to double-check if they +implemented their method correctly. + + }]; +} + def GnuInlineDocs : Documentation { let Category = DocCatFunction; let Content = [{ diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -485,6 +485,8 @@ def ObjCMultipleMethodNames : DiagGroup<"objc-multiple-method-names">; def ObjCFlexibleArray : DiagGroup<"objc-flexible-array">; def ObjCBoxing : DiagGroup<"objc-boxing">; +def CompletionHandler : DiagGroup<"completion-handler">; +def CalledOnceParameter : DiagGroup<"called-once-parameter", [CompletionHandler]>; def OpenCLUnsupportedRGBA: DiagGroup<"opencl-unsupported-rgba">; def UnderalignedExceptionObject : DiagGroup<"underaligned-exception-object">; def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3942,6 +3942,40 @@ def note_protocol_decl_undefined : Note< "protocol %0 has no definition">; +// called-once attribute diagnostics. +def err_called_once_attribute_wrong_type : Error< + "'called_once' attribute only applies to function-like parameters">; + +def warn_completion_handler_never_called : Warning< + "%select{|captured }1completion handler is never called">, + InGroup, DefaultIgnore; +def warn_called_once_never_called : Warning< + "%select{|captured }1%0 parameter marked 'called_once' is never called">, + InGroup; + +def warn_completion_handler_never_called_when : Warning< + "completion handler is never %select{used|called}1 when " + "%select{taking true branch|taking false branch|" + "handling this case|none of the cases applies|" + "entering the loop|skipping the loop|taking one of the branches}2">, + InGroup, DefaultIgnore; +def warn_called_once_never_called_when : Warning< + "%0 parameter marked 'called_once' is never %select{used|called}1 when " + "%select{taking true branch|taking false branch|" + "handling this case|none of the cases applies|" + "entering the loop|skipping the loop|taking one of the branches}2">, + InGroup; + +def warn_completion_handler_called_twice : Warning< + "completion handler is called twice">, + InGroup, DefaultIgnore; +def warn_called_once_gets_called_twice : Warning< + "%0 parameter marked 'called_once' is called twice">, + InGroup; +def note_called_once_gets_called_twice : Note< + "previous call is here%select{; set to nil to indicate " + "it cannot be called afterwards|}0">; + // objc_designated_initializer attribute diagnostics. def warn_objc_designated_init_missing_super_call : Warning< "designated initializer missing a 'super' call to a designated initializer of the super class">, diff --git a/clang/lib/Analysis/CMakeLists.txt b/clang/lib/Analysis/CMakeLists.txt --- a/clang/lib/Analysis/CMakeLists.txt +++ b/clang/lib/Analysis/CMakeLists.txt @@ -6,6 +6,7 @@ add_clang_library(clangAnalysis AnalysisDeclContext.cpp BodyFarm.cpp + CalledOnceCheck.cpp CFG.cpp CFGReachabilityAnalysis.cpp CFGStmtMap.cpp diff --git a/clang/lib/Analysis/CalledOnceCheck.cpp b/clang/lib/Analysis/CalledOnceCheck.cpp new file mode 100644 --- /dev/null +++ b/clang/lib/Analysis/CalledOnceCheck.cpp @@ -0,0 +1,1516 @@ +//===- CalledOnceCheck.cpp - Check 'called once' parameters ---------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Analysis/Analyses/CalledOnceCheck.h" +#include "clang/AST/Attr.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclBase.h" +#include "clang/AST/Expr.h" +#include "clang/AST/ExprObjC.h" +#include "clang/AST/OperationKinds.h" +#include "clang/AST/ParentMap.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/StmtObjC.h" +#include "clang/AST/StmtVisitor.h" +#include "clang/AST/Type.h" +#include "clang/Analysis/AnalysisDeclContext.h" +#include "clang/Analysis/CFG.h" +#include "clang/Analysis/FlowSensitive/DataflowWorklist.h" +#include "clang/Basic/IdentifierTable.h" +#include "clang/Basic/LLVM.h" +#include "llvm/ADT/BitVector.h" +#include "llvm/ADT/BitmaskEnum.h" +#include "llvm/ADT/Optional.h" +#include "llvm/ADT/PointerIntPair.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/Sequence.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Casting.h" +#include "llvm/Support/Compiler.h" +#include "llvm/Support/ErrorHandling.h" +#include + +using namespace clang; + +namespace { +static constexpr unsigned EXPECTED_MAX_NUMBER_OF_PARAMS = 2; +template +using ParamSizedVector = llvm::SmallVector; +static constexpr unsigned EXPECTED_NUMBER_OF_BASIC_BLOCKS = 8; +template +using CFGSizedVector = llvm::SmallVector; +constexpr llvm::StringLiteral CONVENTIONAL_NAMES[] = { + "completionHandler", "completion", "withCompletionHandler"}; +constexpr llvm::StringLiteral CONVENTIONAL_SUFFIXES[] = { + "WithCompletionHandler", "WithCompletion"}; +constexpr llvm::StringLiteral CONVENTIONAL_CONDITIONS[] = { + "error", "cancel", "shouldCall", "done", "OK", "success"}; + +class ParameterStatus { +public: + // Status kind is basically the main part of parameter's status. + // The kind represents our knowledge (so far) about a tracked parameter + // in the context of this analysis. + // + // Since we want to report on missing and extraneous calls, we need to + // track the fact whether paramater was called or not. This automatically + // decides two kinds: `NotCalled` and `Called`. + // + // One of the erroneous situations is the case when parameter is called only + // on some of the paths. We couldv'e considered it `NotCalled`, but we want + // to report double call warnings even if these two calls are not guaranteed + // to happen in every execution. We also don't want to have it as `Called` + // because not calling tracked parameter on all of the paths is an error + // on its own. For these reasons, we need to have a separate kind, + // `MaybeCalled`, and change `Called` to `DefinitelyCalled` to avoid + // confusion. + // + // Two violations of calling parameter more than once and not calling it on + // every path are not, however, mutually exclusive. In situations where both + // violations take place, we prefer to report ONLY double call. It's always + // harder to pinpoint a bug that has arisen when a user neglects to take the + // right action (and therefore, no action is taken), than when a user takes + // the wrong action. And, in order to remember that we already reported + // a double call, we need another kind: `Reported`. + // + // Our analysis is intra-procedural and, while in the perfect world, + // developers only use tracked parameters to call them, in the real world, + // the picture might be different. Parameters can be stored in global + // variables or leaked into other functions that we know nothing about. + // We try to be lenient and trust users. Another kind `Escaped` reflects + // such situations. We don't know if it gets called there or not, but we + // should always think of `Escaped` as the best possible option. + // + // Some of the paths in the analyzed functions might end with a call + // to noreturn functions. Such paths are not required to have parameter + // calls and we want to track that. For the purposes of better diagnostics, + // we don't want to reuse `Escaped` and, thus, have another kind `NoReturn`. + // + // Additionally, we have `NotVisited` kind that tells us nothing about + // a tracked parameter, but is used for tracking analyzed (aka visited) + // basic blocks. + // + // If we consider `|` to be a JOIN operation of two kinds coming from + // two different paths, the following properties must hold: + // + // 1. for any Kind K: K | K == K + // Joining two identical kinds should result in the same kind. + // + // 2. for any Kind K: Reported | K == Reported + // Doesn't matter on which path it was reported, it still is. + // + // 3. for any Kind K: NoReturn | K == K + // We can totally ignore noreturn paths during merges. + // + // 4. DefinitelyCalled | NotCalled == MaybeCalled + // Called on one path, not called on another - that's simply + // a definition for MaybeCalled. + // + // 5. for any Kind K in [DefinitelyCalled, NotCalled, MaybeCalled]: + // Escaped | K == K + // Escaped mirrors other statuses after joins. + // Every situation, when we join any of the listed kinds K, + // is a violation. For this reason, in order to assume the + // best outcome for this escape, we consider it to be the + // same as the other path. + // + // 6. for any Kind K in [DefinitelyCalled, NotCalled]: + // MaybeCalled | K == MaybeCalled + // MaybeCalled should basically stay after almost every join. + enum Kind { + // No-return paths should be absolutely transparent for the analysis. + // 0x0 is the identity element for selected join operation (binary or). + NoReturn = 0x0, /* 0000 */ + // Escaped marks situations when marked parameter escaped into + // another function (so we can assume that it was possibly called there). + Escaped = 0x1, /* 0001 */ + // Parameter was definitely called once at this point. + DefinitelyCalled = 0x3, /* 0011 */ + // Kinds less or equal to NON_ERROR_STATUS are not considered errors. + NON_ERROR_STATUS = DefinitelyCalled, + // Parameter was not yet called. + NotCalled = 0x5, /* 0101 */ + // Parameter was not called at least on one path leading to this point, + // while there is also at least one path that it gets called. + MaybeCalled = 0x7, /* 0111 */ + // Parameter was not yet analyzed. + NotVisited = 0x8, /* 1000 */ + // We already reported a violation and stopped tracking calls for this + // parameter. + Reported = 0x15, /* 1111 */ + LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ Reported) + }; + + constexpr ParameterStatus() = default; + /* implicit */ ParameterStatus(Kind K) : StatusKind(K) { + assert(!canBeCalled(K) && "Can't initialize status without a call"); + } + ParameterStatus(Kind K, const Expr *Call) : StatusKind(K), Call(Call) { + assert(canBeCalled(K) && "This kind is not supposed to have a call"); + } + + const Expr &getCall() const { + assert(canBeCalled(getKind()) && "ParameterStatus doesn't have a call"); + return *Call; + } + static bool canBeCalled(Kind K) { + return (K & DefinitelyCalled) == DefinitelyCalled && K != Reported; + } + bool canBeCalled() const { return canBeCalled(getKind()); } + + static bool isErrorStatus(Kind K) { return K > NON_ERROR_STATUS; } + bool isErrorStatus() const { return isErrorStatus(getKind()); } + + Kind getKind() const { return StatusKind; } + + void join(const ParameterStatus &Other) { + // If we have a pointer already, let's keep it. + // For the purposes of the analysis, it doesn't really matter + // which call we report. + // + // If we don't have a pointer, let's take whatever gets joined. + if (!Call) { + Call = Other.Call; + } + // Join kinds. + StatusKind |= Other.getKind(); + } + + bool operator==(const ParameterStatus &Other) const { + // We compare only kinds, pointers on their own is only additional + // information. + return getKind() == Other.getKind(); + } + +private: + // It would've been a perfect place to use llvm::PointerIntPair, but + // unfortunately NumLowBitsAvailable for clang::Expr had been reduced to 2. + Kind StatusKind = NotVisited; + const Expr *Call = nullptr; +}; + +/// State aggregates statuses of all tracked parameters. +class State { +public: + State(unsigned Size, ParameterStatus::Kind K = ParameterStatus::NotVisited) + : ParamData(Size, K) {} + + /// Return status of a parameter with the given index. + /// \{ + ParameterStatus &getStatusFor(unsigned Index) { return ParamData[Index]; } + const ParameterStatus &getStatusFor(unsigned Index) const { + return ParamData[Index]; + } + /// \} + + /// Return true if parameter with the given index can be called. + bool canBeCalled(unsigned Index) const { + return getStatusFor(Index).canBeCalled(); + } + /// Return a reference that we consider a call. + /// + /// Should only be used for parameters that can be called. + const Expr &getCallFor(unsigned Index) const { + return getStatusFor(Index).getCall(); + } + /// Return status kind of parameter with the given index. + ParameterStatus::Kind getKindFor(unsigned Index) const { + return getStatusFor(Index).getKind(); + } + + bool isVisited() const { + return llvm::all_of(ParamData, [](const ParameterStatus &S) { + return S.getKind() != ParameterStatus::NotVisited; + }); + } + + // Join other state into the current state. + void join(const State &Other) { + assert(ParamData.size() == Other.ParamData.size() && + "Couldn't join statuses with different sizes"); + for (auto Pair : llvm::zip(ParamData, Other.ParamData)) { + std::get<0>(Pair).join(std::get<1>(Pair)); + } + } + + using iterator = ParamSizedVector::iterator; + using const_iterator = ParamSizedVector::const_iterator; + + iterator begin() { return ParamData.begin(); } + iterator end() { return ParamData.end(); } + + const_iterator begin() const { return ParamData.begin(); } + const_iterator end() const { return ParamData.end(); } + + bool operator==(const State &Other) const { + return ParamData == Other.ParamData; + } + +private: + ParamSizedVector ParamData; +}; + +/// A simple class that finds DeclRefExpr in the given expression. +/// +/// However, we don't want to find ANY nested DeclRefExpr skipping whatever +/// expressions on our way. Only certain expressions considered "no-op" +/// for our task are indeed skipped. +class DeclRefFinder + : public ConstStmtVisitor { +public: + /// Find a DeclRefExpr in the given expression. + /// + /// In its most basic form (ShouldRetrieveFromComparisons == false), + /// this function can be simply reduced to the following question: + /// + /// - If expression E is used as a function argument, could we say + /// that DeclRefExpr nested in E is used as an argument? + /// + /// According to this rule, we can say that parens, casts and dereferencing + /// (dereferencing only applied to function pointers, but this is our case) + /// can be skipped. + /// + /// When we should look into comparisons the question changes to: + /// + /// - If expression E is used as a condition, could we say that + /// DeclRefExpr is being checked? + /// + /// And even though, these are two different questions, they have quite a lot + /// in common. Actually, we can say that whatever expression answers + /// positively the first question also fits the second question as well. + /// + /// In addition, we skip binary operators == and !=, and unary opeartor !. + static const DeclRefExpr *find(const Expr *E, + bool ShouldRetrieveFromComparisons = false) { + return DeclRefFinder(ShouldRetrieveFromComparisons).Visit(E); + } + + const DeclRefExpr *VisitDeclRefExpr(const DeclRefExpr *DR) { return DR; } + + const DeclRefExpr *VisitUnaryOperator(const UnaryOperator *UO) { + switch (UO->getOpcode()) { + case UO_LNot: + // We care about logical not only if we care about comparisons. + if (!ShouldRetrieveFromComparisons) + return nullptr; + LLVM_FALLTHROUGH; + // Function pointer/references can be dereferenced before a call. + // That doesn't make it, however, any different from a regular call. + // For this reason, dereference operation is a "no-op". + case UO_Deref: + return Visit(UO->getSubExpr()); + default: + return nullptr; + } + } + + const DeclRefExpr *VisitBinaryOperator(const BinaryOperator *BO) { + if (!ShouldRetrieveFromComparisons) + return nullptr; + + switch (BO->getOpcode()) { + case BO_EQ: + case BO_NE: { + const DeclRefExpr *LHS = Visit(BO->getLHS()); + return LHS ? LHS : Visit(BO->getRHS()); + } + default: + return nullptr; + } + } + + const DeclRefExpr *VisitOpaqueValueExpr(const OpaqueValueExpr *OVE) { + return Visit(OVE->getSourceExpr()); + } + + const DeclRefExpr *VisitExpr(const Expr *E) { + // It is a fallback method that gets called whenever the actual type + // of the given expression is not covered. + // + // We first check if we have anything to skip. And then repeat the whole + // procedure for a nested expression instead. + const Expr *DeclutteredExpr = E->IgnoreParenCasts(); + return E != DeclutteredExpr ? Visit(DeclutteredExpr) : nullptr; + } + +private: + DeclRefFinder(bool ShouldRetrieveFromComparisons) + : ShouldRetrieveFromComparisons(ShouldRetrieveFromComparisons) {} + + bool ShouldRetrieveFromComparisons; +}; + +const DeclRefExpr *findDeclRefExpr(const Expr *In, + bool ShouldRetrieveFromComparisons = false) { + return DeclRefFinder::find(In, ShouldRetrieveFromComparisons); +} + +const ParmVarDecl * +findReferencedParmVarDecl(const Expr *In, + bool ShouldRetrieveFromComparisons = false) { + if (const DeclRefExpr *DR = + findDeclRefExpr(In, ShouldRetrieveFromComparisons)) { + return dyn_cast(DR->getDecl()); + } + + return nullptr; +} + +/// Return conditions expression of a statement if it has one. +const Expr *getCondition(const Stmt *S) { + if (!S) { + return nullptr; + } + + if (const auto *If = dyn_cast(S)) { + return If->getCond(); + } + if (const auto *Ternary = dyn_cast(S)) { + return Ternary->getCond(); + } + + return nullptr; +} + +/// A small helper class that collects all named identifiers in the given +/// expression. It traverses it recursively, so names from deeper levels +/// of the AST will end up in the results. +/// Results might have duplicate names, if this is a problem, convert to +/// string sets afterwards. +class NamesCollector : public RecursiveASTVisitor { +public: + static constexpr unsigned EXPECTED_NUMBER_OF_NAMES = 5; + using NameCollection = + llvm::SmallVector; + + static NameCollection collect(const Expr *From) { + NamesCollector Impl; + Impl.TraverseStmt(const_cast(From)); + return Impl.Result; + } + + bool VisitDeclRefExpr(const DeclRefExpr *E) { + Result.push_back(E->getDecl()->getName()); + return true; + } + + bool VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *E) { + llvm::StringRef Name; + + if (E->isImplicitProperty()) { + ObjCMethodDecl *PropertyMethodDecl = nullptr; + if (E->isMessagingGetter()) { + PropertyMethodDecl = E->getImplicitPropertyGetter(); + } else { + PropertyMethodDecl = E->getImplicitPropertySetter(); + } + assert(PropertyMethodDecl && + "Implicit property must have associated declaration"); + Name = PropertyMethodDecl->getSelector().getNameForSlot(0); + } else { + assert(E->isExplicitProperty()); + Name = E->getExplicitProperty()->getName(); + } + + Result.push_back(Name); + return true; + } + +private: + NamesCollector() = default; + NameCollection Result; +}; + +/// Check whether the given expression mentions any of conventional names. +bool mentionsAnyOfConventionalNames(const Expr *E) { + NamesCollector::NameCollection MentionedNames = NamesCollector::collect(E); + + return llvm::any_of(MentionedNames, [](llvm::StringRef ConditionName) { + return llvm::any_of( + CONVENTIONAL_CONDITIONS, + [ConditionName](const llvm::StringLiteral &Conventional) { + return ConditionName.contains_lower(Conventional); + }); + }); +} + +struct Clarification { + NeverCalledReason Reason; + const Stmt *Location; +}; + +class NotCalledClarifier + : public ConstStmtVisitor> { +public: + static llvm::Optional + clarify(const CFGBlock *Conditional, const CFGBlock *SuccWithoutCall) { + + if (const Stmt *Terminator = Conditional->getTerminatorStmt()) { + return NotCalledClarifier{Conditional, SuccWithoutCall}.Visit(Terminator); + } + return llvm::None; + } + + llvm::Optional VisitIfStmt(const IfStmt *If) { + return VisitBranchingBlock(If, NeverCalledReason::IfThen); + } + + llvm::Optional + VisitAbstractConditionalOperator(const AbstractConditionalOperator *Ternary) { + return VisitBranchingBlock(Ternary, NeverCalledReason::IfThen); + } + + llvm::Optional VisitSwitchStmt(const SwitchStmt *Switch) { + const Stmt *CaseToBlame = SuccInQuestion->getLabel(); + if (!CaseToBlame) { + // If interesting basic block is not labeled, it means that this + // basic block does not represent any of the cases. + return Clarification{NeverCalledReason::SwitchSkipped, Switch}; + } + + for (const SwitchCase *Case = Switch->getSwitchCaseList(); Case; + Case = Case->getNextSwitchCase()) { + if (Case == CaseToBlame) { + return Clarification{NeverCalledReason::Switch, Case}; + } + } + + llvm_unreachable("Found unexpected switch structure"); + } + + llvm::Optional VisitForStmt(const ForStmt *For) { + return VisitBranchingBlock(For, NeverCalledReason::LoopEntered); + } + + llvm::Optional VisitWhileStmt(const WhileStmt *While) { + return VisitBranchingBlock(While, NeverCalledReason::LoopEntered); + } + + llvm::Optional + VisitBranchingBlock(const Stmt *Terminator, NeverCalledReason DefaultReason) { + assert(Parent->succ_size() == 2 && + "Branching block should have exactly two successors"); + unsigned SuccessorIndex = getSuccessorIndex(Parent, SuccInQuestion); + NeverCalledReason ActualReason = + updateForSuccessor(DefaultReason, SuccessorIndex); + return Clarification{ActualReason, Terminator}; + } + + llvm::Optional VisitBinaryOperator(const BinaryOperator *) { + // We don't want to report on short-curcuit logical operations. + return llvm::None; + } + + llvm::Optional VisitStmt(const Stmt *Terminator) { + // If we got here, we didn't have a visit function for more derived + // classes of statement that this terminator actually belongs to. + // + // This is not a good scenario and should not happen in practice, but + // at least we'll warn the user. + return Clarification{NeverCalledReason::FallbackReason, Terminator}; + } + + static unsigned getSuccessorIndex(const CFGBlock *Parent, + const CFGBlock *Child) { + CFGBlock::const_succ_iterator It = llvm::find(Parent->succs(), Child); + assert(It != Parent->succ_end() && + "Given blocks should be in parent-child relationship"); + return It - Parent->succ_begin(); + } + + static NeverCalledReason + updateForSuccessor(NeverCalledReason ReasonForTrueBranch, + unsigned SuccessorIndex) { + assert(SuccessorIndex <= 1); + unsigned RawReason = + static_cast(ReasonForTrueBranch) + SuccessorIndex; + assert(RawReason <= + static_cast(NeverCalledReason::LARGEST_VALUE)); + return static_cast(RawReason); + } + +private: + NotCalledClarifier(const CFGBlock *Parent, const CFGBlock *SuccInQuestion) + : Parent(Parent), SuccInQuestion(SuccInQuestion) {} + + const CFGBlock *Parent, *SuccInQuestion; +}; + +class CalledOnceChecker : public ConstStmtVisitor { +public: + static void check(const CFG &FunctionCFG, AnalysisDeclContext &AC, + CalledOnceCheckHandler &Handler, + bool CheckConventionalParameters) { + CalledOnceChecker(FunctionCFG, AC, Handler, CheckConventionalParameters) + .check(); + } + +private: + CalledOnceChecker(const CFG &FunctionCFG, AnalysisDeclContext &AC, + CalledOnceCheckHandler &Handler, + bool CheckConventionalParameters) + : FunctionCFG(FunctionCFG), AC(AC), Handler(Handler), + CheckConventionalParameters(CheckConventionalParameters), + CurrentState(0) { + initDataStructures(); + assert(size() == 0 || + !States.empty() && "Data structures are inconsistent"); + } + + //===----------------------------------------------------------------------===// + // Initializing functions + //===----------------------------------------------------------------------===// + + void initDataStructures() { + const Decl *AnalyzedDecl = AC.getDecl(); + + if (const auto *Function = dyn_cast(AnalyzedDecl)) { + findParamsToTrack(Function); + } else if (const auto *Method = dyn_cast(AnalyzedDecl)) { + findParamsToTrack(Method); + } else if (const auto *Block = dyn_cast(AnalyzedDecl)) { + findCapturesToTrack(Block); + findParamsToTrack(Block); + } + + // Have something to track, let's init states for every block from the CFG. + if (size() != 0) { + States = + CFGSizedVector(FunctionCFG.getNumBlockIDs(), State(size())); + } + } + + void findCapturesToTrack(const BlockDecl *Block) { + for (const auto &Capture : Block->captures()) { + if (const auto *P = dyn_cast(Capture.getVariable())) { + // Parameter DeclContext is its owning function or method. + const DeclContext *ParamContext = P->getDeclContext(); + if (shouldBeCalledOnce(ParamContext, P)) { + TrackedParams.push_back(P); + } + } + } + } + + template + void findParamsToTrack(const FunctionLikeDecl *Function) { + for (unsigned Index : llvm::seq(0u, Function->param_size())) { + if (shouldBeCalledOnce(Function, Index)) { + TrackedParams.push_back(Function->getParamDecl(Index)); + } + } + } + + //===----------------------------------------------------------------------===// + // Main logic 'check' functions + //===----------------------------------------------------------------------===// + + void check() { + // Nothing to check here: we don't have marked parameters. + if (size() == 0 || isPossiblyEmptyImpl()) + return; + + assert( + llvm::none_of(States, [](const State &S) { return S.isVisited(); }) && + "None of the blocks should be 'visited' before the analysis"); + + // For our task, both backward and forward approaches suite well. + // However, in order to report better diagnostics, we decided to go with + // backward analysis. + // + // Let's consider the following CFG and how forward and backward analyses + // will work for it. + // + // FORWARD: | BACKWARD: + // #1 | #1 + // +---------+ | +-----------+ + // | if | | |MaybeCalled| + // +---------+ | +-----------+ + // |NotCalled| | | if | + // +---------+ | +-----------+ + // / \ | / \ + // #2 / \ #3 | #2 / \ #3 + // +----------------+ +---------+ | +----------------+ +---------+ + // | foo() | | ... | | |DefinitelyCalled| |NotCalled| + // +----------------+ +---------+ | +----------------+ +---------+ + // |DefinitelyCalled| |NotCalled| | | foo() | | ... | + // +----------------+ +---------+ | +----------------+ +---------+ + // \ / | \ / + // \ #4 / | \ #4 / + // +-----------+ | +---------+ + // | ... | | |NotCalled| + // +-----------+ | +---------+ + // |MaybeCalled| | | ... | + // +-----------+ | +---------+ + // + // The most natural way to report lacking call in the block #3 would be to + // message that the false branch of the if statement in the block #1 doesn't + // have a call. And while with the forward approach we'll need to find a + // least common ancestor or something like that to find the 'if' to blame, + // backward analysis gives is to us out of the box. + BackwardDataflowWorklist Worklist(FunctionCFG, AC); + + // Let's visit EXIT. + const CFGBlock *Exit = &FunctionCFG.getExit(); + assignState(Exit, State(size(), ParameterStatus::NotCalled)); + Worklist.enqueuePredecessors(Exit); + + while (const CFGBlock *BB = Worklist.dequeue()) { + assert(BB && "Worklist should filter out null blocks"); + check(BB); + assert(CurrentState.isVisited() && + "After the check, basic block should be visited"); + + // Traverse successor basic blocks if the status of this block + // has changed. + if (assignState(BB, CurrentState)) { + Worklist.enqueuePredecessors(BB); + } + } + + // Check that we have all tracked parameters at the last block. + // As we are performing a backward version of the analysis, + // it should be the ENTRY block. + checkEntry(&FunctionCFG.getEntry()); + } + + void check(const CFGBlock *BB) { + // We start with a state 'inherited' from all the successors. + CurrentState = joinSuccessors(BB); + assert(CurrentState.isVisited() && + "Shouldn't start with a 'not visited' state"); + + // This is the 'exit' situation, broken promises are probably OK + // in such scenarios. + if (BB->hasNoReturnElement()) { + markNoReturn(); + // This block still can have calls (even multiple calls) and + // for this reason there is no early return here. + } + + // We use a backward dataflow propagation and for this reason we + // should travers basic blocks bottom-up. + for (const CFGElement &Element : llvm::reverse(*BB)) { + if (Optional S = Element.getAs()) { + check(S->getStmt()); + } + } + } + void check(const Stmt *S) { Visit(S); } + + void checkEntry(const CFGBlock *Entry) { + // We finalize this algorithm with the ENTRY block because + // we use a backward version of the analysis. This is where + // we can judge that some of the tracked parameters are not called on + // every path from ENTRY to EXIT. + + const State &EntryStatus = getState(Entry); + llvm::BitVector NotCalledOnEveryPath(size(), false); + llvm::BitVector NotUsedOnEveryPath(size(), false); + + // Check if there are no calls of the marked parameter at all + for (const auto &IndexedStatus : llvm::enumerate(EntryStatus)) { + const ParmVarDecl *Parameter = getParameter(IndexedStatus.index()); + + switch (IndexedStatus.value().getKind()) { + case ParameterStatus::NotCalled: + // If there were places where this parameter escapes (aka being used), + // we can provide a more useful diagnostic by pointing at the exact + // branches where it is not even mentioned. + if (!wasEverUsed(IndexedStatus.index())) { + // This parameter is was not used at all, so we should report the + // most generic version of the warning. + if (isCaptured(Parameter)) { + // We want to specify that it was captured by the block. + Handler.handleCapturedNeverCalled(Parameter, AC.getDecl(), + !isExplicitlyMarked(Parameter)); + } else { + Handler.handleNeverCalled(Parameter, + !isExplicitlyMarked(Parameter)); + } + } else { + // Mark it as 'interesting' to figure out which paths don't even + // have escapes. + NotUsedOnEveryPath[IndexedStatus.index()] = true; + } + + break; + case ParameterStatus::MaybeCalled: + // If we have 'maybe called' at this point, we have an error + // that there is at least one path where this parameter + // is not called. + // + // However, reporting the warning with only that information can be + // too vague for the users. For this reason, we mark such parameters + // as "interesting" for further analysis. + NotCalledOnEveryPath[IndexedStatus.index()] = true; + break; + default: + break; + } + } + + // Early exit if we don't have parameters for extra analysis. + if (NotCalledOnEveryPath.none() && NotUsedOnEveryPath.none()) + return; + + // We are looking for a pair of blocks A, B so that the following is true: + // * A is a predecessor of B + // * B is marked as NotCalled + // * A has at least one successor marked as either + // Escaped or DefinitelyCalled + // + // In that situation, it is guaranteed that B is the first block of the path + // where the user doesn't call or use parameter in question. + // + // For this reason, branch A -> B can be used for reporting. + // + // This part of the algorithm is guarded by a condition that the function + // does indeed have a violation of contract. For this reason, we can + // spend more time to find a good spot to place the warning. + // + // The following algorithm has the worst case complexity of O(N + E), + // where N is the number of basic blocks in FunctionCFG, + // E is the number of edges between blocks in FunctionCFG. + for (const CFGBlock *BB : FunctionCFG) { + if (!BB) + continue; + + const State &BlockState = getState(BB); + + for (unsigned Index : llvm::seq(0u, size())) { + // We don't want to use 'isLosingCall' here because we want to report + // the following situation as well: + // + // MaybeCalled + // | ... | + // MaybeCalled NotCalled + // + // Even though successor is not 'DefinitelyCalled', it is still useful + // to report it, it is still a path without a call. + if (NotCalledOnEveryPath[Index] && + BlockState.getKindFor(Index) == ParameterStatus::MaybeCalled) { + + findAndReportNotCalledBranches(BB, Index); + } else if (NotUsedOnEveryPath[Index] && + isLosingEscape(BlockState, BB, Index)) { + + findAndReportNotCalledBranches(BB, Index, /* IsEscape = */ true); + } + } + } + } + + /// Check potential call of a tracked parameter. + void checkDirectCall(const CallExpr *Call) { + if (auto Index = getIndexOfCallee(Call)) { + processCallFor(*Index, Call); + } + } + + /// Check the call expression for being an indirect call of one of the tracked + /// parameters. It is indirect in the sense that this particular call is not + /// calling the parameter itself, but rather uses it as the argument. + template + void checkIndirectCall(const CallLikeExpr *CallOrMessage) { + // CallExpr::arguments does not interact nicely with llvm::enumerate. + llvm::ArrayRef Arguments = llvm::makeArrayRef( + CallOrMessage->getArgs(), CallOrMessage->getNumArgs()); + + // 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) { + // Otherwise, we mark this parameter as escaped, which can be + // interpreted both as called or not called depending on the context. + CurrentParamStatus = ParameterStatus::Escaped; + } + // Otherwise, let's keep the state as it is. + } + } + } + + /// Process call of the parameter with the given index + void processCallFor(unsigned Index, const Expr *Call) { + ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(Index); + + if (CurrentParamStatus.canBeCalled()) { + + // At this point, this parameter was called, so this is a second call. + const ParmVarDecl *Parameter = getParameter(Index); + Handler.handleDoubleCall( + Parameter, &CurrentState.getCallFor(Index), Call, + !isExplicitlyMarked(Parameter), + // We are sure that the second call is definitely + // going to happen if the status is 'DefinitelyCalled'. + CurrentParamStatus.getKind() == ParameterStatus::DefinitelyCalled); + + // Mark this parameter as already reported on, so we don't repeat + // warnings. + CurrentParamStatus = ParameterStatus::Reported; + + } else if (CurrentParamStatus.getKind() != ParameterStatus::Reported) { + // If we didn't report anything yet, let's mark this parameter + // as called. + ParameterStatus Called(ParameterStatus::DefinitelyCalled, Call); + CurrentParamStatus = Called; + } + } + + void findAndReportNotCalledBranches(const CFGBlock *Parent, unsigned Index, + bool IsEscape = false) { + for (const CFGBlock *Succ : Parent->succs()) { + if (!Succ) + continue; + + if (getState(Succ).getKindFor(Index) == ParameterStatus::NotCalled) { + assert(Parent->succ_size() >= 2 && + "Block should have at least two successors at this point"); + if (auto Clarification = NotCalledClarifier::clarify(Parent, Succ)) { + const ParmVarDecl *Parameter = getParameter(Index); + Handler.handleNeverCalled(Parameter, Clarification->Location, + Clarification->Reason, !IsEscape, + !isExplicitlyMarked(Parameter)); + } + } + } + } + + //===----------------------------------------------------------------------===// + // Predicate functions to check parameters + //===----------------------------------------------------------------------===// + + /// Return true if parameter is explicitly marked as 'called_once'. + static bool isExplicitlyMarked(const ParmVarDecl *Parameter) { + return Parameter->hasAttr(); + } + + /// Return true if the given name matches conventional pattens. + static bool isConventional(llvm::StringRef Name) { + return llvm::count(CONVENTIONAL_NAMES, Name) != 0; + } + + /// Return true if the given name has conventional suffixes. + static bool hasConventionalSuffix(llvm::StringRef Name) { + return llvm::any_of(CONVENTIONAL_SUFFIXES, [Name](llvm::StringRef Suffix) { + return Name.endswith(Suffix); + }); + } + + /// Return true if the given type can be used for conventional parameters. + static bool isConventional(QualType Ty) { + if (!Ty->isBlockPointerType()) { + return false; + } + + QualType BlockType = Ty->getAs()->getPointeeType(); + // Completion handlers should have a block type with void return type. + return BlockType->getAs()->getReturnType()->isVoidType(); + } + + /// Return true if the only parameter of the function is conventional. + static bool isOnlyParameterConventional(const FunctionDecl *Function) { + return Function->getNumParams() == 1 && + hasConventionalSuffix(Function->getName()); + } + + /// Return true/false if 'swift_async' attribute states that the given + /// parameter is conventionally called once. + /// Return llvm::None if the given declaration doesn't have 'swift_async' + /// attribute. + static llvm::Optional isConventionalSwiftAsync(const Decl *D, + unsigned ParamIndex) { + if (const SwiftAsyncAttr *A = D->getAttr()) { + if (A->getKind() == SwiftAsyncAttr::None) { + return false; + } else { + return A->getCompletionHandlerIndex().getASTIndex() == ParamIndex; + } + } + return llvm::None; + } + + /// Return true if the specified selector piece matches conventions. + static bool isConventionalSelectorPiece(Selector MethodSelector, + unsigned PieceIndex, + QualType PieceType) { + if (!isConventional(PieceType)) { + return false; + } + + if (MethodSelector.getNumArgs() == 1) { + assert(PieceIndex == 0); + return hasConventionalSuffix(MethodSelector.getNameForSlot(0)); + } + + return isConventional(MethodSelector.getNameForSlot(PieceIndex)); + } + + bool shouldBeCalledOnce(const ParmVarDecl *Parameter) const { + return isExplicitlyMarked(Parameter) || + (CheckConventionalParameters && + isConventional(Parameter->getName()) && + isConventional(Parameter->getType())); + } + + bool shouldBeCalledOnce(const DeclContext *ParamContext, + const ParmVarDecl *Param) { + unsigned ParamIndex = Param->getFunctionScopeIndex(); + if (const auto *Function = dyn_cast(ParamContext)) { + return shouldBeCalledOnce(Function, ParamIndex); + } + if (const auto *Method = dyn_cast(ParamContext)) { + return shouldBeCalledOnce(Method, ParamIndex); + } + return shouldBeCalledOnce(Param); + } + + bool shouldBeCalledOnce(const BlockDecl *Block, unsigned ParamIndex) const { + return shouldBeCalledOnce(Block->getParamDecl(ParamIndex)); + } + + bool shouldBeCalledOnce(const FunctionDecl *Function, + unsigned ParamIndex) const { + if (ParamIndex >= Function->getNumParams()) { + return false; + } + // 'swift_async' goes first and overrides anything else. + if (auto ConventionalAsync = + isConventionalSwiftAsync(Function, ParamIndex)) { + return ConventionalAsync.getValue(); + } + + return shouldBeCalledOnce(Function->getParamDecl(ParamIndex)) || + (CheckConventionalParameters && + isOnlyParameterConventional(Function)); + } + + bool shouldBeCalledOnce(const ObjCMethodDecl *Method, + unsigned ParamIndex) const { + Selector MethodSelector = Method->getSelector(); + if (ParamIndex >= MethodSelector.getNumArgs()) { + return false; + } + + // 'swift_async' goes first and overrides anything else. + if (auto ConventionalAsync = isConventionalSwiftAsync(Method, ParamIndex)) { + return ConventionalAsync.getValue(); + } + + const ParmVarDecl *Parameter = Method->getParamDecl(ParamIndex); + return shouldBeCalledOnce(Parameter) || + (CheckConventionalParameters && + isConventionalSelectorPiece(MethodSelector, ParamIndex, + Parameter->getType())); + } + + bool shouldBeCalledOnce(const CallExpr *Call, unsigned ParamIndex) const { + const FunctionDecl *Function = Call->getDirectCallee(); + return Function && shouldBeCalledOnce(Function, ParamIndex); + } + + bool shouldBeCalledOnce(const ObjCMessageExpr *Message, + unsigned ParamIndex) const { + const ObjCMethodDecl *Method = Message->getMethodDecl(); + return Method && ParamIndex < Method->param_size() && + (shouldBeCalledOnce(Method->getParamDecl(ParamIndex)) || + shouldBeCalledOnce(Method, ParamIndex)); + } + + //===----------------------------------------------------------------------===// + // Utility methods + //===----------------------------------------------------------------------===// + + bool isCaptured(const ParmVarDecl *Parameter) const { + if (const BlockDecl *Block = dyn_cast(AC.getDecl())) { + return Block->capturesVariable(Parameter); + } + return false; + } + + /// Return true if the analyzed function is actually a default implementation + /// of the method that has to be overriden. + /// + /// These functions can have tracked parameters, but wouldn't call them + /// because they are not designed to perform any meaningful actions. + /// + /// There are a couple of flavors of such default implementations: + /// 1. Empty methods or methods with a single return statement + /// 2. Methods that have one block with a call to no return function + /// 3. Methods with only assertion-like operations + bool isPossiblyEmptyImpl() const { + if (!isa(AC.getDecl())) { + // We care only about functions that are not supposed to be called. + // Only methods can be overriden. + return false; + } + + // Case #1 (without return statements) + if (FunctionCFG.size() == 2) { + // Method has only two blocks: ENTRY and EXIT. + // This is equivalent to empty function. + return true; + } + + // Case #2 + if (FunctionCFG.size() == 3) { + const CFGBlock &Entry = FunctionCFG.getEntry(); + if (Entry.succ_empty()) { + return false; + } + + const CFGBlock *OnlyBlock = *Entry.succ_begin(); + // Method has only one block, let's see if it has a no-return + // element. + if (OnlyBlock && OnlyBlock->hasNoReturnElement()) { + return true; + } + // Fallthrough, CFGs with only one block can fall into #1 and #3 as well. + } + + // Cases #1 (return statements) and #3. + // + // It is hard to detect that something is an assertion or came + // from assertion. Here we use a simple heuristic: + // + // - If it came from a macro, it can be an assertion. + // + // Additionally, we can't assume a number of basic blocks or the CFG's + // structure because assertions might include loops and conditions. + return llvm::all_of(FunctionCFG, [](const CFGBlock *BB) { + if (!BB) { + // Unreachable blocks are totally fine. + return true; + } + + // Return statements can have sub-expressions that are represented as + // separate statements of a basic block. We should allow this. + // This parent map will be initialized with a parent tree for all + // subexpressions of the block's return statement (if it has one). + std::unique_ptr ReturnChildren; + + return llvm::all_of( + llvm::reverse(*BB), // we should start with return statements, if we + // have any, i.e. from the bottom of the block + [&ReturnChildren](const CFGElement &Element) { + if (Optional S = Element.getAs()) { + const Stmt *SuspiciousStmt = S->getStmt(); + + if (isa(SuspiciousStmt)) { + // Let's initialize this structure to test whether + // some further statement is a part of this return. + ReturnChildren = std::make_unique( + const_cast(SuspiciousStmt)); + // Return statements are allowed as part of #1. + return true; + } + + return SuspiciousStmt->getBeginLoc().isMacroID() || + (ReturnChildren && + ReturnChildren->getParent(SuspiciousStmt)); + } + return true; + }); + }); + } + + /// Check if parameter with the given index was ever used. + /// NOTE: This function checks only for escapes. + bool wasEverUsed(unsigned Index) const { + return llvm::any_of(States, [Index](const State &StateForOneBB) { + return StateForOneBB.getKindFor(Index) == ParameterStatus::Escaped; + }); + } + + /// Return status stored for the given basic block. + /// \{ + State &getState(const CFGBlock *BB) { + assert(BB); + return States[BB->getBlockID()]; + } + const State &getState(const CFGBlock *BB) const { + assert(BB); + return States[BB->getBlockID()]; + } + /// \} + + /// Assign status to the given basic block. + /// + /// Returns true when the stored status changed. + bool assignState(const CFGBlock *BB, const State &ToAssign) { + State &Current = getState(BB); + if (Current == ToAssign) { + return false; + } + + Current = ToAssign; + return true; + } + + /// Join all incoming statuses for the given basic block. + State joinSuccessors(const CFGBlock *BB) const { + auto Succs = + llvm::make_filter_range(BB->succs(), [this](const CFGBlock *Succ) { + return Succ && this->getState(Succ).isVisited(); + }); + // We came to this block from somewhere after all. + assert(!Succs.empty() && + "Basic block should have at least one visited successor"); + + State Result = getState(*Succs.begin()); + + for (const CFGBlock *Succ : llvm::drop_begin(Succs, 1)) { + Result.join(getState(Succ)); + } + + if (const Expr *Condition = getCondition(BB->getTerminatorStmt())) { + handleConditional(BB, Condition, Result); + } + + return Result; + } + + void handleConditional(const CFGBlock *BB, const Expr *Condition, + State &ToAlter) const { + handleParameterCheck(BB, Condition, ToAlter); + if (SuppressOnConventionalErrorPaths) { + handleConventionalCheck(BB, Condition, ToAlter); + } + } + + void handleParameterCheck(const CFGBlock *BB, const Expr *Condition, + State &ToAlter) const { + // In this function, we try to deal with the following pattern: + // + // if (parameter) + // parameter(...); + // + // It's not good to show a warning here because clearly 'parameter' + // couldn't and shouldn't be called on the 'else' path. + // + // Let's check if this if statement has a check involving one of + // the tracked parameters. + if (const ParmVarDecl *Parameter = findReferencedParmVarDecl( + Condition, + /* ShouldRetrieveFromComparisons = */ true)) { + if (const auto Index = getIndex(*Parameter)) { + ParameterStatus &CurrentStatus = ToAlter.getStatusFor(*Index); + + // We don't want to deep dive into semantics of the check and + // figure out if that check was for null or something else. + // We simply trust the user that they know what they are doing. + // + // For this reason, in the following loop we look for the + // best-looking option. + for (const CFGBlock *Succ : BB->succs()) { + if (!Succ) + continue; + + const ParameterStatus &StatusInSucc = + getState(Succ).getStatusFor(*Index); + + if (StatusInSucc.isErrorStatus()) { + continue; + } + + // Let's use this status instead. + CurrentStatus = StatusInSucc; + + if (StatusInSucc.getKind() == ParameterStatus::DefinitelyCalled) { + // This is the best option to have and we already found it. + break; + } + + // If we found 'Escaped' first, we still might find 'DefinitelyCalled' + // on the other branch. And we prefer the latter. + } + } + } + } + + void handleConventionalCheck(const CFGBlock *BB, const Expr *Condition, + State &ToAlter) const { + // Even when the analyzer is technically correct, it is a widespread pattern + // not to call completion handlers in some scenarios. These usually have + // typical conditional names, such as 'error' or 'cancel'. + if (!mentionsAnyOfConventionalNames(Condition)) { + return; + } + + for (const auto &IndexedStatus : llvm::enumerate(ToAlter)) { + const ParmVarDecl *Parameter = getParameter(IndexedStatus.index()); + // Conventions do not apply to explicitly marked parameters. + if (isExplicitlyMarked(Parameter)) { + continue; + } + + ParameterStatus &CurrentStatus = IndexedStatus.value(); + // If we did find that on one of the branches the user uses the callback + // and doesn't on the other path, we believe that they know what they are + // doing and trust them. + // + // There are two possible scenarios for that: + // 1. Current status is 'MaybeCalled' and one of the branches is + // 'DefinitelyCalled' + // 2. Current status is 'NotCalled' and one of the branches is 'Escaped' + if (isLosingCall(ToAlter, BB, IndexedStatus.index()) || + isLosingEscape(ToAlter, BB, IndexedStatus.index())) { + CurrentStatus = ParameterStatus::Escaped; + } + } + } + + bool isLosingCall(const State &StateAfterJoin, const CFGBlock *JoinBlock, + unsigned ParameterIndex) const { + // Let's check if the block represents DefinitelyCalled -> MaybeCalled + // transition. + return isLosingJoin(StateAfterJoin, JoinBlock, ParameterIndex, + ParameterStatus::MaybeCalled, + ParameterStatus::DefinitelyCalled); + } + + bool isLosingEscape(const State &StateAfterJoin, const CFGBlock *JoinBlock, + unsigned ParameterIndex) const { + // Let's check if the block represents Escaped -> NotCalled transition. + return isLosingJoin(StateAfterJoin, JoinBlock, ParameterIndex, + ParameterStatus::NotCalled, ParameterStatus::Escaped); + } + + bool isLosingJoin(const State &StateAfterJoin, const CFGBlock *JoinBlock, + unsigned ParameterIndex, ParameterStatus::Kind AfterJoin, + ParameterStatus::Kind BeforeJoin) const { + assert(!ParameterStatus::isErrorStatus(BeforeJoin) && + ParameterStatus::isErrorStatus(AfterJoin) && + "It's not a losing join if statuses do not represent " + "correct-to-error transition"); + + const ParameterStatus &CurrentStatus = + StateAfterJoin.getStatusFor(ParameterIndex); + + return CurrentStatus.getKind() == AfterJoin && + isAnyOfSuccessorsHasStatus(JoinBlock, ParameterIndex, BeforeJoin); + } + + /// Return true if any of the successors of the given basic block has + /// a specified status for the given parameter. + bool isAnyOfSuccessorsHasStatus(const CFGBlock *Parent, + unsigned ParameterIndex, + ParameterStatus::Kind ToFind) const { + return llvm::any_of( + Parent->succs(), [this, ParameterIndex, ToFind](const CFGBlock *Succ) { + return Succ && getState(Succ).getKindFor(ParameterIndex) == ToFind; + }); + } + + /// Check given expression that was discovered to escape. + void checkEscapee(const Expr *E) { + if (const ParmVarDecl *Parameter = findReferencedParmVarDecl(E)) { + checkEscapee(*Parameter); + } + } + + /// 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; + } + } + } + + /// Mark all parameters in the current state as 'no-return'. + void markNoReturn() { + for (ParameterStatus &PS : CurrentState) { + PS = ParameterStatus::NoReturn; + } + } + + /// Check if the given assignment represents suppression and act on it. + void checkSuppression(const BinaryOperator *Assignment) { + // Suppression has the following form: + // parameter = 0; + // 0 can be of any form (NULL, nil, etc.) + if (auto Index = getIndexOfExpression(Assignment->getLHS())) { + + // We don't care what is written in the RHS, it could be whatever + // we can interpret as 0. + if (auto Constant = + Assignment->getRHS()->IgnoreParenCasts()->getIntegerConstantExpr( + AC.getASTContext())) { + + ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(*Index); + + if (0 == *Constant && CurrentParamStatus.canBeCalled()) { + // Even though this suppression mechanism is introduced to tackle + // false positives for multiple calls, the fact that the user has + // to use suppression can also tell us that we couldn't figure out + // how different paths cancel each other out. And if that is true, + // we will most certainly have false positives about parameters not + // being called on certain paths. + // + // For this reason, we abandon tracking this parameter altogether. + CurrentParamStatus = ParameterStatus::Reported; + } + } + } + } + +public: + //===----------------------------------------------------------------------===// + // Tree traversal methods + //===----------------------------------------------------------------------===// + + void VisitCallExpr(const CallExpr *Call) { + // This call might be a direct call, i.e. a parameter call... + checkDirectCall(Call); + // ... or an indirect call, i.e. when parameter is an argument. + checkIndirectCall(Call); + } + + void VisitObjCMessageExpr(const ObjCMessageExpr *Message) { + // The most common situation that we are defending against here is + // copying a tracked parameter. + if (const Expr *Receiver = Message->getInstanceReceiver()) { + checkEscapee(Receiver); + } + // Message expressions unlike calls, could not be direct. + checkIndirectCall(Message); + } + + void VisitBlockExpr(const BlockExpr *Block) { + for (const auto &Capture : Block->getBlockDecl()->captures()) { + // If a block captures a tracked parameter, it should be + // considered escaped. + // On one hand, blocks that do that should definitely call it on + // every path. However, it is not guaranteed that the block + // itself gets called whenever it gets created. + // + // Because we don't want to track blocks and whether they get called, + // we consider such parameters simply escaped. + if (const auto *Param = dyn_cast(Capture.getVariable())) { + checkEscapee(*Param); + } + } + } + + void VisitBinaryOperator(const BinaryOperator *Op) { + if (Op->getOpcode() == clang::BO_Assign) { + // Let's check if one of the tracked parameters is assigned into + // something, and if it is we don't want to track extra variables, so we + // consider it as an escapee. + checkEscapee(Op->getRHS()); + + // Let's check whether this assignment is a suppression. + checkSuppression(Op); + } + } + + void VisitDeclStmt(const DeclStmt *DS) { + // Variable initialization is not assignment and should be handled + // separately. + // + // Multiple declarations can be a part of declaration statement. + for (const auto *Declaration : DS->getDeclGroup()) { + if (const auto *Var = dyn_cast(Declaration)) { + if (Var->getInit()) { + checkEscapee(Var->getInit()); + } + } + } + } + + void VisitCStyleCastExpr(const CStyleCastExpr *Cast) { + // We consider '(void)parameter' as a manual no-op escape. + // It should be used to explicitly tell the analysis that this parameter + // is intentionally not called on this path. + if (Cast->getType() == AC.getASTContext().VoidTy) { + checkEscapee(Cast->getSubExpr()); + } + } + + void VisitObjCAtThrowStmt(const ObjCAtThrowStmt *) { + // It is OK not to call marked parameters on exceptional paths. + markNoReturn(); + } + +private: + unsigned size() const { return TrackedParams.size(); } + + llvm::Optional getIndexOfCallee(const CallExpr *Call) const { + return getIndexOfExpression(Call->getCallee()); + } + + llvm::Optional getIndexOfExpression(const Expr *E) const { + if (const ParmVarDecl *Parameter = findReferencedParmVarDecl(E)) { + return getIndex(*Parameter); + } + + return llvm::None; + } + + llvm::Optional getIndex(const ParmVarDecl &Parameter) const { + // Expected number of parameters that we actually track is 1. + // + // Also, the maximum number of declared parameters could not be on a scale + // of hundreds of thousands. + // + // In this setting, linear search seems reasonable and even performs better + // than bisection. + ParamSizedVector::const_iterator It = + llvm::find(TrackedParams, &Parameter); + + if (It != TrackedParams.end()) { + return It - TrackedParams.begin(); + } + + return llvm::None; + } + + const ParmVarDecl *getParameter(unsigned Index) const { + assert(Index < TrackedParams.size()); + return TrackedParams[Index]; + } + + const CFG &FunctionCFG; + AnalysisDeclContext &AC; + CalledOnceCheckHandler &Handler; + bool CheckConventionalParameters; + // As of now, we turn this behavior off. So, we still are going to report + // missing calls on paths that look like it was intentional. + // Technically such reports are true positives, but they can make some users + // grumpy because of the sheer number of warnings. + // It can be turned back on if we decide that we want to have the other way + // around. + bool SuppressOnConventionalErrorPaths = false; + + State CurrentState; + ParamSizedVector TrackedParams; + CFGSizedVector States; +}; + +} // end anonymous namespace + +namespace clang { +void checkCalledOnceParameters(const CFG &FunctionCFG, AnalysisDeclContext &AC, + CalledOnceCheckHandler &Handler, + bool CheckConventionalParameters) { + CalledOnceChecker::check(FunctionCFG, AC, Handler, + CheckConventionalParameters); +} +} // end namespace clang diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -24,6 +24,7 @@ #include "clang/AST/StmtObjC.h" #include "clang/AST/StmtVisitor.h" #include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h" +#include "clang/Analysis/Analyses/CalledOnceCheck.h" #include "clang/Analysis/Analyses/Consumed.h" #include "clang/Analysis/Analyses/ReachableCode.h" #include "clang/Analysis/Analyses/ThreadSafety.h" @@ -36,6 +37,7 @@ #include "clang/Lex/Preprocessor.h" #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaInternal.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/BitVector.h" #include "llvm/ADT/MapVector.h" #include "llvm/ADT/SmallString.h" @@ -1623,6 +1625,82 @@ }); } }; + +class CalledOnceCheckReporter : public CalledOnceCheckHandler { +public: + CalledOnceCheckReporter(Sema &S) : S(S) {} + void handleDoubleCall(const ParmVarDecl *Parameter, const Expr *Call, + const Expr *PrevCall, bool IsCompletionHandler, + bool Poised) override { + auto DiagToReport = IsCompletionHandler + ? diag::warn_completion_handler_called_twice + : diag::warn_called_once_gets_called_twice; + S.Diag(Call->getBeginLoc(), DiagToReport) << Parameter; + S.Diag(PrevCall->getBeginLoc(), diag::note_called_once_gets_called_twice) + << Poised; + } + + void handleNeverCalled(const ParmVarDecl *Parameter, + bool IsCompletionHandler) override { + auto DiagToReport = IsCompletionHandler + ? diag::warn_completion_handler_never_called + : diag::warn_called_once_never_called; + S.Diag(Parameter->getBeginLoc(), DiagToReport) + << Parameter << /* Captured */ false; + } + + void handleNeverCalled(const ParmVarDecl *Parameter, const Stmt *Where, + NeverCalledReason Reason, bool IsCalledDirectly, + bool IsCompletionHandler) override { + auto DiagToReport = IsCompletionHandler + ? diag::warn_completion_handler_never_called_when + : diag::warn_called_once_never_called_when; + S.Diag(Where->getBeginLoc(), DiagToReport) + << Parameter << IsCalledDirectly << (unsigned)Reason; + } + + void handleCapturedNeverCalled(const ParmVarDecl *Parameter, + const Decl *Where, + bool IsCompletionHandler) override { + auto DiagToReport = IsCompletionHandler + ? diag::warn_completion_handler_never_called + : diag::warn_called_once_never_called; + S.Diag(Where->getBeginLoc(), DiagToReport) + << Parameter << /* Captured */ true; + } + +private: + Sema &S; +}; + +constexpr unsigned CalledOnceWarnings[] = { + diag::warn_called_once_never_called, + diag::warn_called_once_never_called_when, + diag::warn_called_once_gets_called_twice}; + +constexpr unsigned CompletionHandlerWarnings[]{ + diag::warn_completion_handler_never_called, + diag::warn_completion_handler_never_called_when, + diag::warn_completion_handler_called_twice}; + +bool shouldAnalyzeCalledOnceImpl(llvm::ArrayRef DiagIDs, + const DiagnosticsEngine &Diags, + SourceLocation At) { + return llvm::any_of(DiagIDs, [&Diags, At](unsigned DiagID) { + return !Diags.isIgnored(DiagID, At); + }); +} + +bool shouldAnalyzeCalledOnceConventions(const DiagnosticsEngine &Diags, + SourceLocation At) { + return shouldAnalyzeCalledOnceImpl(CompletionHandlerWarnings, Diags, At); +} + +bool shouldAnalyzeCalledOnceParameters(const DiagnosticsEngine &Diags, + SourceLocation At) { + return shouldAnalyzeCalledOnceImpl(CalledOnceWarnings, Diags, At) || + shouldAnalyzeCalledOnceConventions(Diags, At); +} } // anonymous namespace namespace clang { @@ -2264,6 +2342,17 @@ } } + // Check for violations of "called once" parameter properties. + if (S.getLangOpts().ObjC && + shouldAnalyzeCalledOnceParameters(Diags, D->getBeginLoc())) { + if (CFG *FunctionCFG = AC.getCFG()) { + CalledOnceCheckReporter Reporter(S); + checkCalledOnceParameters( + *FunctionCFG, AC, Reporter, + shouldAnalyzeCalledOnceConventions(Diags, D->getBeginLoc())); + } + } + bool FallThroughDiagFull = !Diags.isIgnored(diag::warn_unannotated_fallthrough, D->getBeginLoc()); bool FallThroughDiagPerFunction = !Diags.isIgnored( diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3612,6 +3612,26 @@ S.Context, AL, EncodingIndices.data(), EncodingIndices.size())); } +static bool isFunctionLike(const Type &T) { + // Check for explicit function types. + // 'called_once' is only supported in Objective-C and it has + // function pointers and block pointers. + return T.isFunctionPointerType() || T.isBlockPointerType(); +} + +/// Handle 'called_once' attribute. +static void handleCalledOnceAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + // 'called_once' only applies to parameters representing functions. + QualType T = cast(D)->getType(); + + if (!isFunctionLike(*T)) { + S.Diag(AL.getLoc(), diag::err_called_once_attribute_wrong_type); + return; + } + + D->addAttr(::new (S.Context) CalledOnceAttr(S.Context, AL)); +} + static void handleTransparentUnionAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // Try to find the underlying union declaration. RecordDecl *RD = nullptr; @@ -7611,6 +7631,9 @@ case ParsedAttr::AT_Callback: handleCallbackAttr(S, D, AL); break; + case ParsedAttr::AT_CalledOnce: + handleCalledOnceAttr(S, D, AL); + break; case ParsedAttr::AT_CUDAGlobal: handleGlobalAttr(S, D, AL); break; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -15398,10 +15398,6 @@ PopDeclContext(); - // Pop the block scope now but keep it alive to the end of this function. - AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy(); - PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(&WP, BD, BlockTy); - // Set the captured variables on the block. SmallVector Captures; for (Capture &Cap : BSI->Captures) { @@ -15469,6 +15465,10 @@ } BD->setCaptures(Context, Captures, BSI->CXXThisCaptureIndex != 0); + // Pop the block scope now but keep it alive to the end of this function. + AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy(); + PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(&WP, BD, BlockTy); + BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy); // If the block isn't obviously global, i.e. it captures anything at diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -39,6 +39,7 @@ // CHECK-NEXT: CXX11NoReturn (SubjectMatchRule_function) // CHECK-NEXT: CallableWhen (SubjectMatchRule_function_is_member) // CHECK-NEXT: Callback (SubjectMatchRule_function) +// CHECK-NEXT: CalledOnce (SubjectMatchRule_variable_is_parameter) // CHECK-NEXT: Capability (SubjectMatchRule_record, SubjectMatchRule_type_alias) // CHECK-NEXT: CarriesDependency (SubjectMatchRule_variable_is_parameter, SubjectMatchRule_objc_method, SubjectMatchRule_function) // CHECK-NEXT: Cleanup (SubjectMatchRule_variable_is_local) diff --git a/clang/test/SemaObjC/attr-called-once.m b/clang/test/SemaObjC/attr-called-once.m new file mode 100644 --- /dev/null +++ b/clang/test/SemaObjC/attr-called-once.m @@ -0,0 +1,9 @@ +// RUN: %clang_cc1 -verify -fsyntax-only -fobjc-arc -fblocks %s + +#define CALLED_ONCE __attribute__((called_once)) + +void test1(int x CALLED_ONCE); // expected-error{{'called_once' attribute only applies to function-like parameters}} +void test2(double x CALLED_ONCE); // expected-error{{'called_once' attribute only applies to function-like parameters}} + +void test3(void (*foo)() CALLED_ONCE); // no-error +void test4(int (^foo)(int) CALLED_ONCE); // no-error diff --git a/clang/test/SemaObjC/warn-called-once.m b/clang/test/SemaObjC/warn-called-once.m new file mode 100644 --- /dev/null +++ b/clang/test/SemaObjC/warn-called-once.m @@ -0,0 +1,1050 @@ +// RUN: %clang_cc1 -verify -fsyntax-only -fblocks -fobjc-exceptions -Wcompletion-handler %s + +#define NULL (void *)0 +#define nil (id)0 +#define CALLED_ONCE __attribute__((called_once)) +#define NORETURN __attribute__((noreturn)) + +@protocol NSObject +@end +@interface NSObject +- (id)copy; +- (id)class; +- autorelease; +@end + +typedef unsigned int NSUInteger; +typedef struct { +} NSFastEnumerationState; + +@interface NSArray <__covariant NSFastEnumeration> +- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState *)state objects:(id *)stackbuf count:(NSUInteger)len; +@end +@interface NSMutableArray : NSArray +- addObject:anObject; +@end +@class NSString, Protocol; +extern void NSLog(NSString *format, ...); + +void escape(void (^callback)(void)); +void escape_void(void *); +void indirect_call(void (^callback)(void) CALLED_ONCE); +void indirect_conv(void (^completionHandler)(void)); +void filler(void); +void exit(int) NORETURN; + +void double_call_one_block(void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void double_call_one_block_parens(void (^callback)(void) CALLED_ONCE) { + (callback)(); // expected-note{{previous call is here}} + (callback)(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void double_call_one_block_ptr(void (*callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void double_call_one_block_ptr_deref(void (*callback)(void) CALLED_ONCE) { + (*callback)(); // expected-note{{previous call is here}} + (*callback)(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void multiple_call_one_block(void (^callback)(void) CALLED_ONCE) { + // We don't really need to repeat the same warning for the same parameter. + callback(); // no-warning + callback(); // no-warning + callback(); // no-warning + callback(); // expected-note{{previous call is here}} + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void double_call_branching_1(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + callback(); // expected-note{{previous call is here}} + } else { + cond += 42; + } + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void double_call_branching_2(int cond, void (^callback)(void) CALLED_ONCE) { + callback(); + // expected-note@-1{{previous call is here; set to nil to indicate it cannot be called afterwards}} + + if (cond) { + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} + } else { + cond += 42; + } +} + +void double_call_branching_3(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + callback(); + } else { + callback(); + } + // no-warning +} + +void double_call_branching_4(int cond1, int cond2, void (^callback)(void) CALLED_ONCE) { + if (cond1) { + cond2 = !cond2; + } else { + callback(); + // expected-note@-1{{previous call is here; set to nil to indicate it cannot be called afterwards}} + } + + if (cond2) { + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} + } +} + +void double_call_loop(int counter, void (^callback)(void) CALLED_ONCE) { + while (counter > 0) { + counter--; + // Both note and warning are on the same line, which is a common situation + // in loops. + callback(); // expected-note{{previous call is here}} + // expected-warning@-1{{'callback' parameter marked 'called_once' is called twice}} + } +} + +void never_called_trivial(void (^callback)(void) CALLED_ONCE) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called}} +} + +int never_called_branching(int x, void (^callback)(void) CALLED_ONCE) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called}} + x -= 42; + + if (x == 10) { + return 0; + } + + return x + 15; +} + +void escaped_one_block_1(void (^callback)(void) CALLED_ONCE) { + escape(callback); // no-warning +} + +void escaped_one_block_2(void (^callback)(void) CALLED_ONCE) { + escape(callback); // no-warning + callback(); +} + +void escaped_one_path_1(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + escape(callback); // no-warning + } else { + callback(); + } +} + +void escaped_one_path_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + escape(callback); // no-warning + } + + callback(); +} + +void escaped_one_path_3(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never used when taking false branch}} + escape(callback); + } +} + +void escape_in_between_1(void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + escape(callback); + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void escape_in_between_2(int cond, void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + if (cond) { + escape(callback); + } + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void escape_in_between_3(int cond, void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + + if (cond) { + escape(callback); + } else { + escape_void((__bridge void *)callback); + } + + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void escaped_as_void_ptr(void (^callback)(void) CALLED_ONCE) { + escape_void((__bridge void *)callback); // no-warning +} + +void indirect_call_no_warning_1(void (^callback)(void) CALLED_ONCE) { + indirect_call(callback); // no-warning +} + +void indirect_call_no_warning_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + indirect_call(callback); + } else { + callback(); + } + // no-warning +} + +void indirect_call_double_call(void (^callback)(void) CALLED_ONCE) { + indirect_call(callback); // expected-note{{previous call is here}} + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +void indirect_call_within_direct_call(void (^callback)(void) CALLED_ONCE, + void (^meta)(void (^param)(void) CALLED_ONCE) CALLED_ONCE) { + // TODO: Report warning for 'callback'. + // At the moment, it is not possible to access 'called_once' attribute from the type + // alone when there is no actual declaration of the marked parameter. + meta(callback); + callback(); + // no-warning +} + +void block_call_1(void (^callback)(void) CALLED_ONCE) { + indirect_call(^{ + callback(); + }); + callback(); + // no-warning +} + +void block_call_2(void (^callback)(void) CALLED_ONCE) { + escape(^{ + callback(); + }); + callback(); + // no-warning +} + +void block_call_3(int cond, void (^callback)(void) CALLED_ONCE) { + ^{ + if (cond) { + callback(); // expected-note{{previous call is here}} + } + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} + }(); // no-warning +} + +void block_call_4(int cond, void (^callback)(void) CALLED_ONCE) { + ^{ + if (cond) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never used when taking false branch}} + escape(callback); + } + }(); +} + +void block_call_5(void (^outer)(void) CALLED_ONCE) { + ^(void (^inner)(void) CALLED_ONCE) { + // expected-warning@-1{{'inner' parameter marked 'called_once' is never called}} + }(outer); +} + +void block_with_called_once(void (^outer)(void) CALLED_ONCE) { + escape_void((__bridge void *)^(void (^inner)(void) CALLED_ONCE) { + inner(); // expected-note{{previous call is here}} + inner(); // expected-warning{{'inner' parameter marked 'called_once' is called twice}} + }); + outer(); // expected-note{{previous call is here}} + outer(); // expected-warning{{'outer' parameter marked 'called_once' is called twice}} +} + +void never_called_one_exit(int cond, void (^callback)(void) CALLED_ONCE) { + if (!cond) // expected-warning{{'callback' parameter marked 'called_once' is never called when taking true branch}} + return; + + callback(); +} + +void never_called_if_then_1(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking true branch}} + } else { + callback(); + } +} + +void never_called_if_then_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking true branch}} + // This way the first statement in the basic block is different from + // the first statement in the compound statement + (void)cond; + } else { + callback(); + } +} + +void never_called_if_else_1(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking false branch}} + callback(); + } else { + } +} + +void never_called_if_else_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking false branch}} + callback(); + } +} + +void never_called_two_ifs(int cond1, int cond2, void (^callback)(void) CALLED_ONCE) { + if (cond1) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking false branch}} + if (cond2) { // expected-warning{{'callback' parameter marked 'called_once' is never called when taking true branch}} + return; + } + callback(); + } +} + +void never_called_ternary_then(int cond, void (^other)(void), void (^callback)(void) CALLED_ONCE) { + return cond ? // expected-warning{{'callback' parameter marked 'called_once' is never called when taking true branch}} + other() + : callback(); +} + +void never_called_for_false(int size, void (^callback)(void) CALLED_ONCE) { + for (int i = 0; i < size; ++i) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called when skipping the loop}} + callback(); + break; + } +} + +void never_called_for_true(int size, void (^callback)(void) CALLED_ONCE) { + for (int i = 0; i < size; ++i) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called when entering the loop}} + return; + } + callback(); +} + +void never_called_while_false(int cond, void (^callback)(void) CALLED_ONCE) { + while (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when skipping the loop}} + callback(); + break; + } +} + +void never_called_while_true(int cond, void (^callback)(void) CALLED_ONCE) { + while (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when entering the loop}} + return; + } + callback(); +} + +void never_called_switch_case(int cond, void (^callback)(void) CALLED_ONCE) { + switch (cond) { + case 1: + callback(); + break; + case 2: + callback(); + break; + case 3: // expected-warning{{'callback' parameter marked 'called_once' is never called when handling this case}} + break; + default: + callback(); + break; + } +} + +void never_called_switch_default(int cond, void (^callback)(void) CALLED_ONCE) { + switch (cond) { + case 1: + callback(); + break; + case 2: + callback(); + break; + default: // expected-warning{{'callback' parameter marked 'called_once' is never called when handling this case}} + break; + } +} + +void never_called_switch_two_cases(int cond, void (^callback)(void) CALLED_ONCE) { + switch (cond) { + case 1: // expected-warning{{'callback' parameter marked 'called_once' is never called when handling this case}} + break; + case 2: // expected-warning{{'callback' parameter marked 'called_once' is never called when handling this case}} + break; + default: + callback(); + break; + } +} + +void never_called_switch_none(int cond, void (^callback)(void) CALLED_ONCE) { + switch (cond) { // expected-warning{{'callback' parameter marked 'called_once' is never called when none of the cases applies}} + case 1: + callback(); + break; + case 2: + callback(); + break; + } +} + +enum YesNoOrMaybe { + YES, + NO, + MAYBE +}; + +void exhaustive_switch(enum YesNoOrMaybe cond, void (^callback)(void) CALLED_ONCE) { + switch (cond) { + case YES: + callback(); + break; + case NO: + callback(); + break; + case MAYBE: + callback(); + break; + } + // no-warning +} + +void called_twice_exceptions(void (^callback)(void) CALLED_ONCE) { + // TODO: Obj-C exceptions are not supported in CFG, + // we should report warnings in these as well. + @try { + callback(); + callback(); + } + @finally { + callback(); + } +} + +void noreturn_1(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + exit(1); + } else { + callback(); + } + // no-warning +} + +void noreturn_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + callback(); + exit(1); + } else { + callback(); + } + // no-warning +} + +void noreturn_3(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + exit(1); + } + + callback(); + // no-warning +} + +void noreturn_4(void (^callback)(void) CALLED_ONCE) { + exit(1); + // no-warning +} + +void noreturn_5(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + // NOTE: This is an ambiguous case caused by the fact that we do a backward + // analysis. We can probably report it here, but for the sake of + // the simplicity of our analysis, we don't. + if (cond == 42) { + callback(); + } + exit(1); + } + callback(); + // no-warning +} + +void never_called_noreturn_1(int cond, void (^callback)(void) CALLED_ONCE) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called}} + if (cond) { + exit(1); + } +} + +void double_call_noreturn(int cond, void (^callback)(void) CALLED_ONCE) { + callback(); // expected-note{{previous call is here}} + + if (cond) { + if (cond == 42) { + callback(); // expected-warning{{'callback' parameter marked 'called_once' is called twice}} + } + exit(1); + } +} + +void call_with_check_1(void (^callback)(void) CALLED_ONCE) { + if (callback) + callback(); + // no-warning +} + +void call_with_check_2(void (^callback)(void) CALLED_ONCE) { + if (!callback) { + } else { + callback(); + } + // no-warning +} + +void call_with_check_3(void (^callback)(void) CALLED_ONCE) { + if (callback != NULL) + callback(); + // no-warning +} + +void call_with_check_4(void (^callback)(void) CALLED_ONCE) { + if (NULL != callback) + callback(); + // no-warning +} + +void call_with_check_5(void (^callback)(void) CALLED_ONCE) { + if (callback == NULL) { + } else { + callback(); + } + // no-warning +} + +void call_with_check_6(void (^callback)(void) CALLED_ONCE) { + if (NULL == callback) { + } else { + callback(); + } + // no-warning +} + +int call_with_check_7(int (^callback)(void) CALLED_ONCE) { + return callback ? callback() : 0; + // no-warning +} + +void unreachable_true_branch(void (^callback)(void) CALLED_ONCE) { + if (0) { + + } else { + callback(); + } + // no-warning +} + +void unreachable_false_branch(void (^callback)(void) CALLED_ONCE) { + if (1) { + callback(); + } + // no-warning +} + +void never_called_conv_1(void (^completionHandler)(void)) { + // expected-warning@-1{{completion handler is never called}} +} + +void never_called_conv_2(void (^completion)(void)) { + // expected-warning@-1{{completion handler is never called}} +} + +void never_called_conv_WithCompletion(void (^callback)(void)) { + // expected-warning@-1{{completion handler is never called}} +} + +void indirectly_called_conv(void (^completionHandler)(void)) { + indirect_conv(completionHandler); + // no-warning +} + +void escape_through_assignment_1(void (^callback)(void) CALLED_ONCE) { + id escapee; + escapee = callback; + escape(escapee); + // no-warning +} + +void escape_through_assignment_2(void (^callback)(void) CALLED_ONCE) { + id escapee = callback; + escape(escapee); + // no-warning +} + +void escape_through_assignment_3(void (^callback1)(void) CALLED_ONCE, + void (^callback2)(void) CALLED_ONCE) { + id escapee1 = callback1, escapee2 = callback2; + escape(escapee1); + escape(escapee2); + // no-warning +} + +void not_called_in_throw_branch_1(id exception, void (^callback)(void) CALLED_ONCE) { + if (exception) { + @throw exception; + } + + callback(); +} + +void not_called_in_throw_branch_2(id exception, void (^callback)(void) CALLED_ONCE) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called}} + if (exception) { + @throw exception; + } +} + +void conventional_error_path_1(int error, void (^completionHandler)(void)) { + if (error) { + // expected-warning@-1{{completion handler is never called when taking true branch}} + // This behavior might be tweaked in the future + return; + } + + completionHandler(); +} + +void conventional_error_path_2(int error, void (^callback)(void) CALLED_ONCE) { + // Conventions do not apply to explicitly marked parameters. + if (error) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called when taking true branch}} + return; + } + + callback(); +} + +void suppression_1(void (^callback)(void) CALLED_ONCE) { + // This is a way to tell the analysis that we know about this path, + // and we do not want to call the callback here. + (void)callback; // no-warning +} + +void suppression_2(int cond, void (^callback)(void) CALLED_ONCE) { + if (cond) { + (void)callback; // no-warning + } else { + callback(); + } +} + +void suppression_3(int cond, void (^callback)(void) CALLED_ONCE) { + // Even if we do this on one of the paths, it doesn't mean we should + // forget about other paths. + if (cond) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never used when taking false branch}} + (void)callback; + } +} + +@interface TestBase : NSObject +- (void)escape:(void (^)(void))callback; +- (void)indirect_call:(void (^)(void))CALLED_ONCE callback; +- (void)indirect_call_conv_1:(int)cond + completionHandler:(void (^)(void))completionHandler; +- (void)indirect_call_conv_2:(int)cond + completionHandler:(void (^)(void))handler; +- (void)indirect_call_conv_3WithCompletion:(void (^)(void))handler; +- (void)indirect_call_conv_4:(void (^)(void))handler + __attribute__((swift_async(swift_private, 1))); +- (void)exit:(int)code NORETURN; +- (int)condition; +@end + +@interface TestClass : TestBase +@property(strong) NSMutableArray *handlers; +@property(strong) id storedHandler; +@property int wasCanceled; +@property(getter=hasErrors) int error; +@end + +@implementation TestClass + +- (void)double_indirect_call_1:(void (^)(void))CALLED_ONCE callback { + [self indirect_call:callback]; // expected-note{{previous call is here}} + [self indirect_call:callback]; // expected-warning{{'callback' parameter marked 'called_once' is called twice}} +} + +- (void)double_indirect_call_2:(void (^)(void))CALLED_ONCE callback { + [self indirect_call_conv_1:0 // expected-note{{previous call is here}} + completionHandler:callback]; + [self indirect_call_conv_1:1 // expected-warning{{'callback' parameter marked 'called_once' is called twice}} + completionHandler:callback]; +} + +- (void)double_indirect_call_3:(void (^)(void))completionHandler { + [self indirect_call_conv_2:0 // expected-note{{previous call is here}} + completionHandler:completionHandler]; + [self indirect_call_conv_2:1 // expected-warning{{completion handler is called twice}} + completionHandler:completionHandler]; +} + +- (void)double_indirect_call_4:(void (^)(void))completion { + [self indirect_call_conv_2:0 // expected-note{{previous call is here}} + completionHandler:completion]; + [self indirect_call_conv_2:1 // expected-warning{{completion handler is called twice}} + completionHandler:completion]; +} + +- (void)double_indirect_call_5:(void (^)(void))withCompletionHandler { + [self indirect_call_conv_2:0 // expected-note{{previous call is here}} + completionHandler:withCompletionHandler]; + [self indirect_call_conv_2:1 // expected-warning{{completion handler is called twice}} + completionHandler:withCompletionHandler]; +} + +- (void)double_indirect_call_6:(void (^)(void))completionHandler { + [self indirect_call_conv_3WithCompletion: // expected-note{{previous call is here}} + completionHandler]; + [self indirect_call_conv_3WithCompletion: // expected-warning{{completion handler is called twice}} + completionHandler]; +} + +- (void)double_indirect_call_7:(void (^)(void))completionHandler { + [self indirect_call_conv_4: // expected-note{{previous call is here}} + completionHandler]; + [self indirect_call_conv_4: // expected-warning{{completion handler is called twice}} + completionHandler]; +} + +- (void)never_called_trivial:(void (^)(void))CALLED_ONCE callback { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never called}} + filler(); +} + +- (void)noreturn:(int)cond callback:(void (^)(void))CALLED_ONCE callback { + if (cond) { + [self exit:1]; + } + + callback(); + // no-warning +} + +- (void)escaped_one_path:(int)cond callback:(void (^)(void))CALLED_ONCE callback { + if (cond) { + [self escape:callback]; // no-warning + } else { + callback(); + } +} + +- (void)block_call_1:(void (^)(void))CALLED_ONCE callback { + // We consider captures by blocks as escapes + [self indirect_call:(^{ + callback(); + })]; + callback(); + // no-warning +} + +- (void)block_call_2:(int)cond callback:(void (^)(void))CALLED_ONCE callback { + [self indirect_call: + ^{ + if (cond) { + // expected-warning@-1{{'callback' parameter marked 'called_once' is never used when taking false branch}} + [self escape:callback]; + } + }]; +} + +- (void)block_call_3:(int)cond + completionHandler:(void (^)(void))callback { + [self indirect_call: + ^{ + if (cond) { + // expected-warning@-1{{completion handler is never used when taking false branch}} + [self escape:callback]; + } + }]; +} + +- (void)block_call_4WithCompletion:(void (^)(void))callback { + [self indirect_call: + ^{ + if ([self condition]) { + // expected-warning@-1{{completion handler is never used when taking false branch}} + [self escape:callback]; + } + }]; +} + +- (void)never_called_conv:(void (^)(void))completionHandler { + // expected-warning@-1{{completion handler is never called}} + filler(); +} + +- (void)indirectly_called_conv:(void (^)(void))completionHandler { + indirect_conv(completionHandler); + // no-warning +} + +- (void)never_called_one_exit_conv:(int)cond completionHandler:(void (^)(void))handler { + if (!cond) // expected-warning{{completion handler is never called when taking true branch}} + return; + + handler(); +} + +- (void)escape_through_assignment:(void (^)(void))completionHandler { + _storedHandler = completionHandler; + // no-warning +} + +- (void)escape_through_copy:(void (^)(void))completionHandler { + _storedHandler = [completionHandler copy]; + // no-warning +} + +- (void)escape_through_copy_and_autorelease:(void (^)(void))completionHandler { + _storedHandler = [[completionHandler copy] autorelease]; + // no-warning +} + +- (void)complex_escape:(void (^)(void))completionHandler { + if (completionHandler) { + [_handlers addObject:[[completionHandler copy] autorelease]]; + } + // no-warning +} + +- (void)test_crash:(void (^)(void))completionHandler cond:(int)cond { + if (cond) { + // expected-warning@-1{{completion handler is never used when taking false branch}} + for (id _ in _handlers) { + } + + [_handlers addObject:completionHandler]; + } +} + +- (void)conventional_error_path_1:(void (^)(void))completionHandler { + if (self.wasCanceled) + // expected-warning@-1{{completion handler is never called when taking true branch}} + // This behavior might be tweaked in the future + return; + + completionHandler(); +} + +- (void)conventional_error_path_2:(void (^)(void))completionHandler { + if (self.wasCanceled) + // expected-warning@-1{{completion handler is never used when taking true branch}} + // This behavior might be tweaked in the future + return; + + [_handlers addObject:completionHandler]; +} + +- (void)conventional_error_path_3:(void (^)(void))completionHandler { + if (self.hasErrors) + // expected-warning@-1{{completion handler is never called when taking true branch}} + // This behavior might be tweaked in the future + return; + + completionHandler(); +} + +- (void)conventional_error_path_3:(int)cond completionHandler:(void (^)(void))handler { + if (self.wasCanceled) + // expected-warning@-1{{completion handler is never called when taking true branch}} + // TODO: When we have an error on some other path, in order not to prevent it from + // being reported, we report this one as well. + // Probably, we should address this at some point. + return; + + if (cond) { + // expected-warning@-1{{completion handler is never called when taking false branch}} + handler(); + } +} + +#define NSAssert(condition, desc, ...) NSLog(desc, ##__VA_ARGS__); + +- (void)empty_base_1:(void (^)(void))completionHandler { + NSAssert(0, @"Subclass must implement"); + // no-warning +} + +- (void)empty_base_2:(void (^)(void))completionHandler { + // no-warning +} + +- (int)empty_base_3:(void (^)(void))completionHandler { + return 1; + // no-warning +} + +- (int)empty_base_4:(void (^)(void))completionHandler { + NSAssert(0, @"Subclass must implement"); + return 1; + // no-warning +} + +- (int)empty_base_5:(void (^)(void))completionHandler { + NSAssert(0, @"%@ doesn't support", [self class]); + return 1; + // no-warning +} + +#undef NSAssert +#define NSAssert(condition, desc, ...) \ + if (!(condition)) { \ + NSLog(desc, ##__VA_ARGS__); \ + } + +- (int)empty_base_6:(void (^)(void))completionHandler { + NSAssert(0, @"%@ doesn't support", [self class]); + return 1; + // no-warning +} + +#undef NSAssert +#define NSAssert(condition, desc, ...) \ + do { \ + NSLog(desc, ##__VA_ARGS__); \ + } while (0) + +- (int)empty_base_7:(void (^)(void))completionHandler { + NSAssert(0, @"%@ doesn't support", [self class]); + return 1; + // no-warning +} + +- (void)two_conditions_1:(int)first + second:(int)second + completionHandler:(void (^)(void))completionHandler { + if (first && second) { + // expected-warning@-1{{completion handler is never called when taking false branch}} + completionHandler(); + } +} + +- (void)two_conditions_2:(int)first + second:(int)second + completionHandler:(void (^)(void))completionHandler { + if (first || second) { + // expected-warning@-1{{completion handler is never called when taking true branch}} + return; + } + + completionHandler(); +} + +- (void)testWithCompletionHandler:(void (^)(void))callback { + if ([self condition]) { + // expected-warning@-1{{completion handler is never called when taking false branch}} + callback(); + } +} + +- (void)testWithCompletion:(void (^)(void))callback { + if ([self condition]) { + // expected-warning@-1{{completion handler is never called when taking false branch}} + callback(); + } +} + +- (void)completion_handler_wrong_type:(int (^)(void))completionHandler { + // We don't want to consider completion handlers with non-void return types. + if ([self condition]) { + // no-warning + completionHandler(); + } +} + +- (void)test_swift_async_none:(int)cond + completionHandler:(void (^)(void))handler __attribute__((swift_async(none))) { + if (cond) { + // no-warning + handler(); + } +} + +- (void)test_swift_async_param:(int)cond + callback:(void (^)(void))callback + __attribute__((swift_async(swift_private, 2))) { + if (cond) { + // expected-warning@-1{{completion handler is never called when taking false branch}} + callback(); + } +} + +- (void)test_nil_suggestion:(int)cond1 + second:(int)cond2 + completion:(void (^)(void))handler { + if (cond1) { + handler(); + // expected-note@-1{{previous call is here; set to nil to indicate it cannot be called afterwards}} + } + + if (cond2) { + handler(); // expected-warning{{completion handler is called twice}} + } +} + +- (void)test_nil_suppression_1:(int)cond1 + second:(int)cond2 + completion:(void (^)(void))handler { + if (cond1) { + handler(); + handler = nil; + // no-warning + } + + if (cond2) { + handler(); + } +} + +- (void)test_nil_suppression_2:(int)cond1 + second:(int)cond2 + completion:(void (^)(void))handler { + if (cond1) { + handler(); + handler = NULL; + // no-warning + } + + if (cond2) { + handler(); + } +} + +- (void)test_nil_suppression_3:(int)cond1 + second:(int)cond2 + completion:(void (^)(void))handler { + if (cond1) { + handler(); + handler = 0; + // no-warning + } + + if (cond2) { + handler(); + } +} + +@end