<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">For what it's worth, I also feel that adding LLVM_NODISCARD to StringRef (and APInt) is a bit excessive. I can see two reasons why one would use LLVM_NODISCARD:</div><div class=""><br class=""></div><div class="">- Performance: They both should end up occupying two GP registers so not really a concern, the classes seem simple enough to me that the compiler should be able to optimize away the values when they are not used.</div><div class="">- API contracts: This is obvious for cases like error values which should not be discarded. However I find having APIs with an "optional" return value legitimate and I see no reason why you wouldn't want to use a StringRef or APInt there sometimes. There are a lot of APIs returning bools or iterators that are optional I don't see why StringRef/APInt would be special here and force me to "(void)" cast the result.</div><div class=""><br class=""></div><div class="">- Matthias</div><div class=""><br class=""></div><div><blockquote type="cite" class=""><div class="">On Nov 28, 2016, at 10:32 AM, Mehdi Amini via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Nov 28, 2016, at 10:13 AM, Zachary Turner <<a href="mailto:zturner@google.com" class="">zturner@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Well, I mean for starters it assumes that our existing codebase is an absolutely inarguable model of what's good :-/ That doesn't mean it's bad, I certainly think we have an above average codebase, but I don't see how we can use the mere existence of something as evidence of its quality.</div></div></blockquote><div class=""><br class=""></div><div class="">I’m not sure why “our existing codebase is the model of good” is a premise of the reasoning here. My point is rather that our codebase is the best example of what kind of APIs are suited for what we do and our style of development.</div><div class="">This does not have to be immutable, and indeed it evolved a lot over the time. However for such kind of decision (that can be easily reverted), it makes sense to me to look at what’s appropriate for our current state, just from a point of view of cost/benefit.</div><div class=""><br class=""></div><div class=""><div class="">I’m not really convinced that it is an issue that some of our APIs wouldn’t make sense if they were in Boost for example, and symmetrically, even if I tend to believe that Boost APIs are likely better designed that ours, what makes sense there is not necessarily the best in our codebase / for our immediate needs.</div><div class=""><br class=""></div></div><div class="">— </div><div class="">Mehdi</div><div class=""><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Mon, Nov 28, 2016 at 9:59 AM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" class="">mehdi.amini@apple.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Nov 28, 2016, at 9:55 AM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:</div><br class="gmail_msg m_8968939757515517270Apple-interchange-newline"><div class="gmail_msg"><div dir="ltr" class="gmail_msg">The real use case in question was a function very similar to StringRef's consumeInteger function, but instead of modifying the StringRef in place (e.g. by accepting a StringRef&) to point to the first portion after the consumed string, it returned a StringRef representing the unconsumed portion of the input. <div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">A caller not interested in the unconsumed portion of the input would simply ignore the return value.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">This aside, I still think this is irrelevant. If we're going to make the blanket decision that ignoring return values is bad, then add -Wunused-result-strict and try to get it turned on across the board in LLVM. "It's good API design to not ignore StringRef return values because nobody has ever ignored a StringRef return value before" is a dubious argument at best.</div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">I strongly disagree with this assessment, it has nothing dubious in my opinion.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">On the opposite, I found dubious at best that you find this dubious :p</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">— </div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">Mehdi</div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Mon, Nov 28, 2016 at 9:52 AM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" class="gmail_msg" target="_blank">mehdi.amini@apple.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Nov 28, 2016, at 9:18 AM, David Blaikie via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="gmail_msg m_8968939757515517270m_4050574981016713563Apple-interchange-newline"><div class="gmail_msg"><div dir="ltr" class="gmail_msg">I'm going to +1 Zach here, this doesn't seem like a good use of the attribute.<br class="gmail_msg"><br class="gmail_msg">It's perfectly reasonable to have an API that returns an optionally useful StringRef (& I'd be fairly sure we have a few in the LLVM project already, but even if we don't I'd still be in favor of continuing to allow such API design without workarounds like void casts or extra attributes to opt out - especially considering templates in APIs that might expose such things indirectly (& possibly be unannotatable because they're in the STL))<br class="gmail_msg"></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">Since the attribute was committed for some time and no failure was reported, that seems quite hypothetical.</div><div class="gmail_msg">I rather see a real use-case in our codebase before ditching something like that speculatively.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">— </div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">Mehdi</div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><br class="gmail_msg">LLVM_ATTRIBUTE_UNUSED_RESULT on a type was really designed/intended for types like llvm::Error (eg: <a href="http://google.github.io/google-api-cpp-client/latest/doxygen/classgoogleapis_1_1util_1_1Status.html" class="gmail_msg" target="_blank">http://google.github.io/google-api-cpp-client/latest/doxygen/classgoogleapis_1_1util_1_1Status.html</a> ). It doesn't seem appropriate to put this on generic value types (& as Zach said - if that were the case we'd probably end up putting it on basically every type, and at that point should consider a generic warning for ignored 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"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Sun, Oct 16, 2016 at 11:44 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"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 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: <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">
--- 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, whose<br class="gmail_msg">
/// lifetime extends past that of the StringRef. For this reason, it is 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 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 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">
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 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 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 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 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 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 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 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 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 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 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) + 1));<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
/// Return string with consecutive characters in \p Chars starting 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) + 1));<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
/// Return string with consecutive \p Char characters starting from 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 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">
</blockquote></div></div>
_______________________________________________<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" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg"></div></blockquote></div></div></blockquote></div>
</div></blockquote></div></div></blockquote></div>
</div></blockquote></div><br class=""></div>_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""></body></html>