<div dir="ltr">Merging Erich Keane's response here. Somehow his reply branched.<div><br></div><div><br></div><div><span style="font-size:12.8px">Responses inline, note that Aaron is likely thebest one to :</span><br style="font-size:12.8px"><br style="font-size:12.8px"><span style="font-size:12.8px">I think this is because of the behavior outlined here.</span><br style="font-size:12.8px"><br style="font-size:12.8px"><span style="font-size:12.8px"> /// \brief Returns true if this function or its return type has the</span><br style="font-size:12.8px"><span style="font-size:12.8px"> /// warn_unused_result attribute. If the return type has the attribute and</span><br style="font-size:12.8px"><span style="font-size:12.8px"> /// this function is a method of the return type's class, then false will be</span><br style="font-size:12.8px"><span style="font-size:12.8px"> /// returned to avoid spurious warnings on member methods such as assignment</span><br style="font-size:12.8px"><span style="font-size:12.8px"> /// operators.</span><br style="font-size:12.8px"><span style="font-size:12.8px"> bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; }</span><br style="font-size:12.8px"><span style="font-size:12.8px">[Keane, Erich] I don't really see a situation where the exception for spurious warnings is correct. Assignment operators typically return a reference, thus this is suppressed anyway. Either way, this is a much bigger hammer than it should use, I suspect we should just special-case the ones we wish to suppress. What is particularly annoying about this suppression is that it will only work for MEMBER operators, not all operators, so it is wrong in the first place!! Looking through operators only the following would be subject to the warning anyway:</span><br style="font-size:12.8px"><br style="font-size:12.8px"><span style="font-size:12.8px">1- Post increment/decrement <-- warning is possibly invalid?</span><br style="font-size:12.8px"><span style="font-size:12.8px">2- arithmetic operators <-- warning is probably valid.</span><br style="font-size:12.8px"><br style="font-size:12.8px"><span style="font-size:12.8px">Looking through history, it seems that the suppression of the warning on references/pointers was added AFTER this, so I wonder if simply removing the "</span><wbr style="font-size:12.8px"><span style="font-size:12.8px">getCorrespondingMethodInClass" condition is the right thing? If we decide situation 1 above should be excepted, we should specifically except IT, for both member and non-member.</span><br></div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature" data-smartmail="gmail_signature">~Craig</div></div>
<br><div class="gmail_quote">On Tue, Apr 18, 2017 at 9:56 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">*nod* maybe it could be filtered to only suppress that when the result is a T& but not a T value. That'd catch the APInt::trunc case, but not produce false positives for assignment operators..</div><br><div class="gmail_quote"><div><div class="h5"><div dir="ltr">On Tue, Apr 18, 2017 at 12:05 AM Craig Topper via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><div dir="ltr">I think this is because of the behavior outlined here.<div><br></div><div><div> /// \brief Returns true if this function or its return type has the</div><div> /// warn_unused_result attribute. If the return type has the attribute and</div><div> /// this function is a method of the return type's class, then false will be</div><div> /// returned to avoid spurious warnings on member methods such as assignment</div><div> /// operators.</div><div> bool hasUnusedResultAttr() const { return getUnusedResultAttr() != nullptr; }</div></div></div><div class="gmail_extra"></div><div class="gmail_extra"><br clear="all"><div><div class="m_-6779883503737819511m_5834635147297841932gmail_signature" data-smartmail="gmail_signature">~Craig</div></div></div><div class="gmail_extra">
<br><div class="gmail_quote">On Mon, Apr 17, 2017 at 11:48 PM, Craig Topper <span dir="ltr"><<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>It seems if warn_unused_result is put on a class definition it doesn't apply to any of the methods in that class that return the class type?</div><div><br></div><div>This test should generate 2 warnings but only generates one.</div><div><br></div><div><a href="https://godbolt.org/g/kImv9k" target="_blank">https://godbolt.org/g/kImv9k</a><br></div><div><br></div><div>The ArrayRef and APInt classes both do this. I accidentally created a case where I didn't consume the result of APInt::trunc and received no warning.</div><span class="m_-6779883503737819511m_5834635147297841932HOEnZb"><font color="#888888"><br clear="all"><div><div class="m_-6779883503737819511m_5834635147297841932m_-2057099972093008886gmail_signature">~Craig</div></div>
</font></span></div>
</blockquote></div><br></div></div></div>
______________________________<wbr>_________________<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/<wbr>mailman/listinfo/cfe-dev</a><br>
</blockquote></div>
</blockquote></div><br></div>