Properly implement warn_unused_result checking for classes/structs.

Kaelyn Takata rikka at google.com
Fri Apr 3 12:49:58 PDT 2015


After a bit of internal testing and discussion, I've updated the patch to
not look through pointers and references when checking if the return type
of a function has the warn_unused_result. This fixes a lot of bogus
warnings from functions like std::copy which return an iterator but for
which the return value doesn't matter, when the iterator is defined as a
raw pointer. The core idea behind the change is that a pointer or reference
return type don't imply a transfer of ownership of the underlying object in
a way that suggests the contents of the object should be accessed before
the object goes out of scope.

On Thu, Apr 2, 2015 at 7:20 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> LGTM!
>
> ~Aaron
>
> On Wed, Apr 1, 2015 at 7:30 PM, Kaelyn Takata <rikka at google.com> wrote:
> > 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.
> > </commit-message>
> >
> > I've unified the two locations that performed the check into a helper
> method
> > of FunctionDecl that returns true if the decl or its return type have the
> > warn_unused_result attribute, except if only the return type has the
> > attribute and the decl is a member of the return type (which avoids
> spurious
> > messages on e.g. assignment operators).
> >
> > Cheers,
> > Kaelyn
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150403/175228d6/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-class-warn_unused_result-v2.patch
Type: text/x-patch
Size: 6040 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150403/175228d6/attachment.bin>


More information about the cfe-commits mailing list