From 2ba5ca9d4fd50309ea067e435c3d84a6718f69e3 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Sun, 16 Aug 2009 16:57:27 +0000 Subject: [PATCH] Improve the diagnostic emitted when an unused ObjC property getter is found. Instead of complaining about a generic "unused expr", emit: t.m:7:3: warning: property access result unused - getters should not have side effects While objc property getters *could* have side effects, according to the language best practices, they *shouldn't*. Hopefully the diagnostic now gets this across. llvm-svn: 79192 --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 3 +++ clang/lib/Sema/SemaStmt.cpp | 12 ++++++++++-- clang/test/SemaObjC/unused.m | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 2ebcfd36f5b9..b5f035267748 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -1687,6 +1687,9 @@ def ext_typecheck_expression_not_constant_but_accepted : Extension< "expression is not a constant, but is accepted as one by GNU extensions">; def warn_unused_expr : Warning<"expression result unused">, InGroup; +def warn_unused_property_expr : Warning< + "property access result unused - getters should not have side effects">, + InGroup; def err_incomplete_type_used_in_type_trait_expr : Error< "incomplete type %0 used in type trait expression">; diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index f66ee1e30f2e..579433849e6c 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -15,7 +15,7 @@ #include "clang/AST/APValue.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclObjC.h" -#include "clang/AST/Expr.h" +#include "clang/AST/ExprObjC.h" #include "clang/AST/StmtObjC.h" #include "clang/AST/StmtCXX.h" #include "clang/Basic/TargetInfo.h" @@ -65,7 +65,15 @@ void Sema::DiagnoseUnusedExprResult(const Stmt *S) { if (!E->isUnusedResultAWarning(Loc, R1, R2)) return; - Diag(Loc, diag::warn_unused_expr) << R1 << R2; + // Okay, we have an unused result. Depending on what the base expression is, + // we might want to make a more specific diagnostic. Check for one of these + // cases now. + unsigned DiagID = diag::warn_unused_expr; + E = E->IgnoreParens(); + if (isa(E)) + DiagID = diag::warn_unused_property_expr; + + Diag(Loc, DiagID) << R1 << R2; } Action::OwningStmtResult diff --git a/clang/test/SemaObjC/unused.m b/clang/test/SemaObjC/unused.m index ce77cf1b5f56..bbe3109a2817 100644 --- a/clang/test/SemaObjC/unused.m +++ b/clang/test/SemaObjC/unused.m @@ -25,7 +25,7 @@ int test1(void) { @end void test2() { - @"pointless example call for test purposes".length; // expected-warning {{expression result unused}} + @"pointless example call for test purposes".length; // expected-warning {{property access result unused - getters should not have side effects}} }