r234526 - Properly implement warn_unused_result checking for classes/structs.
Kaelyn Takata
rikka at google.com
Thu Apr 9 12:43:05 PDT 2015
Author: rikka
Date: Thu Apr 9 14:43:04 2015
New Revision: 234526
URL: http://llvm.org/viewvc/llvm-project?rev=234526&view=rev
Log:
Properly implement warn_unused_result checking for classes/structs.
The previous implementation would copy the attribute from the class to
functions that have the class as their return type when the functions
are first declared. This proved to have two flaws:
1) if the class is forward-declared without the attribute and a
function or method with the class as a its return type is declared,
and afterward the class is defined with warn_unused_result, the
function or method would never inherit the attribute, and
2) the check simply failed for functions and methods that are part of
a template instantiation, regardless of whether the class with
warn_unused_result is part of a specific instantiation or part of
the template itself (presumably because those function/method
declaration does not hit the same code path as a non-template one
and so never inherits the attribute).
The new approach is to instead modify the two places where a function or
method call is checked for the warn_unused_result attribute on the decl
by extending the checks to also look for the attribute on the decl's
return type.
Additionally, the check for return types that have the warn_unused_result
now excludes pointers and references to such types, as such return types do
not necessarily imply a transfer of ownership for the underlying object
being referred to by the return value. This does not change the behavior
of functions that are directly given the warn_unused_result attribute.
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/SemaCXX/warn-unused-result.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=234526&r1=234525&r2=234526&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Thu Apr 9 14:43:04 2015
@@ -1965,6 +1965,13 @@ public:
return getType()->getAs<FunctionType>()->getCallResultType(getASTContext());
}
+ /// \brief Returns true if this function or its return type has the
+ /// warn_unused_result attribute. If the return type has the attribute and
+ /// this function is a method of the return type's class, then false will be
+ /// returned to avoid spurious warnings on member methods such as assignment
+ /// operators.
+ bool hasUnusedResultAttr() const;
+
/// \brief Returns the storage class as written in the source. For the
/// computed linkage of symbol, see getLinkage.
StorageClass getStorageClass() const { return StorageClass(SClass); }
Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=234526&r1=234525&r2=234526&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Thu Apr 9 14:43:04 2015
@@ -2822,6 +2822,18 @@ SourceRange FunctionDecl::getReturnTypeS
return RTRange;
}
+bool FunctionDecl::hasUnusedResultAttr() const {
+ QualType RetType = getReturnType();
+ if (RetType->isRecordType()) {
+ const CXXRecordDecl *Ret = RetType->getAsCXXRecordDecl();
+ const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(this);
+ if (Ret && Ret->hasAttr<WarnUnusedResultAttr>() &&
+ !(MD && MD->getCorrespondingMethodInClass(Ret, true)))
+ return true;
+ }
+ return hasAttr<WarnUnusedResultAttr>();
+}
+
/// \brief For an inline function definition in C, or for a gnu_inline function
/// in C++, determine whether the definition will be externally visible.
///
Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=234526&r1=234525&r2=234526&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Thu Apr 9 14:43:04 2015
@@ -2161,12 +2161,16 @@ bool Expr::isUnusedResultAWarning(const
// If this is a direct call, get the callee.
const CallExpr *CE = cast<CallExpr>(this);
if (const Decl *FD = CE->getCalleeDecl()) {
+ const FunctionDecl *Func = dyn_cast<FunctionDecl>(FD);
+ bool HasWarnUnusedResultAttr = Func ? Func->hasUnusedResultAttr()
+ : FD->hasAttr<WarnUnusedResultAttr>();
+
// If the callee has attribute pure, const, or warn_unused_result, warn
// about it. void foo() { strlen("bar"); } should warn.
//
// Note: If new cases are added here, DiagnoseUnusedExprResult should be
// updated to match for QoI.
- if (FD->hasAttr<WarnUnusedResultAttr>() ||
+ if (HasWarnUnusedResultAttr ||
FD->hasAttr<PureAttr>() || FD->hasAttr<ConstAttr>()) {
WarnE = this;
Loc = CE->getCallee()->getLocStart();
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=234526&r1=234525&r2=234526&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Apr 9 14:43:04 2015
@@ -7489,23 +7489,10 @@ Sema::ActOnFunctionDeclarator(Scope *S,
// Handle attributes.
ProcessDeclAttributes(S, NewFD, D);
- QualType RetType = NewFD->getReturnType();
- const CXXRecordDecl *Ret = RetType->isRecordType() ?
- RetType->getAsCXXRecordDecl() : RetType->getPointeeCXXRecordDecl();
- if (!NewFD->isInvalidDecl() && !NewFD->hasAttr<WarnUnusedResultAttr>() &&
- Ret && Ret->hasAttr<WarnUnusedResultAttr>()) {
- const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(NewFD);
- // Attach WarnUnusedResult to functions returning types with that attribute.
- // Don't apply the attribute to that type's own non-static member functions
- // (to avoid warning on things like assignment operators)
- if (!MD || MD->getParent() != Ret)
- NewFD->addAttr(WarnUnusedResultAttr::CreateImplicit(Context));
- }
-
if (getLangOpts().OpenCL) {
// OpenCL v1.1 s6.5: Using an address space qualifier in a function return
// type declaration will generate a compilation error.
- unsigned AddressSpace = RetType.getAddressSpace();
+ unsigned AddressSpace = NewFD->getReturnType().getAddressSpace();
if (AddressSpace == LangAS::opencl_local ||
AddressSpace == LangAS::opencl_global ||
AddressSpace == LangAS::opencl_constant) {
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=234526&r1=234525&r2=234526&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Apr 9 14:43:04 2015
@@ -239,7 +239,9 @@ void Sema::DiagnoseUnusedExprResult(cons
// is written in a macro body, only warn if it has the warn_unused_result
// attribute.
if (const Decl *FD = CE->getCalleeDecl()) {
- if (FD->hasAttr<WarnUnusedResultAttr>()) {
+ const FunctionDecl *Func = dyn_cast<FunctionDecl>(FD);
+ if (Func ? Func->hasUnusedResultAttr()
+ : FD->hasAttr<WarnUnusedResultAttr>()) {
Diag(Loc, diag::warn_unused_result) << R1 << R2;
return;
}
Modified: cfe/trunk/test/SemaCXX/warn-unused-result.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-result.cpp?rev=234526&r1=234525&r2=234526&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-unused-result.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unused-result.cpp Thu Apr 9 14:43:04 2015
@@ -44,6 +44,12 @@ void bah() {
}
namespace warn_unused_CXX11 {
+class Status;
+class Foo {
+ public:
+ Status doStuff();
+};
+
struct [[clang::warn_unused_result]] Status {
bool ok() const;
Status& operator=(const Status& x);
@@ -73,10 +79,23 @@ void lazy() {
(void)DoYetAnotherThing();
DoSomething(); // expected-warning {{ignoring return value}}
- DoSomethingElse(); // expected-warning {{ignoring return value}}
- DoAnotherThing(); // expected-warning {{ignoring return value}}
+ DoSomethingElse();
+ DoAnotherThing();
DoYetAnotherThing();
}
+
+template <typename T>
+class [[clang::warn_unused_result]] StatusOr {
+};
+StatusOr<int> doit();
+void test() {
+ Foo f;
+ f.doStuff(); // expected-warning {{ignoring return value}}
+ doit(); // expected-warning {{ignoring return value}}
+
+ auto func = []() { return Status(); };
+ func(); // expected-warning {{ignoring return value}}
+}
}
namespace PR17587 {
More information about the cfe-commits
mailing list