[cfe-commits] r165719 - in /cfe/trunk: lib/Sema/AnalysisBasedWarnings.cpp test/SemaObjC/arc-repeated-weak.mm
Jordan Rose
jordan_rose at apple.com
Thu Oct 11 09:10:19 PDT 2012
Author: jrose
Date: Thu Oct 11 11:10:19 2012
New Revision: 165719
URL: http://llvm.org/viewvc/llvm-project?rev=165719&view=rev
Log:
-Warc-repeated-use-of-weak: Don't warn on a single read followed by writes.
This is a "safe" pattern, or at least one that cannot be helped by using
a strong local variable. However, if the single read is within a loop,
it should /always/ be treated as potentially dangerous.
<rdar://problem/12437490>
Modified:
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/test/SemaObjC/arc-repeated-weak.mm
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=165719&r1=165718&r2=165719&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Thu Oct 11 11:10:19 2012
@@ -27,6 +27,7 @@
#include "clang/AST/StmtObjC.h"
#include "clang/AST/StmtCXX.h"
#include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/AST/ParentMap.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Analysis/AnalysisContext.h"
@@ -903,10 +904,30 @@
};
}
+static bool isInLoop(const ParentMap &PM, const Stmt *S) {
+ assert(S);
+
+ do {
+ switch (S->getStmtClass()) {
+ case Stmt::DoStmtClass:
+ case Stmt::ForStmtClass:
+ case Stmt::WhileStmtClass:
+ case Stmt::CXXForRangeStmtClass:
+ case Stmt::ObjCForCollectionStmtClass:
+ return true;
+ default:
+ break;
+ }
+ } while ((S = PM.getParent(S)));
+
+ return false;
+}
+
static void diagnoseRepeatedUseOfWeak(Sema &S,
const sema::FunctionScopeInfo *CurFn,
- const Decl *D) {
+ const Decl *D,
+ const ParentMap &PM) {
typedef sema::FunctionScopeInfo::WeakObjectProfileTy WeakObjectProfileTy;
typedef sema::FunctionScopeInfo::WeakObjectUseMap WeakObjectUseMap;
typedef sema::FunctionScopeInfo::WeakUseVector WeakUseVector;
@@ -918,8 +939,6 @@
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();
@@ -932,6 +951,19 @@
if (UI == UE)
continue;
+ // If there was only one read, followed by any number of writes, and the
+ // read is not within a loop, don't warn.
+ if (UI == Uses.begin()) {
+ WeakUseVector::const_iterator UI2 = UI;
+ for (++UI2; UI2 != UE; ++UI2)
+ if (UI2->isUnsafe())
+ break;
+
+ if (UI2 == UE)
+ if (!isInLoop(PM, UI->getUseExpr()))
+ continue;
+ }
+
UsesByStmt.push_back(StmtUsesPair(UI->getUseExpr(), I));
}
@@ -1519,7 +1551,7 @@
if (S.getLangOpts().ObjCARCWeak &&
Diags.getDiagnosticLevel(diag::warn_arc_repeated_use_of_weak,
D->getLocStart()) != DiagnosticsEngine::Ignored)
- diagnoseRepeatedUseOfWeak(S, fscope, D);
+ diagnoseRepeatedUseOfWeak(S, fscope, D, AC.getParentMap());
// Collect statistics about the CFG if it was built.
if (S.CollectStats && AC.isCFGBuilt()) {
Modified: cfe/trunk/test/SemaObjC/arc-repeated-weak.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-repeated-weak.mm?rev=165719&r1=165718&r2=165719&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/arc-repeated-weak.mm (original)
+++ cfe/trunk/test/SemaObjC/arc-repeated-weak.mm Thu Oct 11 11:10:19 2012
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -Warc-repeated-use-of-weak -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -std=c++11 -Warc-repeated-use-of-weak -verify %s
@interface Test {
@public
@@ -181,6 +181,70 @@
}
}
+void assignAfterRead(Test *a) {
+ // Special exception for a single read before any writes.
+ if (!a.weakProp) // no-warning
+ a.weakProp = get(); // no-warning
+}
+
+void readOnceWriteMany(Test *a) {
+ if (!a.weakProp) { // no-warning
+ a.weakProp = get(); // no-warning
+ a.weakProp = get(); // no-warning
+ }
+}
+
+void readOnceAfterWrite(Test *a) {
+ a.weakProp = get(); // expected-note{{also accessed here}}
+ if (!a.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}}
+ a.weakProp = get(); // expected-note{{also accessed here}}
+ }
+}
+
+void readOnceWriteManyLoops(Test *a, Test *b, Test *c, Test *d, Test *e) {
+ while (condition()) {
+ if (!a.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}}
+ a.weakProp = get(); // expected-note{{also accessed here}}
+ a.weakProp = get(); // expected-note{{also accessed here}}
+ }
+ }
+
+ do {
+ if (!b.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}}
+ b.weakProp = get(); // expected-note{{also accessed here}}
+ b.weakProp = get(); // expected-note{{also accessed here}}
+ }
+ } while (condition());
+
+ for (id x = get(); x; x = get()) {
+ if (!c.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}}
+ c.weakProp = get(); // expected-note{{also accessed here}}
+ c.weakProp = get(); // expected-note{{also accessed here}}
+ }
+ }
+
+ for (id x in get()) {
+ if (!d.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}}
+ d.weakProp = get(); // expected-note{{also accessed here}}
+ d.weakProp = get(); // expected-note{{also accessed here}}
+ }
+ }
+
+ int array[] = { 1, 2, 3 };
+ for (int i : array) {
+ if (!e.weakProp) { // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}}
+ e.weakProp = get(); // expected-note{{also accessed here}}
+ e.weakProp = get(); // expected-note{{also accessed here}}
+ }
+ }
+}
+
+void readOnlyLoop(Test *a) {
+ while (condition()) {
+ use(a.weakProp); // expected-warning{{weak property 'weakProp' is accessed multiple times in this function}}
+ }
+}
+
@interface Test (Methods)
@end
@@ -248,11 +312,6 @@
// Most of these would require flow-sensitive analysis to silence correctly.
-void assignAfterRead(Test *a) {
- if (!a.weakProp) // expected-warning{{weak property 'weakProp' 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}}
More information about the cfe-commits
mailing list