5 files changed, 308 insertions, 1 deletions

diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index a3aee9afe08..86e9dc2d648 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -871,6 +871,121 @@ static void DiagnoseSwitchLabelsFallthrough(Sema &S, AnalysisDeclContext &AC,
 }
 
 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 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
     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;
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 08ccfa42598..84cdb2b6f04 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -54,6 +54,132 @@ void FunctionScopeInfo::Clear() {
   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() { }
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 01aaf8be323..592956b9dbc 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6588,6 +6588,21 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init,
 
     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);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 630281a4242..597e4d6f77c 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -7726,6 +7726,19 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
         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());
       }
diff --git a/clang/lib/Sema/SemaPseudoObject.cpp b/clang/lib/Sema/SemaPseudoObject.cpp
index 31496b32a07..b07a59b81c2 100644
--- a/clang/lib/Sema/SemaPseudoObject.cpp
+++ b/clang/lib/Sema/SemaPseudoObject.cpp
@@ -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 @@ namespace {
                                     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 @@ namespace {
     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 @@ static ObjCMethodDecl *LookupMethodInReceiverType(Sema &S, Selector sel,
   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 @@ ObjCPropertyOpBuilder::buildIncDecOperation(Scope *Sc, SourceLocation opcLoc,
   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.
 //