<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=""><br class=""><div><blockquote type="cite" class=""><div class="">On Nov 28, 2016, at 8:59 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" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Sun, Nov 27, 2016 at 10:53 PM 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: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Nov 27, 2016, at 10:24 PM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:</div><br class="gmail_msg m_533053069121649671Apple-interchange-newline"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Sun, Nov 27, 2016 at 8:36 PM 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: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class="gmail_msg">> On Nov 27, 2016, at 6:24 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"><br class="gmail_msg">> Similarly, what functions return a `const char *` where you<br class="gmail_msg">> don't care about the return value? I feel like the vast majority of<br class="gmail_msg">> functions that return either a `StringRef` or a `const char *` expect<br class="gmail_msg">> you to use the return value, and if you're ignoring it you probably<br class="gmail_msg">> either called the wrong function or are doing something weird (like<br class="gmail_msg">> calling an accessor function for side effects) and having to add a void<br class="gmail_msg">> cast will make that clearer.<br class="gmail_msg">><br class="gmail_msg">> I feel pretty strongly that having nodiscard on StringRef makes for<br class="gmail_msg">> overall better code, so I'm certainly opposed to removing it like<br class="gmail_msg">> r287586 does. What real world code motivates this?<br class="gmail_msg"><br class="gmail_msg">That’s a legitimate question!<br class="gmail_msg"></blockquote><div class="gmail_msg"> </div><div class="gmail_msg">Tomorrow I can find the code that was triggering this warning which is what led me to it in the first place, but I think this is really missing the point. </div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><div class="gmail_msg">The point here is that if after years of uses in the LLVM projects, we never made use of an API where the no_discard does not make sense, it seems like good data on what make sense from an API point of view for StringRef. I don’t think this is irrelevant.</div></div></div></blockquote><div class="">We could just as easily say that if after years of uses in the LLVM projects, we never made use of an API where no_discard would trigger a warning, then it is not providing a meaningful signal. </div></div></div></div></blockquote><div><br class=""></div><div>That’s not much different from what I said I think, basically look at the existing use to decide if it makes sense or not.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote"><div class=""> So I don't think this is a useful metric either for or against.</div></div></div></div></blockquote><div><br class=""></div><div>I don’t get that.</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote"><div class=""><br class=""></div><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">We are a library, we cannot make any assumptions about how users of our library use our support classes. </div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><div class="gmail_msg">Right, that’s why instead of making assumptions, looking at the codebase as whole answers the question perfectly.</div></div></div></blockquote><div class="">Libraries, by definition, cannot possibly see all of its uses.</div></div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">They are intended to be generic and serve arbitrary use cases, some of which are downstream and which we cannot possibly know about. To impose what we consider good or bad API design on someone else's product is not something we should be in the business of, and we certainly should not be doing it via compiler errors (e.g. if someone is using -Werror) if someone chooses an API design that we deem "bad”. <br class="gmail_msg"></div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><div class="gmail_msg">I disagree with this: we’re not in the business of designing ADT for downstream users in the first place: if they like it and want to use it, great! Otherwise I believe we should put the needs of the LLVM projects first: we design our support libraries to make LLVM more robust, faster, etc. I wouldn’t want to tradeoff this for hypothetical downstream users.</div></div></div><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">I'm not taking a position on whether or not LLVM_NODISCARD on StringRef leads to overall better code,</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><div class="gmail_msg">That’s where I have a different approach, it seems to be the main trigger to me.</div></div></div><div class="gmail_msg" style="word-wrap: break-word;"><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"><div class="gmail_quote gmail_msg"><div class="gmail_msg">I'm only arguing that it should not be used at a class level except in cases where a return value must be checked **by design**. StringRef is not such a class. Neither are ArrayRef and a couple of other classes which have also had this attribute applied. </div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">If we're going to put it on StringRef, then we need to justify why it's not on APFloat or Optional<T> or PointerIntPair in the name of consistency. What makes StringRef fundamentally different than pretty much any other type anywhere with regards to when it should be checked as a return value?</div></div></div></div></blockquote><br class="gmail_msg"></div></div><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"></div><div class="gmail_msg">Consistency here is answering for each of the types you mention: “whether or not LLVM_NODISCARD on XYZ leads to overall better code”. Possibly trading-off with how many times an explicit (void) is needed at call-sites.</div></div></blockquote><div class=""> </div><div class="">Why would it lead to better code for one and not the other? You're going to have to explain what's fundamentally different about these two classes for there to be a reasonable argument that one deserves the attribute and the other doesn't. "Because existing code already satisfies it for X and not for Y" is not a compelling argument.</div></div></div></div></blockquote><br class=""></div><div>We’re not designing anything from scratch here, we have a large codebase, I believe this is enough to drive these kind of decision.</div><div><br class=""></div><div>— </div><div>Mehdi</div><div><br class=""></div><br class=""></body></html>