[cfe-commits] r164854 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/ScopeInfo.h lib/Sema/AnalysisBasedWarnings.cpp lib/Sema/Sema.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.
David Blaikie
dblaikie at gmail.com
Tue Oct 2 14:43:44 PDT 2012
On Fri, Sep 28, 2012 at 3:21 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> Author: jrose
> Date: Fri Sep 28 17:21:30 2012
> New Revision: 164854
>
> URL: http://llvm.org/viewvc/llvm-project?rev=164854&view=rev
> Log:
> Add a warning (off by default) for repeated use of the same weak property.
>
> The motivating example:
>
> if (self.weakProp)
> use(self.weakProp);
I wonder if this could be generalized/modified to help for things like
std::weak_ptr too (obviously weak_ptr would need some kind of
annotation to specify its test/use operations I suppose).
> As with any non-atomic test-then-use, it is possible a weak property to be
> non-nil at the 'if', but be deallocated by the time it is used. The correct
> way to write this example is as follows:
>
> id tmp = self.weakProp;
> if (tmp)
> use(tmp);
>
> The warning is controlled by -Warc-repeated-use-of-receiver, and uses the
> property name and base to determine if the same property on the same object
> is being accessed multiple times. In cases where the base is more
> complicated than just a single Decl (e.g. 'foo.bar.weakProp'), it picks a
> Decl for some degree of uniquing and reports the problem under a subflag,
> -Warc-maybe-repeated-use-of-receiver. This gives a way to tune the
> aggressiveness of the warning for a particular project.
>
> The warning is not on by default because it is not flow-sensitive and thus
> may have a higher-than-acceptable rate of false positives,
I believe we're generally trying to avoid adding off-by-default
warnings (Doug seems to feel strongly that if it's off-by-default it's
simply not good enough). Is there a good reason to make an exception
here?
> though it is
> less noisy than -Wreceiver-is-weak. On the other hand, it will not warn
> about some cases that may be legitimate issues that -Wreceiver-is-weak
> will catch, and it does not attempt to reason about methods returning weak
> values.
>
> Even though this is not a real "analysis-based" check I've put the bug
> emission code in AnalysisBasedWarnings for two reasons: (1) to run on
> every kind of code body (function, method, block, or lambda), and (2) to
> suggest that it may be enhanced by flow-sensitive analysis in the future.
>
> The second (smaller) half of this work is to extend it to weak locals
> and weak ivars. This should use most of the same infrastructure.
>
> Part of <rdar://problem/12280249>
>
> Added:
> cfe/trunk/test/SemaObjC/arc-repeated-weak.mm
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Sema/ScopeInfo.h
> cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
> cfe/trunk/lib/Sema/Sema.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/lib/Sema/SemaPseudoObject.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=164854&r1=164853&r2=164854&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Sep 28 17:21:30 2012
> @@ -282,6 +282,9 @@
> [ARCUnsafeRetainedAssign,
> ARCRetainCycles,
> ARCNonPodMemAccess]>;
> +def ARCRepeatedUseOfWeakMaybe : DiagGroup<"arc-maybe-repeated-use-of-weak">;
> +def ARCRepeatedUseOfWeak : DiagGroup<"arc-repeated-use-of-weak",
> + [ARCRepeatedUseOfWeakMaybe]>;
> def Selector : DiagGroup<"selector">;
> def Protocol : DiagGroup<"protocol">;
> def SuperSubClassMismatch : DiagGroup<"super-class-method-mismatch">;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=164854&r1=164853&r2=164854&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep 28 17:21:30 2012
> @@ -709,6 +709,18 @@
> "weak %select{receiver|property|implicit property}0 may be "
> "unpredictably null in ARC mode">,
> InGroup<DiagGroup<"receiver-is-weak">>, DefaultIgnore;
> +def warn_arc_repeated_use_of_weak : Warning <
> + "weak property is accessed multiple times in this "
> + "%select{function|method|block|lambda}0 but may be unpredictably set to nil; "
> + "assign to a strong variable to keep the object alive">,
> + InGroup<ARCRepeatedUseOfWeak>, DefaultIgnore;
> +def warn_arc_possible_repeated_use_of_weak : Warning <
> + "weak property may be accessed multiple times in this "
> + "%select{function|method|block|lambda}0 but may be unpredictably set to nil; "
> + "assign to a strong variable to keep the object alive">,
> + InGroup<ARCRepeatedUseOfWeakMaybe>, DefaultIgnore;
> +def note_arc_weak_also_accessed_here : Note<
> + "also accessed here">;
> def err_incomplete_synthesized_property : Error<
> "cannot synthesize property %0 with incomplete type %1">;
>
>
> Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=164854&r1=164853&r2=164854&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
> +++ cfe/trunk/include/clang/Sema/ScopeInfo.h Fri Sep 28 17:21:30 2012
> @@ -21,6 +21,7 @@
>
> namespace clang {
>
> +class Decl;
> class BlockDecl;
> class CXXMethodDecl;
> class IdentifierInfo;
> @@ -29,6 +30,7 @@
> class Scope;
> class SwitchStmt;
> class VarDecl;
> +class ObjCPropertyRefExpr;
>
> namespace sema {
>
> @@ -113,6 +115,125 @@
> /// prior to being emitted.
> SmallVector<PossiblyUnreachableDiag, 4> PossiblyUnreachableDiags;
>
> +public:
> + /// Represents a simple identification of a weak object.
> + ///
> + /// Part of the implementation of -Wrepeated-use-of-weak.
> + ///
> + /// This is used to determine if two weak accesses refer to the same object.
> + /// Here are some examples of how various accesses are "profiled":
> + ///
> + /// Access Expression | "Base" Decl | "Property" Decl
> + /// :---------------: | :-----------------: | :------------------------------:
> + /// self.property | self (VarDecl) | property (ObjCPropertyDecl)
> + /// self.implicitProp | self (VarDecl) | -implicitProp (ObjCMethodDecl)
> + /// self->ivar.prop | ivar (ObjCIvarDecl) | prop (ObjCPropertyDecl)
> + /// cxxObj.obj.prop | obj (FieldDecl) | prop (ObjCPropertyDecl)
> + /// [self foo].prop | 0 (unknown) | prop (ObjCPropertyDecl)
> + /// self.prop1.prop2 | prop1 (ObjCPropertyDecl) | prop2 (ObjCPropertyDecl)
> + /// MyClass.prop | MyClass (ObjCInterfaceDecl) | -prop (ObjCMethodDecl)
> + ///
> + /// Objects are identified with only two Decls to make it reasonably fast to
> + /// compare them.
> + class WeakObjectProfileTy {
> + /// The base object decl, as described in the class documentation.
> + ///
> + /// The extra flag is "true" if the Base and Property are enough to uniquely
> + /// identify the object in memory.
> + ///
> + /// \sa isExactProfile()
> + typedef llvm::PointerIntPair<const NamedDecl *, 1, bool> BaseInfoTy;
> + BaseInfoTy Base;
> +
> + /// The "property" decl, as described in the class documentation.
> + ///
> + /// Note that this may not actually be an ObjCPropertyDecl, e.g. in the
> + /// case of "implicit" properties (regular methods accessed via dot syntax).
> + const NamedDecl *Property;
> +
> + // For use in DenseMap.
> + friend struct llvm::DenseMapInfo<WeakObjectProfileTy>;
> + inline WeakObjectProfileTy();
> + static inline WeakObjectProfileTy getSentinel();
> +
> + public:
> + WeakObjectProfileTy(const ObjCPropertyRefExpr *RE);
> +
> + const NamedDecl *getProperty() const { return Property; }
> +
> + /// Returns true if the object base specifies a known object in memory,
> + /// rather than, say, an instance variable or property of another object.
> + ///
> + /// Note that this ignores the effects of aliasing; that is, \c foo.bar is
> + /// considered an exact profile if \c foo is a local variable, even if
> + /// another variable \c foo2 refers to the same object as \c foo.
> + ///
> + /// For increased precision, accesses with base variables that are
> + /// properties or ivars of 'self' (e.g. self.prop1.prop2) are considered to
> + /// be exact, though this is not true for arbitrary variables
> + /// (foo.prop1.prop2).
> + bool isExactProfile() const {
> + return Base.getInt();
> + }
> +
> + bool operator==(const WeakObjectProfileTy &Other) const {
> + return Base == Other.Base && Property == Other.Property;
> + }
> + };
> +
> + /// Represents a single use of a weak object.
> + ///
> + /// Stores both the expression and whether the access is potentially unsafe
> + /// (i.e. it could potentially be warned about).
> + ///
> + /// Part of the implementation of -Wrepeated-use-of-weak.
> + class WeakUseTy {
> + llvm::PointerIntPair<const Expr *, 1, bool> Rep;
> + public:
> + WeakUseTy(const Expr *Use, bool IsRead) : Rep(Use, IsRead) {}
> +
> + const Expr *getUseExpr() const { return Rep.getPointer(); }
> + bool isUnsafe() const { return Rep.getInt(); }
> + void markSafe() { Rep.setInt(false); }
> +
> + bool operator==(const WeakUseTy &Other) const {
> + return Rep == Other.Rep;
> + }
> + };
> +
> + /// Used to collect uses of a particular weak object in a function body.
> + ///
> + /// Part of the implementation of -Wrepeated-use-of-weak.
> + typedef SmallVector<WeakUseTy, 4> WeakUseVector;
> +
> + /// Used to collect all uses of weak objects in a function body.
> + ///
> + /// Part of the implementation of -Wrepeated-use-of-weak.
> + typedef llvm::SmallDenseMap<WeakObjectProfileTy, WeakUseVector, 8>
> + WeakObjectUseMap;
> +
> +private:
> + /// Used to collect all uses of weak objects in this function body.
> + ///
> + /// Part of the implementation of -Wrepeated-use-of-weak.
> + WeakObjectUseMap WeakObjectUses;
> +
> +public:
> + /// Record that a weak property was accessed.
> + ///
> + /// Part of the implementation of -Wrepeated-use-of-weak.
> + void recordUseOfWeak(const ObjCPropertyRefExpr *PropE);
> +
> + /// Record that a given expression is a "safe" access of a weak object (e.g.
> + /// assigning it to a strong variable.)
> + ///
> + /// Part of the implementation of -Wrepeated-use-of-weak.
> + void markSafeWeakUse(const Expr *E);
> +
> + const WeakObjectUseMap &getWeakObjectUses() const {
> + return WeakObjectUses;
> + }
> +
> void setHasBranchIntoScope() {
> HasBranchIntoScope = true;
> }
> @@ -387,7 +508,39 @@
>
> };
>
> +FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy()
> + : Base(0, false), Property(0) {}
> +
> +FunctionScopeInfo::WeakObjectProfileTy
> +FunctionScopeInfo::WeakObjectProfileTy::getSentinel() {
> + FunctionScopeInfo::WeakObjectProfileTy Result;
> + Result.Base.setInt(true);
> + return Result;
> +}
> +
> }
> }
>
> +namespace llvm {
> + template <>
> + struct DenseMapInfo<clang::sema::FunctionScopeInfo::WeakObjectProfileTy> {
> + typedef clang::sema::FunctionScopeInfo::WeakObjectProfileTy ProfileTy;
> + static inline ProfileTy getEmptyKey() {
> + return ProfileTy();
> + }
> + static inline ProfileTy getTombstoneKey() {
> + return ProfileTy::getSentinel();
> + }
> +
> + static unsigned getHashValue(const ProfileTy &Val) {
> + typedef std::pair<ProfileTy::BaseInfoTy, const clang::NamedDecl *> Pair;
> + return DenseMapInfo<Pair>::getHashValue(Pair(Val.Base, Val.Property));
> + }
> +
> + static bool isEqual(const ProfileTy &LHS, const ProfileTy &RHS) {
> + return LHS == RHS;
> + }
> + };
> +} // end namespace llvm
> +
> #endif
>
> Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=164854&r1=164853&r2=164854&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
> +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Fri Sep 28 17:21:30 2012
> @@ -871,6 +871,121 @@
> }
>
> namespace {
> + typedef std::pair<const Stmt *,
> + sema::FunctionScopeInfo::WeakObjectUseMap::const_iterator>
> + StmtUsesPair;
> +}
> +
> +template<>
> +class BeforeThanCompare<StmtUsesPair> {
> + const SourceManager &SM;
> +
> +public:
> + explicit BeforeThanCompare(const SourceManager &SM) : SM(SM) { }
> +
> + bool operator()(const StmtUsesPair &LHS, const StmtUsesPair &RHS) {
> + return SM.isBeforeInTranslationUnit(LHS.first->getLocStart(),
> + RHS.first->getLocStart());
> + }
> +};
> +
> +
> +static void diagnoseRepeatedUseOfWeak(Sema &S,
> + const sema::FunctionScopeInfo *CurFn,
> + const Decl *D) {
> + typedef sema::FunctionScopeInfo::WeakObjectProfileTy WeakObjectProfileTy;
> + typedef sema::FunctionScopeInfo::WeakObjectUseMap WeakObjectUseMap;
> + typedef sema::FunctionScopeInfo::WeakUseVector WeakUseVector;
> +
> + const WeakObjectUseMap &WeakMap = CurFn->getWeakObjectUses();
> +
> + // Extract all weak objects that are referenced more than once.
> + SmallVector<StmtUsesPair, 8> UsesByStmt;
> + for (WeakObjectUseMap::const_iterator I = WeakMap.begin(), E = WeakMap.end();
> + I != E; ++I) {
> + const WeakUseVector &Uses = I->second;
> + if (Uses.size() <= 1)
> + continue;
> +
> + // Find the first read of the weak object.
> + WeakUseVector::const_iterator UI = Uses.begin(), UE = Uses.end();
> + for ( ; UI != UE; ++UI) {
> + if (UI->isUnsafe())
> + break;
> + }
> +
> + // If there were only writes to this object, don't warn.
> + if (UI == UE)
> + continue;
> +
> + UsesByStmt.push_back(StmtUsesPair(UI->getUseExpr(), I));
> + }
> +
> + if (UsesByStmt.empty())
> + return;
> +
> + // Sort by first use so that we emit the warnings in a deterministic order.
> + std::sort(UsesByStmt.begin(), UsesByStmt.end(),
> + BeforeThanCompare<StmtUsesPair>(S.getSourceManager()));
> +
> + // Classify the current code body for better warning text.
> + // This enum should stay in sync with the cases in
> + // warn_arc_repeated_use_of_weak and warn_arc_possible_repeated_use_of_weak.
> + // FIXME: Should we use a common classification enum and the same set of
> + // possibilities all throughout Sema?
> + enum {
> + Function,
> + Method,
> + Block,
> + Lambda
> + } FunctionKind;
> +
> + if (isa<sema::BlockScopeInfo>(CurFn))
> + FunctionKind = Block;
> + else if (isa<sema::LambdaScopeInfo>(CurFn))
> + FunctionKind = Lambda;
> + else if (isa<ObjCMethodDecl>(D))
> + FunctionKind = Method;
> + else
> + FunctionKind = Function;
> +
> + // Iterate through the sorted problems and emit warnings for each.
> + for (SmallVectorImpl<StmtUsesPair>::const_iterator I = UsesByStmt.begin(),
> + E = UsesByStmt.end();
> + I != E; ++I) {
> + const Stmt *FirstRead = I->first;
> + const WeakObjectProfileTy &Key = I->second->first;
> + const WeakUseVector &Uses = I->second->second;
> +
> + // For complicated expressions like self.foo.bar, it's hard to keep track
> + // of whether 'self.foo' is the same between two cases. We can only be
> + // 100% sure of a repeated use if the "base" part of the key is a variable,
> + // rather than, say, another property.
> + unsigned DiagKind;
> + if (Key.isExactProfile())
> + DiagKind = diag::warn_arc_repeated_use_of_weak;
> + else
> + DiagKind = diag::warn_arc_possible_repeated_use_of_weak;
> +
> + // Show the first time the object was read.
> + S.Diag(FirstRead->getLocStart(), DiagKind)
> + << FunctionKind
> + << FirstRead->getSourceRange();
> +
> + // Print all the other accesses as notes.
> + for (WeakUseVector::const_iterator UI = Uses.begin(), UE = Uses.end();
> + UI != UE; ++UI) {
> + if (UI->getUseExpr() == FirstRead)
> + continue;
> + S.Diag(UI->getUseExpr()->getLocStart(),
> + diag::note_arc_weak_also_accessed_here)
> + << UI->getUseExpr()->getSourceRange();
> + }
> + }
> +}
> +
> +
> +namespace {
> struct SLocSort {
> bool operator()(const UninitUse &a, const UninitUse &b) {
> // Prefer a more confident report over a less confident one.
> @@ -1364,6 +1479,11 @@
> DiagnoseSwitchLabelsFallthrough(S, AC, !FallThroughDiagFull);
> }
>
> + if (S.getLangOpts().ObjCARCWeak &&
> + Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
> + D->getLocStart()) != DiagnosticsEngine::Ignored)
> + diagnoseRepeatedUseOfWeak(S, fscope, D);
> +
> // Collect statistics about the CFG if it was built.
> if (S.CollectStats && AC.isCFGBuilt()) {
> ++NumFunctionsAnalyzed;
>
> Modified: cfe/trunk/lib/Sema/Sema.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=164854&r1=164853&r2=164854&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/Sema.cpp (original)
> +++ cfe/trunk/lib/Sema/Sema.cpp Fri Sep 28 17:21:30 2012
> @@ -54,6 +54,132 @@
> Returns.clear();
> ErrorTrap.reset();
> PossiblyUnreachableDiags.clear();
> + WeakObjectUses.clear();
> +}
> +
> +static const NamedDecl *getBestPropertyDecl(const ObjCPropertyRefExpr *PropE) {
> + if (PropE->isExplicitProperty())
> + return PropE->getExplicitProperty();
> +
> + return PropE->getImplicitPropertyGetter();
> +}
> +
> +static bool isSelfExpr(const Expr *E) {
> + E = E->IgnoreParenImpCasts();
> +
> + const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E);
> + if (!DRE)
> + return false;
> +
> + const ImplicitParamDecl *Param = dyn_cast<ImplicitParamDecl>(DRE->getDecl());
> + if (!Param)
> + return false;
> +
> + const ObjCMethodDecl *M = dyn_cast<ObjCMethodDecl>(Param->getDeclContext());
> + if (!M)
> + return false;
> +
> + return M->getSelfDecl() == Param;
> +}
> +
> +FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy(
> + const ObjCPropertyRefExpr *PropE)
> + : Base(0, false), Property(getBestPropertyDecl(PropE)) {
> +
> + if (PropE->isObjectReceiver()) {
> + const OpaqueValueExpr *OVE = cast<OpaqueValueExpr>(PropE->getBase());
> + const Expr *E = OVE->getSourceExpr()->IgnoreParenCasts();
> +
> + switch (E->getStmtClass()) {
> + case Stmt::DeclRefExprClass:
> + Base.setPointer(cast<DeclRefExpr>(E)->getDecl());
> + Base.setInt(isa<VarDecl>(Base.getPointer()));
> + break;
> + case Stmt::MemberExprClass: {
> + const MemberExpr *ME = cast<MemberExpr>(E);
> + Base.setPointer(ME->getMemberDecl());
> + Base.setInt(isa<CXXThisExpr>(ME->getBase()->IgnoreParenImpCasts()));
> + break;
> + }
> + case Stmt::ObjCIvarRefExprClass: {
> + const ObjCIvarRefExpr *IE = cast<ObjCIvarRefExpr>(E);
> + Base.setPointer(IE->getDecl());
> + if (isSelfExpr(IE->getBase()))
> + Base.setInt(true);
> + break;
> + }
> + case Stmt::PseudoObjectExprClass: {
> + const PseudoObjectExpr *POE = cast<PseudoObjectExpr>(E);
> + const ObjCPropertyRefExpr *BaseProp =
> + dyn_cast<ObjCPropertyRefExpr>(POE->getSyntacticForm());
> + if (BaseProp) {
> + Base.setPointer(getBestPropertyDecl(BaseProp));
> +
> + const Expr *DoubleBase = BaseProp->getBase();
> + if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(DoubleBase))
> + DoubleBase = OVE->getSourceExpr();
> +
> + if (isSelfExpr(DoubleBase))
> + Base.setInt(true);
> + }
> + break;
> + }
> + default:
> + break;
> + }
> + } else if (PropE->isClassReceiver()) {
> + Base.setPointer(PropE->getClassReceiver());
> + Base.setInt(true);
> + } else {
> + assert(PropE->isSuperReceiver());
> + Base.setInt(true);
> + }
> +}
> +
> +void FunctionScopeInfo::recordUseOfWeak(const ObjCPropertyRefExpr *RefExpr) {
> + assert(RefExpr);
> + WeakUseVector &Uses =
> + WeakObjectUses[FunctionScopeInfo::WeakObjectProfileTy(RefExpr)];
> + Uses.push_back(WeakUseTy(RefExpr, RefExpr->isMessagingGetter()));
> +}
> +
> +void FunctionScopeInfo::markSafeWeakUse(const Expr *E) {
> + E = E->IgnoreParenImpCasts();
> +
> + if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(E)) {
> + markSafeWeakUse(POE->getSyntacticForm());
> + return;
> + }
> +
> + if (const ConditionalOperator *Cond = dyn_cast<ConditionalOperator>(E)) {
> + markSafeWeakUse(Cond->getTrueExpr());
> + markSafeWeakUse(Cond->getFalseExpr());
> + return;
> + }
> +
> + if (const BinaryConditionalOperator *Cond =
> + dyn_cast<BinaryConditionalOperator>(E)) {
> + markSafeWeakUse(Cond->getCommon());
> + markSafeWeakUse(Cond->getFalseExpr());
> + return;
> + }
> +
> + if (const ObjCPropertyRefExpr *RefExpr = dyn_cast<ObjCPropertyRefExpr>(E)) {
> + // Has this property been seen as a weak property?
> + FunctionScopeInfo::WeakObjectUseMap::iterator Uses =
> + WeakObjectUses.find(FunctionScopeInfo::WeakObjectProfileTy(RefExpr));
> + if (Uses == WeakObjectUses.end())
> + return;
> +
> + // Has there been a read from the property using this Expr?
> + FunctionScopeInfo::WeakUseVector::reverse_iterator ThisUse =
> + std::find(Uses->second.rbegin(), Uses->second.rend(), WeakUseTy(E, true));
> + if (ThisUse == Uses->second.rend())
> + return;
> +
> + ThisUse->markSafe();
> + return;
> + }
> }
>
> BlockScopeInfo::~BlockScopeInfo() { }
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=164854&r1=164853&r2=164854&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Sep 28 17:21:30 2012
> @@ -6588,6 +6588,21 @@
>
> if (VDecl->hasAttr<BlocksAttr>())
> checkRetainCycles(VDecl, Init);
> +
> + // It is safe to assign a weak reference into a strong variable.
> + // Although this code can still have problems:
> + // id x = self.weakProp;
> + // id y = self.weakProp;
> + // we do not warn to warn spuriously when 'x' and 'y' are on separate
> + // paths through the function. This should be revisited if
> + // -Wrepeated-use-of-weak is made flow-sensitive.
> + if (VDecl->getType().getObjCLifetime() == Qualifiers::OCL_Strong) {
> + DiagnosticsEngine::Level Level =
> + Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
> + Init->getLocStart());
> + if (Level != DiagnosticsEngine::Ignored)
> + getCurFunction()->markSafeWeakUse(Init);
> + }
> }
>
> Init = MaybeCreateExprWithCleanups(Init);
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=164854&r1=164853&r2=164854&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Sep 28 17:21:30 2012
> @@ -7726,6 +7726,19 @@
> if (!DRE || DRE->getDecl()->hasAttr<BlocksAttr>())
> checkRetainCycles(LHSExpr, RHS.get());
>
> + // It is safe to assign a weak reference into a strong variable.
> + // Although this code can still have problems:
> + // id x = self.weakProp;
> + // id y = self.weakProp;
> + // we do not warn to warn spuriously when 'x' and 'y' are on separate
> + // paths through the function. This should be revisited if
> + // -Wrepeated-use-of-weak is made flow-sensitive.
> + DiagnosticsEngine::Level Level =
> + Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
> + RHS.get()->getLocStart());
> + if (Level != DiagnosticsEngine::Ignored)
> + getCurFunction()->markSafeWeakUse(RHS.get());
> +
> } else if (getLangOpts().ObjCAutoRefCount) {
> checkUnsafeExprAssigns(Loc, LHSExpr, RHS.get());
> }
>
> Modified: cfe/trunk/lib/Sema/SemaPseudoObject.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaPseudoObject.cpp?rev=164854&r1=164853&r2=164854&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaPseudoObject.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaPseudoObject.cpp Fri Sep 28 17:21:30 2012
> @@ -31,6 +31,7 @@
> //===----------------------------------------------------------------------===//
>
> #include "clang/Sema/SemaInternal.h"
> +#include "clang/Sema/ScopeInfo.h"
> #include "clang/Sema/Initialization.h"
> #include "clang/AST/ExprObjC.h"
> #include "clang/Lex/Preprocessor.h"
> @@ -186,7 +187,7 @@
> UnaryOperatorKind opcode,
> Expr *op);
>
> - ExprResult complete(Expr *syntacticForm);
> + virtual ExprResult complete(Expr *syntacticForm);
>
> OpaqueValueExpr *capture(Expr *op);
> OpaqueValueExpr *captureValueAsResult(Expr *op);
> @@ -238,6 +239,9 @@
> Expr *rebuildAndCaptureObject(Expr *syntacticBase);
> ExprResult buildGet();
> ExprResult buildSet(Expr *op, SourceLocation, bool);
> + ExprResult complete(Expr *SyntacticForm);
> +
> + bool isWeakProperty() const;
> };
>
> /// A PseudoOpBuilder for Objective-C array/dictionary indexing.
> @@ -471,6 +475,23 @@
> return S.LookupMethodInObjectType(sel, IT, false);
> }
>
> +bool ObjCPropertyOpBuilder::isWeakProperty() const {
> + QualType T;
> + if (RefExpr->isExplicitProperty()) {
> + const ObjCPropertyDecl *Prop = RefExpr->getExplicitProperty();
> + if (Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak)
> + return true;
> +
> + T = Prop->getType();
> + } else if (Getter) {
> + T = Getter->getResultType();
> + } else {
> + return false;
> + }
> +
> + return T.getObjCLifetime() == Qualifiers::OCL_Weak;
> +}
> +
> bool ObjCPropertyOpBuilder::findGetter() {
> if (Getter) return true;
>
> @@ -818,6 +839,18 @@
> return PseudoOpBuilder::buildIncDecOperation(Sc, opcLoc, opcode, op);
> }
>
> +ExprResult ObjCPropertyOpBuilder::complete(Expr *SyntacticForm) {
> + if (S.getLangOpts().ObjCAutoRefCount && isWeakProperty()) {
> + DiagnosticsEngine::Level Level =
> + S.Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
> + SyntacticForm->getLocStart());
> + if (Level != DiagnosticsEngine::Ignored)
> + S.getCurFunction()->recordUseOfWeak(SyntacticRefExpr);
> + }
> +
> + return PseudoOpBuilder::complete(SyntacticForm);
> +}
> +
> // ObjCSubscript build stuff.
> //
>
>
> Added: cfe/trunk/test/SemaObjC/arc-repeated-weak.mm
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-repeated-weak.mm?rev=164854&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/arc-repeated-weak.mm (added)
> +++ cfe/trunk/test/SemaObjC/arc-repeated-weak.mm Fri Sep 28 17:21:30 2012
> @@ -0,0 +1,203 @@
> +// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -Warc-repeated-use-of-weak -verify %s
> +
> + at interface Test {
> + at public
> + Test *ivar;
> +}
> + at property(weak) Test *weakProp;
> + at property(strong) Test *strongProp;
> +
> +- (__weak id)implicitProp;
> +
> ++ (__weak id)weakProp;
> + at end
> +
> +extern void use(id);
> +extern id get();
> +extern bool condition();
> +#define nil ((id)0)
> +
> +void sanity(Test *a) {
> + use(a.weakProp); // expected-warning{{weak property is accessed multiple times in this function but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
> + use(a.weakProp); // expected-note{{also accessed here}}
> +
> + use(a.strongProp);
> + use(a.strongProp); // no-warning
> +
> + use(a.weakProp); // expected-note{{also accessed here}}
> +}
> +
> +void singleUse(Test *a) {
> + use(a.weakProp); // no-warning
> + use(a.strongProp); // no-warning
> +}
> +
> +void assignsOnly(Test *a) {
> + a.weakProp = get(); // no-warning
> +
> + id next = get();
> + if (next)
> + a.weakProp = next; // no-warning
> +}
> +
> +void assignThenRead(Test *a) {
> + a.weakProp = get(); // expected-note{{also accessed here}}
> + use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
> +}
> +
> +void twoVariables(Test *a, Test *b) {
> + use(a.weakProp); // no-warning
> + use(b.weakProp); // no-warning
> +}
> +
> +void doubleLevelAccess(Test *a) {
> + use(a.strongProp.weakProp); // expected-warning{{weak property may be accessed multiple times in this function but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
> + use(a.strongProp.weakProp); // expected-note{{also accessed here}}
> +}
> +
> +void doubleLevelAccessIvar(Test *a) {
> + use(a.strongProp.weakProp); // expected-warning{{weak property may be accessed multiple times}}
> + use(a.strongProp.weakProp); // expected-note{{also accessed here}}
> +}
> +
> +void implicitProperties(Test *a) {
> + use(a.implicitProp); // expected-warning{{weak property is accessed multiple times}}
> + use(a.implicitProp); // expected-note{{also accessed here}}
> +}
> +
> +void classProperties() {
> + use(Test.weakProp); // expected-warning{{weak property is accessed multiple times}}
> + use(Test.weakProp); // expected-note{{also accessed here}}
> +}
> +
> +void classPropertiesAreDifferent(Test *a) {
> + use(Test.weakProp); // no-warning
> + use(a.weakProp); // no-warning
> + use(a.strongProp.weakProp); // no-warning
> +}
> +
> +
> +void assignToStrongWrongInit(Test *a) {
> + id val = a.weakProp; // expected-note{{also accessed here}}
> + use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
> +}
> +
> +void assignToStrongWrong(Test *a) {
> + id val;
> + val = a.weakProp; // expected-note{{also accessed here}}
> + use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
> +}
> +
> +void assignToStrongOK(Test *a) {
> + if (condition()) {
> + id val = a.weakProp; // no-warning
> + (void)val;
> + } else {
> + id val;
> + val = a.weakProp; // no-warning
> + (void)val;
> + }
> +}
> +
> +void assignToStrongConditional(Test *a) {
> + id val = (condition() ? a.weakProp : a.weakProp); // no-warning
> + id val2 = a.implicitProp ?: a.implicitProp; // no-warning
> +}
> +
> +void testBlock(Test *a) {
> + use(a.weakProp); // no-warning
> +
> + use(^{
> + use(a.weakProp); // expected-warning{{weak property is accessed multiple times in this block}}
> + use(a.weakProp); // expected-note{{also accessed here}}
> + });
> +}
> +
> +
> + at interface Test (Methods)
> + at end
> +
> + at implementation Test (Methods)
> +- (void)sanity {
> + use(self.weakProp); // expected-warning{{weak property is accessed multiple times in this method but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
> + use(self.weakProp); // expected-note{{also accessed here}}
> +}
> +
> +- (void)doubleLevelAccessForSelf {
> + use(self.strongProp.weakProp); // expected-warning{{weak property is accessed multiple times}}
> + use(self.strongProp.weakProp); // expected-note{{also accessed here}}
> +
> + use(self->ivar.weakProp); // expected-warning{{weak property is accessed multiple times}}
> + use(self->ivar.weakProp); // expected-note{{also accessed here}}
> +}
> +
> +- (void)distinctFromOther:(Test *)other {
> + use(self.strongProp.weakProp); // no-warning
> + use(other.strongProp.weakProp); // no-warning
> +
> + use(self->ivar.weakProp); // no-warning
> + use(other->ivar.weakProp); // no-warning
> +}
> + at end
> +
> +
> +class Wrapper {
> + Test *a;
> +
> +public:
> + void fields() {
> + use(a.weakProp); // expected-warning{{weak property is accessed multiple times in this function but may be unpredictably set to nil; assign to a strong variable to keep the object alive}}
> + use(a.weakProp); // expected-note{{also accessed here}}
> + }
> +
> + void distinctFromOther(Test *b, const Wrapper &w) {
> + use(a.weakProp); // no-warning
> + use(b.weakProp); // no-warning
> + use(w.a.weakProp); // no-warning
> + }
> +
> + static void doubleLevelAccessField(const Wrapper &x, const Wrapper &y) {
> + use(x.a.weakProp); // expected-warning{{weak property may be accessed multiple times}}
> + use(y.a.weakProp); // expected-note{{also accessed here}}
> + }
> +};
> +
> +
> +// -----------------------
> +// False positives
> +// -----------------------
> +
> +// Most of these would require flow-sensitive analysis to silence correctly.
> +
> +void assignAfterRead(Test *a) {
> + if (!a.weakProp) // expected-warning{{weak property is accessed multiple times}}
> + a.weakProp = get(); // expected-note{{also accessed here}}
> +}
> +
> +void assignNil(Test *a) {
> + if (condition())
> + a.weakProp = nil; // expected-note{{also accessed here}}
> +
> + use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
> +}
> +
> +void branch(Test *a) {
> + if (condition())
> + use(a.weakProp); // expected-warning{{weak property is accessed multiple times}}
> + else
> + use(a.weakProp); // expected-note{{also accessed here}}
> +}
> +
> +void doubleLevelAccess(Test *a, Test *b) {
> + use(a.strongProp.weakProp); // expected-warning{{weak property may be accessed multiple times}}
> + use(b.strongProp.weakProp); // expected-note{{also accessed here}}
> +
> + use(a.weakProp.weakProp); // no-warning
> +}
> +
> +void doubleLevelAccessIvar(Test *a, Test *b) {
> + use(a->ivar.weakProp); // expected-warning{{weak property may be accessed multiple times}}
> + use(b->ivar.weakProp); // expected-note{{also accessed here}}
> +
> + use(a.strongProp.weakProp); // no-warning
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list