<div dir="ltr">I mentioned it earlier, it was causing a problem on one function. It is not a blocking issue, as mentioned I can add a void cast. But again, I really think this is missing the point. This is not the problem that LLVM_NODISCARD was designed to solve, and the choice of StringRef seems completely arbitrary. <div><br></div><div>Again, if we're adding it to StringRef, why not to every other class in LLVM? Justin mentioned he would add it to int if he could. So perhaps what he really wants is -wunused-result-strict that ignores this attribute and just warns on every unchecked return value.</div><div><br></div><div>Until someone comes up with a philosophical argument about why StringRef and ArrayRef are fundamentally different than every other type in LLVM in how they should be used, I remain unconvinced that this is an appropriate use of the attribute.</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Nov 28, 2016 at 1:11 PM Pete Cooper via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br class="gmail_msg">
> On Nov 28, 2016, at 1:02 PM, Justin Bogner via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
> David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> writes:<br class="gmail_msg">
>> I'm going to +1 Zach here, this doesn't seem like a good use of the<br class="gmail_msg">
>> attribute.<br class="gmail_msg">
>><br class="gmail_msg">
>> It's perfectly reasonable to have an API that returns an optionally useful<br class="gmail_msg">
>> StringRef (& I'd be fairly sure we have a few in the LLVM project already,<br class="gmail_msg">
><br class="gmail_msg">
> As I've stated elsewhere, I disagree that it's particularly reasonable,<br class="gmail_msg">
> and I haven't seen any concrete arguments as to why it is other than<br class="gmail_msg">
> Zach's very confusing sounding consumeInteger variant.<br class="gmail_msg">
><br class="gmail_msg">
> We certainly don't have any such functions in LLVM already - this<br class="gmail_msg">
> attribute was on StringRef for a month before any complaints and the the<br class="gmail_msg">
> only changes needed to add the attribute were:<br class="gmail_msg">
><br class="gmail_msg">
> - Adding an explicit cast to a use of an accessor that looked spurious,<br class="gmail_msg">
> but was actually just highly misleading by calling an accessor for<br class="gmail_msg">
> side effects only (r284341)<br class="gmail_msg">
> - Casting some accessors to void in a unit test that didn't bother to<br class="gmail_msg">
> actually check anything (r284350)<br class="gmail_msg">
> - Removing a StringRef return that was never used (r284348)<br class="gmail_msg">
Honestly i don't really mind whether we end up with the attribute or not, but right now i haven't seen a good reason to remove it.<br class="gmail_msg">
<br class="gmail_msg">
Zachary, I know you are active in LLDB. Is this causing problems in that codebase that we just aren't aware of?<br class="gmail_msg">
<br class="gmail_msg">
Pete<br class="gmail_msg">
><br class="gmail_msg">
>> but even if we don't I'd still be in favor of continuing to allow such API<br class="gmail_msg">
>> design without workarounds like void casts or extra attributes to opt out -<br class="gmail_msg">
>> especially considering templates in APIs that might expose such things<br class="gmail_msg">
>> indirectly (& possibly be unannotatable because they're in the STL))<br class="gmail_msg">
>><br class="gmail_msg">
>> LLVM_ATTRIBUTE_UNUSED_RESULT on a type was really designed/intended for<br class="gmail_msg">
>> types like llvm::Error (eg:<br class="gmail_msg">
>> <a href="http://google.github.io/google-api-cpp-client/latest/doxygen/classgoogleapis_1_1util_1_1Status.html" rel="noreferrer" class="gmail_msg" target="_blank">http://google.github.io/google-api-cpp-client/latest/doxygen/classgoogleapis_1_1util_1_1Status.html</a><br class="gmail_msg">
>> ).<br class="gmail_msg">
>> It doesn't seem appropriate to put this on generic value types (& as Zach<br class="gmail_msg">
>> said - if that were the case we'd probably end up putting it on basically<br class="gmail_msg">
>> every type, and at that point should consider a generic warning for ignored<br class="gmail_msg">
>> return values - but I don't think that would be practical)<br class="gmail_msg">
>><br class="gmail_msg">
>> - Dave<br class="gmail_msg">
>><br class="gmail_msg">
>> On Sun, Oct 16, 2016 at 11:44 PM Justin Bogner via llvm-commits <<br class="gmail_msg">
>> <a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br class="gmail_msg">
>><br class="gmail_msg">
>>> Author: bogner<br class="gmail_msg">
>>> Date: Mon Oct 17 01:35:23 2016<br class="gmail_msg">
>>> New Revision: 284364<br class="gmail_msg">
>>><br class="gmail_msg">
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=284364&view=rev" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=284364&view=rev</a><br class="gmail_msg">
>>> Log:<br class="gmail_msg">
>>> ADT: Use LLVM_NODISCARD instead of LLVM_ATTRIBUTE_UNUSED_RESULT for<br class="gmail_msg">
>>> StringRef<br class="gmail_msg">
>>><br class="gmail_msg">
>>> Instead of annotating (most of) the StringRef API, we can just<br class="gmail_msg">
>>> annotate the type directly. This is less code and it will warn in more<br class="gmail_msg">
>>> cases.<br class="gmail_msg">
>>><br class="gmail_msg">
>>> Modified:<br class="gmail_msg">
>>> llvm/trunk/include/llvm/ADT/StringRef.h<br class="gmail_msg">
>>><br class="gmail_msg">
>>> Modified: llvm/trunk/include/llvm/ADT/StringRef.h<br class="gmail_msg">
>>> URL:<br class="gmail_msg">
>>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringRef.h?rev=284364&r1=284363&r2=284364&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringRef.h?rev=284364&r1=284363&r2=284364&view=diff</a><br class="gmail_msg">
>>><br class="gmail_msg">
>>> ==============================================================================<br class="gmail_msg">
>>> --- llvm/trunk/include/llvm/ADT/StringRef.h (original)<br class="gmail_msg">
>>> +++ llvm/trunk/include/llvm/ADT/StringRef.h Mon Oct 17 01:35:23 2016<br class="gmail_msg">
>>> @@ -44,7 +44,7 @@ namespace llvm {<br class="gmail_msg">
>>> /// situations where the character data resides in some other buffer,<br class="gmail_msg">
>>> whose<br class="gmail_msg">
>>> /// lifetime extends past that of the StringRef. For this reason, it is<br class="gmail_msg">
>>> not in<br class="gmail_msg">
>>> /// general safe to store a StringRef.<br class="gmail_msg">
>>> - class StringRef {<br class="gmail_msg">
>>> + class LLVM_NODISCARD StringRef {<br class="gmail_msg">
>>> public:<br class="gmail_msg">
>>> typedef const char *iterator;<br class="gmail_msg">
>>> typedef const char *const_iterator;<br class="gmail_msg">
>>> @@ -281,8 +281,8 @@ namespace llvm {<br class="gmail_msg">
>>> ///<br class="gmail_msg">
>>> /// \returns The index of the first character satisfying \p F<br class="gmail_msg">
>>> starting from<br class="gmail_msg">
>>> /// \p From, or npos if not found.<br class="gmail_msg">
>>> + LLVM_NODISCARD<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> size_t find_if(function_ref<bool(char)> F, size_t From = 0) const {<br class="gmail_msg">
>>> StringRef S = drop_front(From);<br class="gmail_msg">
>>> while (!S.empty()) {<br class="gmail_msg">
>>> @@ -297,8 +297,8 @@ namespace llvm {<br class="gmail_msg">
>>> ///<br class="gmail_msg">
>>> /// \returns The index of the first character not satisfying \p F<br class="gmail_msg">
>>> starting<br class="gmail_msg">
>>> /// from \p From, or npos if not found.<br class="gmail_msg">
>>> + LLVM_NODISCARD<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> size_t find_if_not(function_ref<bool(char)> F, size_t From = 0) const<br class="gmail_msg">
>>> {<br class="gmail_msg">
>>> return find_if([F](char c) { return !F(c); }, From);<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>> @@ -500,7 +500,6 @@ namespace llvm {<br class="gmail_msg">
>>> /// exceeds the number of characters remaining in the string, the<br class="gmail_msg">
>>> string<br class="gmail_msg">
>>> /// suffix (starting with \p Start) will be returned.<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef substr(size_t Start, size_t N = npos) const {<br class="gmail_msg">
>>> Start = std::min(Start, Length);<br class="gmail_msg">
>>> return StringRef(Data + Start, std::min(N, Length - Start));<br class="gmail_msg">
>>> @@ -510,7 +509,6 @@ namespace llvm {<br class="gmail_msg">
>>> /// elements remaining. If \p N is greater than the length of the<br class="gmail_msg">
>>> /// string, the entire string is returned.<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef take_front(size_t N = 1) const {<br class="gmail_msg">
>>> if (N >= size())<br class="gmail_msg">
>>> return *this;<br class="gmail_msg">
>>> @@ -521,7 +519,6 @@ namespace llvm {<br class="gmail_msg">
>>> /// elements remaining. If \p N is greater than the length of the<br class="gmail_msg">
>>> /// string, the entire string is returned.<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef take_back(size_t N = 1) const {<br class="gmail_msg">
>>> if (N >= size())<br class="gmail_msg">
>>> return *this;<br class="gmail_msg">
>>> @@ -531,7 +528,6 @@ namespace llvm {<br class="gmail_msg">
>>> /// Return the longest prefix of 'this' such that every character<br class="gmail_msg">
>>> /// in the prefix satisfies the given predicate.<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef take_while(function_ref<bool(char)> F) const {<br class="gmail_msg">
>>> return substr(0, find_if_not(F));<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>> @@ -539,7 +535,6 @@ namespace llvm {<br class="gmail_msg">
>>> /// Return the longest prefix of 'this' such that no character in<br class="gmail_msg">
>>> /// the prefix satisfies the given predicate.<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef take_until(function_ref<bool(char)> F) const {<br class="gmail_msg">
>>> return substr(0, find_if(F));<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>> @@ -547,7 +542,6 @@ namespace llvm {<br class="gmail_msg">
>>> /// Return a StringRef equal to 'this' but with the first \p N<br class="gmail_msg">
>>> elements<br class="gmail_msg">
>>> /// dropped.<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef drop_front(size_t N = 1) const {<br class="gmail_msg">
>>> assert(size() >= N && "Dropping more elements than exist");<br class="gmail_msg">
>>> return substr(N);<br class="gmail_msg">
>>> @@ -556,7 +550,6 @@ namespace llvm {<br class="gmail_msg">
>>> /// Return a StringRef equal to 'this' but with the last \p N elements<br class="gmail_msg">
>>> /// dropped.<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef drop_back(size_t N = 1) const {<br class="gmail_msg">
>>> assert(size() >= N && "Dropping more elements than exist");<br class="gmail_msg">
>>> return substr(0, size()-N);<br class="gmail_msg">
>>> @@ -565,7 +558,6 @@ namespace llvm {<br class="gmail_msg">
>>> /// Return a StringRef equal to 'this', but with all characters<br class="gmail_msg">
>>> satisfying<br class="gmail_msg">
>>> /// the given predicate dropped from the beginning of the string.<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef drop_while(function_ref<bool(char)> F) const {<br class="gmail_msg">
>>> return substr(find_if_not(F));<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>> @@ -573,15 +565,14 @@ namespace llvm {<br class="gmail_msg">
>>> /// Return a StringRef equal to 'this', but with all characters not<br class="gmail_msg">
>>> /// satisfying the given predicate dropped from the beginning of the<br class="gmail_msg">
>>> string.<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef drop_until(function_ref<bool(char)> F) const {<br class="gmail_msg">
>>> return substr(find_if(F));<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>><br class="gmail_msg">
>>> /// Returns true if this StringRef has the given prefix and removes<br class="gmail_msg">
>>> that<br class="gmail_msg">
>>> /// prefix.<br class="gmail_msg">
>>> + LLVM_NODISCARD<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> bool consume_front(StringRef Prefix) {<br class="gmail_msg">
>>> if (!startswith(Prefix))<br class="gmail_msg">
>>> return false;<br class="gmail_msg">
>>> @@ -592,8 +583,8 @@ namespace llvm {<br class="gmail_msg">
>>><br class="gmail_msg">
>>> /// Returns true if this StringRef has the given suffix and removes<br class="gmail_msg">
>>> that<br class="gmail_msg">
>>> /// suffix.<br class="gmail_msg">
>>> + LLVM_NODISCARD<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> bool consume_back(StringRef Suffix) {<br class="gmail_msg">
>>> if (!endswith(Suffix))<br class="gmail_msg">
>>> return false;<br class="gmail_msg">
>>> @@ -614,7 +605,6 @@ namespace llvm {<br class="gmail_msg">
>>> /// will be returned. If this is less than \p Start, an empty string<br class="gmail_msg">
>>> will<br class="gmail_msg">
>>> /// be returned.<br class="gmail_msg">
>>> LLVM_ATTRIBUTE_ALWAYS_INLINE<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef slice(size_t Start, size_t End) const {<br class="gmail_msg">
>>> Start = std::min(Start, Length);<br class="gmail_msg">
>>> End = std::min(std::max(Start, End), Length);<br class="gmail_msg">
>>> @@ -709,42 +699,36 @@ namespace llvm {<br class="gmail_msg">
>>><br class="gmail_msg">
>>> /// Return string with consecutive \p Char characters starting from<br class="gmail_msg">
>>> the<br class="gmail_msg">
>>> /// the left removed.<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef ltrim(char Char) const {<br class="gmail_msg">
>>> return drop_front(std::min(Length, find_first_not_of(Char)));<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>><br class="gmail_msg">
>>> /// Return string with consecutive characters in \p Chars starting<br class="gmail_msg">
>>> from<br class="gmail_msg">
>>> /// the left removed.<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef ltrim(StringRef Chars = " \t\n\v\f\r") const {<br class="gmail_msg">
>>> return drop_front(std::min(Length, find_first_not_of(Chars)));<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>><br class="gmail_msg">
>>> /// Return string with consecutive \p Char characters starting from<br class="gmail_msg">
>>> the<br class="gmail_msg">
>>> /// right removed.<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef rtrim(char Char) const {<br class="gmail_msg">
>>> return drop_back(Length - std::min(Length, find_last_not_of(Char) +<br class="gmail_msg">
>>> 1));<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>><br class="gmail_msg">
>>> /// Return string with consecutive characters in \p Chars starting<br class="gmail_msg">
>>> from<br class="gmail_msg">
>>> /// the right removed.<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef rtrim(StringRef Chars = " \t\n\v\f\r") const {<br class="gmail_msg">
>>> return drop_back(Length - std::min(Length, find_last_not_of(Chars)<br class="gmail_msg">
>>> + 1));<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>><br class="gmail_msg">
>>> /// Return string with consecutive \p Char characters starting from<br class="gmail_msg">
>>> the<br class="gmail_msg">
>>> /// left and right removed.<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef trim(char Char) const {<br class="gmail_msg">
>>> return ltrim(Char).rtrim(Char);<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>><br class="gmail_msg">
>>> /// Return string with consecutive characters in \p Chars starting<br class="gmail_msg">
>>> from<br class="gmail_msg">
>>> /// the left and right removed.<br class="gmail_msg">
>>> - LLVM_ATTRIBUTE_UNUSED_RESULT<br class="gmail_msg">
>>> StringRef trim(StringRef Chars = " \t\n\v\f\r") const {<br class="gmail_msg">
>>> return ltrim(Chars).rtrim(Chars);<br class="gmail_msg">
>>> }<br class="gmail_msg">
>>><br class="gmail_msg">
>>><br class="gmail_msg">
>>> _______________________________________________<br class="gmail_msg">
>>> llvm-commits mailing list<br class="gmail_msg">
>>> <a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
>>><br class="gmail_msg">
> _______________________________________________<br class="gmail_msg">
> llvm-commits mailing list<br class="gmail_msg">
> <a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
<br class="gmail_msg">
_______________________________________________<br class="gmail_msg">
llvm-commits mailing list<br class="gmail_msg">
<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
</blockquote></div>