Properly implement warn_unused_result checking for classes/structs.

Aaron Ballman aaron at aaronballman.com
Fri Apr 3 17:48:47 PDT 2015


LGTM!

~Aaron

On Fri, Apr 3, 2015 at 3:49 PM, Kaelyn Takata <rikka at google.com> wrote:
> 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
>> >
>
>



More information about the cfe-commits mailing list