r198432 - [objc] Refactor and improve functionality for the -Wunused-property-ivar warning.
Argyrios Kyrtzidis
akyrtzi at gmail.com
Fri Jan 3 10:32:18 PST 2014
Author: akirtzidis
Date: Fri Jan 3 12:32:18 2014
New Revision: 198432
URL: http://llvm.org/viewvc/llvm-project?rev=198432&view=rev
Log:
[objc] Refactor and improve functionality for the -Wunused-property-ivar warning.
- Remove the additions to ObjCMethodDecl & ObjCIVarDecl that were getting de/serialized and consolidate
all functionality for the checking for this warning in Sema::DiagnoseUnusedBackingIvarInAccessor
- Don't check immediately after the method body is finished, check when the @implementation is finished.
This is so we can see if the ivar was referenced by any other method, even if the method was defined after the accessor.
- Don't silence the warning if any method is called from the accessor silence it if the accessor delegates to another method via self.
rdar://15727325
Modified:
cfe/trunk/include/clang/AST/DeclObjC.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/DeclObjC.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/test/SemaObjC/unused-backing-ivar-warning.m
Modified: cfe/trunk/include/clang/AST/DeclObjC.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclObjC.h (original)
+++ cfe/trunk/include/clang/AST/DeclObjC.h Fri Jan 3 12:32:18 2014
@@ -161,9 +161,6 @@ private:
/// \brief Indicates if the method was a definition but its body was skipped.
unsigned HasSkippedBody : 1;
-
- /// \brief True if method body makes a method call.
- unsigned MethodCallsMethod : 1;
// Result type of this method.
QualType MethodDeclType;
@@ -245,7 +242,7 @@ private:
DeclImplementation(impControl), objcDeclQualifier(OBJC_TQ_None),
RelatedResultType(HasRelatedResultType),
SelLocsKind(SelLoc_StandardNoSpace), IsOverriding(0), HasSkippedBody(0),
- MethodCallsMethod(0), MethodDeclType(T), ResultTInfo(ResultTInfo),
+ MethodDeclType(T), ResultTInfo(ResultTInfo),
ParamsAndSelLocs(0), NumParams(0),
DeclEndLoc(endLoc), Body(), SelfDecl(0), CmdDecl(0) {
setImplicit(isImplicitlyDeclared);
@@ -439,9 +436,6 @@ public:
bool hasSkippedBody() const { return HasSkippedBody; }
void setHasSkippedBody(bool Skipped = true) { HasSkippedBody = Skipped; }
- bool getMethodCallsMethod() const { return MethodCallsMethod; }
- void setMethodCallsMethod(bool val = true) { MethodCallsMethod = val; }
-
/// \brief Returns the property associated with this method's selector.
///
/// Note that even if this particular method is not marked as a property
@@ -1326,12 +1320,10 @@ private:
ObjCIvarDecl(ObjCContainerDecl *DC, SourceLocation StartLoc,
SourceLocation IdLoc, IdentifierInfo *Id,
QualType T, TypeSourceInfo *TInfo, AccessControl ac, Expr *BW,
- bool synthesized,
- bool backingIvarReferencedInAccessor)
+ bool synthesized)
: FieldDecl(ObjCIvar, DC, StartLoc, IdLoc, Id, T, TInfo, BW,
/*Mutable=*/false, /*HasInit=*/ICIS_NoInit),
- NextIvar(0), DeclAccess(ac), Synthesized(synthesized),
- BackingIvarReferencedInAccessor(backingIvarReferencedInAccessor) {}
+ NextIvar(0), DeclAccess(ac), Synthesized(synthesized) {}
public:
static ObjCIvarDecl *Create(ASTContext &C, ObjCContainerDecl *DC,
@@ -1339,8 +1331,7 @@ public:
IdentifierInfo *Id, QualType T,
TypeSourceInfo *TInfo,
AccessControl ac, Expr *BW = NULL,
- bool synthesized=false,
- bool backingIvarReferencedInAccessor=false);
+ bool synthesized=false);
static ObjCIvarDecl *CreateDeserialized(ASTContext &C, unsigned ID);
@@ -1362,13 +1353,6 @@ public:
return DeclAccess == None ? Protected : AccessControl(DeclAccess);
}
- void setBackingIvarReferencedInAccessor(bool val) {
- BackingIvarReferencedInAccessor = val;
- }
- bool getBackingIvarReferencedInAccessor() const {
- return BackingIvarReferencedInAccessor;
- }
-
void setSynthesize(bool synth) { Synthesized = synth; }
bool getSynthesize() const { return Synthesized; }
@@ -1383,7 +1367,6 @@ private:
// NOTE: VC++ treats enums as signed, avoid using the AccessControl enum
unsigned DeclAccess : 3;
unsigned Synthesized : 1;
- unsigned BackingIvarReferencedInAccessor : 1;
};
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Jan 3 12:32:18 2014
@@ -833,6 +833,7 @@ public:
/// Private Helper predicate to check for 'self'.
bool isSelfExpr(Expr *RExpr);
+ bool isSelfExpr(Expr *RExpr, const ObjCMethodDecl *Method);
/// \brief Cause the active diagnostic on the DiagosticsEngine to be
/// emitted. This is closely coupled to the SemaDiagnosticBuilder class and
@@ -2639,7 +2640,8 @@ public:
/// DiagnoseUnusedBackingIvarInAccessor - Issue an 'unused' warning if ivar which
/// backs the property is not used in the property's accessor.
- void DiagnoseUnusedBackingIvarInAccessor(Scope *S);
+ void DiagnoseUnusedBackingIvarInAccessor(Scope *S,
+ const ObjCImplementationDecl *ImplD);
/// GetIvarBackingPropertyAccessor - If method is a property setter/getter and
/// it property has a backing ivar, returns this ivar; otherwise, returns NULL.
Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Fri Jan 3 12:32:18 2014
@@ -1241,8 +1241,6 @@ void ASTDumper::VisitObjCIvarDecl(const
dumpType(D->getType());
if (D->getSynthesize())
OS << " synthesize";
- if (D->getBackingIvarReferencedInAccessor())
- OS << " BackingIvarReferencedInAccessor";
switch (D->getAccessControl()) {
case ObjCIvarDecl::None:
Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Jan 3 12:32:18 2014
@@ -3016,8 +3016,7 @@ Decl *ASTNodeImporter::VisitObjCIvarDecl
Importer.Import(D->getInnerLocStart()),
Loc, Name.getAsIdentifierInfo(),
T, TInfo, D->getAccessControl(),
- BitWidth, D->getSynthesize(),
- D->getBackingIvarReferencedInAccessor());
+ BitWidth, D->getSynthesize());
ToIvar->setLexicalDeclContext(LexicalDC);
Importer.Imported(D, ToIvar);
LexicalDC->addDeclInternal(ToIvar);
Modified: cfe/trunk/lib/AST/DeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclObjC.cpp (original)
+++ cfe/trunk/lib/AST/DeclObjC.cpp Fri Jan 3 12:32:18 2014
@@ -1440,8 +1440,7 @@ ObjCIvarDecl *ObjCIvarDecl::Create(ASTCo
SourceLocation IdLoc, IdentifierInfo *Id,
QualType T, TypeSourceInfo *TInfo,
AccessControl ac, Expr *BW,
- bool synthesized,
- bool backingIvarReferencedInAccessor) {
+ bool synthesized) {
if (DC) {
// Ivar's can only appear in interfaces, implementations (via synthesized
// properties), and class extensions (via direct declaration, or synthesized
@@ -1469,13 +1468,12 @@ ObjCIvarDecl *ObjCIvarDecl::Create(ASTCo
}
return new (C, DC) ObjCIvarDecl(DC, StartLoc, IdLoc, Id, T, TInfo, ac, BW,
- synthesized, backingIvarReferencedInAccessor);
+ synthesized);
}
ObjCIvarDecl *ObjCIvarDecl::CreateDeserialized(ASTContext &C, unsigned ID) {
return new (C, ID) ObjCIvarDecl(0, SourceLocation(), SourceLocation(), 0,
- QualType(), 0, ObjCIvarDecl::None, 0, false,
- false);
+ QualType(), 0, ObjCIvarDecl::None, 0, false);
}
const ObjCInterfaceDecl *ObjCIvarDecl::getContainingInterface() const {
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jan 3 12:32:18 2014
@@ -1392,7 +1392,6 @@ void Sema::ActOnPopScope(SourceLocation
// Remove this name from our lexical scope.
IdResolver.RemoveDecl(D);
}
- DiagnoseUnusedBackingIvarInAccessor(S);
}
void Sema::ActOnStartFunctionDeclarator() {
Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Fri Jan 3 12:32:18 2014
@@ -15,6 +15,7 @@
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTMutationListener.h"
+#include "clang/AST/DataRecursiveASTVisitor.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprObjC.h"
@@ -2693,6 +2694,7 @@ Decl *Sema::ActOnAtEnd(Scope *S, SourceR
ImplMethodsVsClassMethods(S, IC, IDecl);
AtomicPropertySetterGetterRules(IC, IDecl);
DiagnoseOwningPropertyGetterSynthesis(IC);
+ DiagnoseUnusedBackingIvarInAccessor(S, IC);
if (IDecl->hasDesignatedInitializers())
DiagnoseMissingDesignatedInitOverrides(IC, IDecl);
@@ -3494,39 +3496,83 @@ Sema::GetIvarBackingPropertyAccessor(con
const ObjCInterfaceDecl *IDecl = Method->getClassInterface();
if (!IDecl)
return 0;
- Method = IDecl->lookupMethod(Method->getSelector(), true);
+ Method = IDecl->lookupMethod(Method->getSelector(), /*isInstance=*/true,
+ /*shallowCategoryLookup=*/false,
+ /*followSuper=*/false);
if (!Method || !Method->isPropertyAccessor())
return 0;
- if ((PDecl = Method->findPropertyDecl())) {
- if (!PDecl->getDeclContext())
- return 0;
- // Make sure property belongs to accessor's class and not to
- // one of its super classes.
- if (const ObjCInterfaceDecl *CID =
- dyn_cast<ObjCInterfaceDecl>(PDecl->getDeclContext()))
- if (CID != IDecl)
- return 0;
+ if ((PDecl = Method->findPropertyDecl()))
return PDecl->getPropertyIvarDecl();
- }
+
return 0;
}
-void Sema::DiagnoseUnusedBackingIvarInAccessor(Scope *S) {
- if (S->hasUnrecoverableErrorOccurred() || !S->isInObjcMethodOuterScope())
- return;
-
- const ObjCMethodDecl *CurMethod = getCurMethodDecl();
- if (!CurMethod)
+namespace {
+ /// Used by Sema::DiagnoseUnusedBackingIvarInAccessor to check if a property
+ /// accessor references the backing ivar.
+ class UnusedBackingIvarChecker : public DataRecursiveASTVisitor<UnusedBackingIvarChecker> {
+ public:
+ Sema &S;
+ const ObjCMethodDecl *Method;
+ const ObjCIvarDecl *IvarD;
+ bool AccessedIvar;
+ bool InvokedSelfMethod;
+
+ UnusedBackingIvarChecker(Sema &S, const ObjCMethodDecl *Method,
+ const ObjCIvarDecl *IvarD)
+ : S(S), Method(Method), IvarD(IvarD),
+ AccessedIvar(false), InvokedSelfMethod(false) {
+ assert(IvarD);
+ }
+
+ bool VisitObjCIvarRefExpr(ObjCIvarRefExpr *E) {
+ if (E->getDecl() == IvarD) {
+ AccessedIvar = true;
+ return false;
+ }
+ return true;
+ }
+
+ bool VisitObjCMessageExpr(ObjCMessageExpr *E) {
+ if (E->getReceiverKind() == ObjCMessageExpr::Instance &&
+ S.isSelfExpr(E->getInstanceReceiver(), Method)) {
+ InvokedSelfMethod = true;
+ }
+ return true;
+ }
+ };
+}
+
+void Sema::DiagnoseUnusedBackingIvarInAccessor(Scope *S,
+ const ObjCImplementationDecl *ImplD) {
+ if (S->hasUnrecoverableErrorOccurred())
return;
- const ObjCPropertyDecl *PDecl;
- const ObjCIvarDecl *IV = GetIvarBackingPropertyAccessor(CurMethod, PDecl);
- if (IV && !IV->getBackingIvarReferencedInAccessor()) {
+
+ for (ObjCImplementationDecl::instmeth_iterator
+ MI = ImplD->instmeth_begin(),
+ ME = ImplD->instmeth_end(); MI != ME; ++MI) {
+ const ObjCMethodDecl *CurMethod = *MI;
+ unsigned DIAG = diag::warn_unused_property_backing_ivar;
+ SourceLocation Loc = CurMethod->getLocation();
+ if (Diags.getDiagnosticLevel(DIAG, Loc) == DiagnosticsEngine::Ignored)
+ continue;
+
+ const ObjCPropertyDecl *PDecl;
+ const ObjCIvarDecl *IV = GetIvarBackingPropertyAccessor(CurMethod, PDecl);
+ if (!IV)
+ continue;
+
+ UnusedBackingIvarChecker Checker(*this, CurMethod, IV);
+ Checker.TraverseStmt(CurMethod->getBody());
+ if (Checker.AccessedIvar)
+ continue;
+
// Do not issue this warning if backing ivar is used somewhere and accessor
- // implementation makes a call to a method. This is to prevent false positive in
- // some corner cases.
- if (!IV->isReferenced() || !CurMethod->getMethodCallsMethod()) {
- Diag(getCurMethodDecl()->getLocation(), diag::warn_unused_property_backing_ivar)
- << IV->getDeclName();
+ // implementation makes a self call. This is to prevent false positive in
+ // cases where the ivar is accessed by another method that the accessor
+ // delegates to.
+ if (!IV->isReferenced() || !Checker.InvokedSelfMethod) {
+ Diag(Loc, DIAG) << IV->getDeclName();
Diag(PDecl->getLocation(), diag::note_property_declare);
}
}
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Jan 3 12:32:18 2014
@@ -2248,15 +2248,7 @@ Sema::LookupInObjCMethod(LookupResult &L
return ExprError();
MarkAnyDeclReferenced(Loc, IV, true);
- if (!IV->getBackingIvarReferencedInAccessor()) {
- // Mark this ivar 'referenced' in this method, if it is a backing ivar
- // of a property and current method is one of its property accessor.
- const ObjCPropertyDecl *PDecl;
- const ObjCIvarDecl *BIV = GetIvarBackingPropertyAccessor(CurMethod, PDecl);
- if (BIV && BIV == IV)
- IV->setBackingIvarReferencedInAccessor(true);
- }
-
+
ObjCMethodFamily MF = CurMethod->getMethodFamily();
if (MF != OMF_init && MF != OMF_dealloc && MF != OMF_finalize &&
!IvarBacksCurrentMethodAccessor(IFace, CurMethod, IV))
Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Fri Jan 3 12:32:18 2014
@@ -1380,10 +1380,14 @@ bool Sema::CheckMessageArgumentTypes(Qua
return IsError;
}
-bool Sema::isSelfExpr(Expr *receiver) {
+bool Sema::isSelfExpr(Expr *RExpr) {
// 'self' is objc 'self' in an objc method only.
- ObjCMethodDecl *method =
- dyn_cast_or_null<ObjCMethodDecl>(CurContext->getNonClosureAncestor());
+ ObjCMethodDecl *Method =
+ dyn_cast_or_null<ObjCMethodDecl>(CurContext->getNonClosureAncestor());
+ return isSelfExpr(RExpr, Method);
+}
+
+bool Sema::isSelfExpr(Expr *receiver, const ObjCMethodDecl *method) {
if (!method) return false;
receiver = receiver->IgnoreParenLValueCasts();
@@ -2643,8 +2647,6 @@ ExprResult Sema::BuildInstanceMessage(Ex
}
}
}
- if (ObjCMethodDecl *CurMeth = getCurMethodDecl())
- CurMeth->setMethodCallsMethod(true);
return MaybeBindToTemporary(Result);
}
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Jan 3 12:32:18 2014
@@ -702,7 +702,6 @@ void ASTDeclReader::VisitObjCMethodDecl(
MD->setDefined(Record[Idx++]);
MD->IsOverriding = Record[Idx++];
MD->HasSkippedBody = Record[Idx++];
- MD->MethodCallsMethod = Record[Idx++];
MD->IsRedeclaration = Record[Idx++];
MD->HasRedeclaration = Record[Idx++];
@@ -803,8 +802,6 @@ void ASTDeclReader::VisitObjCIvarDecl(Ob
IVD->setNextIvar(0);
bool synth = Record[Idx++];
IVD->setSynthesize(synth);
- bool backingIvarReferencedInAccessor = Record[Idx++];
- IVD->setBackingIvarReferencedInAccessor(backingIvarReferencedInAccessor);
}
void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) {
Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Fri Jan 3 12:32:18 2014
@@ -438,7 +438,6 @@ void ASTDeclWriter::VisitObjCMethodDecl(
Record.push_back(D->isDefined());
Record.push_back(D->IsOverriding);
Record.push_back(D->HasSkippedBody);
- Record.push_back(D->MethodCallsMethod);
Record.push_back(D->IsRedeclaration);
Record.push_back(D->HasRedeclaration);
@@ -530,7 +529,6 @@ void ASTDeclWriter::VisitObjCIvarDecl(Ob
// FIXME: stable encoding for @public/@private/@protected/@package
Record.push_back(D->getAccessControl());
Record.push_back(D->getSynthesize());
- Record.push_back(D->getBackingIvarReferencedInAccessor());
if (!D->hasAttrs() &&
!D->isImplicit() &&
Modified: cfe/trunk/test/SemaObjC/unused-backing-ivar-warning.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/unused-backing-ivar-warning.m?rev=198432&r1=198431&r2=198432&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/unused-backing-ivar-warning.m (original)
+++ cfe/trunk/test/SemaObjC/unused-backing-ivar-warning.m Fri Jan 3 12:32:18 2014
@@ -124,3 +124,26 @@ typedef char BOOL;
- (long) r { [self Meth]; return p; } // expected-warning {{ivar 'r' which backs the property is not referenced in this property's accessor}}
@end
+ at interface I1
+ at property (readonly) int p1;
+ at property (readonly) int p2; // expected-note {{property declared here}}
+ at end
+
+ at implementation I1
+ at synthesize p1=_p1;
+ at synthesize p2=_p2;
+
+-(int)p1 {
+ return [self getP1];
+}
+-(int)getP1 {
+ return _p1;
+}
+-(int)getP2 {
+ return _p2;
+}
+-(int)p2 { // expected-warning {{ivar '_p2' which backs the property is not referenced in this property's accessor}}
+ Radar15727327 *o;
+ return [o Meth];
+}
+ at end
More information about the cfe-commits
mailing list