<div dir="ltr">GCC only supports this on functions - so there's no behavior we need to be consistent with in that regard:<br><br><div>warn.cpp:1:44: warning: ‘warn_unused_result’ attribute only applies to function types [-Wattributes]</div><div> struct __attribute__((warn_unused_result)) foo {</div><div>                                            ^~~<br><br>(I think we support a lot of attributes in other compiler's syntaxes even though those other compilers don't support the attribute themselves (especially eg: MSVC syntax, I think) - but I don't have much specific/concrete evidence to backup that claim)</div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Apr 19, 2017 at 5:58 AM Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, Apr 18, 2017 at 6:55 PM, Craig Topper via cfe-dev<br>
<<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
> Merging Erich Keane's response here. Somehow his reply branched.<br>
><br>
><br>
> Responses inline, note that Aaron is likely thebest one to :<br>
><br>
> I think this is because of the behavior outlined here.<br>
><br>
>   /// \brief Returns true if this function or its return type has the<br>
>   /// warn_unused_result attribute. If the return type has the attribute and<br>
>   /// this function is a method of the return type's class, then false will<br>
> be<br>
>   /// returned to avoid spurious warnings on member methods such as<br>
> assignment<br>
>   /// operators.<br>
>   bool hasUnusedResultAttr() const { return getUnusedResultAttr() !=<br>
> nullptr; }<br>
> [Keane, Erich] I don't really see a situation where the exception for<br>
> spurious warnings is correct.  Assignment operators typically return a<br>
> reference, thus this is suppressed anyway.  Either way, this is a much<br>
> bigger hammer than it should use, I suspect we should just special-case the<br>
> ones we wish to suppress.  What is particularly annoying about this<br>
> suppression is that it will only work for MEMBER operators, not all<br>
> operators, so it is wrong in the first place!! Looking through operators<br>
> only the following would be subject to the warning anyway:<br>
><br>
> 1- Post increment/decrement <-- warning is possibly invalid?<br>
> 2- arithmetic operators  <-- warning is probably valid.<br>
><br>
> Looking through history, it seems that the suppression of the warning on<br>
> references/pointers was added AFTER this, so I wonder if simply removing the<br>
> "getCorrespondingMethodInClass" condition is the right thing?  If we decide<br>
> situation 1 above should be excepted, we should specifically except IT, for<br>
> both member and non-member.<br>
<br>
Keep in mind that this attribute has three different spellings:<br>
<br>
* [[clang::warn_unused_result]] -- we can do whatever we want here.<br>
* __attribute__((warn_unused_result)) and [[gnu:warn_unused_result]]<br>
-- we have some latitude, but should prefer to behave the same way as<br>
GCC.<br>
* [[nodiscard]] -- we should follow what the standard says.<br>
<br>
For the [[nodiscard]] spelling, it sounds like this is behaving<br>
incorrectly already. For the [[clang::warn_unused_result]] spelling, I<br>
agree with Erich -- this seems like a bigger hammer than we need, and<br>
it surely should work for nonmember operators as well as member<br>
operators. I would probably diagnose on post inc/dec (and arithmetic<br>
operators) because ignoring that result when the class is marked to<br>
warn on unused results seems in keeping with the intent of the<br>
attribute. For the GNU spellings, I'd say we should mimic whatever GCC<br>
does if we think it's defensible and it isn't a burden to support that<br>
way.<br>
<br>
~Aaron<br>
<br>
><br>
> ~Craig<br>
><br>
> On Tue, Apr 18, 2017 at 9:56 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>> *nod* maybe it could be filtered to only suppress that when the result is<br>
>> a T& but not a T value. That'd catch the APInt::trunc case, but not produce<br>
>> false positives for assignment operators..<br>
>><br>
>> On Tue, Apr 18, 2017 at 12:05 AM Craig Topper via cfe-dev<br>
>> <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
>>><br>
>>> I think this is because of the behavior outlined here.<br>
>>><br>
>>>   /// \brief Returns true if this function or its return type has the<br>
>>>   /// warn_unused_result attribute. If the return type has the attribute<br>
>>> and<br>
>>>   /// this function is a method of the return type's class, then false<br>
>>> will be<br>
>>>   /// returned to avoid spurious warnings on member methods such as<br>
>>> assignment<br>
>>>   /// operators.<br>
>>>   bool hasUnusedResultAttr() const { return getUnusedResultAttr() !=<br>
>>> nullptr; }<br>
>>><br>
>>> ~Craig<br>
>>><br>
>>> On Mon, Apr 17, 2017 at 11:48 PM, Craig Topper <<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>><br>
>>> wrote:<br>
>>>><br>
>>>> It seems if warn_unused_result is put on a class definition it doesn't<br>
>>>> apply to any of the methods in that class that return the class type?<br>
>>>><br>
>>>> This test should generate 2 warnings but only generates one.<br>
>>>><br>
>>>> <a href="https://godbolt.org/g/kImv9k" rel="noreferrer" target="_blank">https://godbolt.org/g/kImv9k</a><br>
>>>><br>
>>>> The ArrayRef and APInt classes both do this. I accidentally created a<br>
>>>> case where I didn't consume the result of APInt::trunc and received no<br>
>>>> warning.<br>
>>>><br>
>>>> ~Craig<br>
>>><br>
>>><br>
>>> _______________________________________________<br>
>>> cfe-dev mailing list<br>
>>> <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> cfe-dev mailing list<br>
> <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
><br>
</blockquote></div>