[llvm] r287586 - Remove LLVM_NODISCARD from StringRef.
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Sun Nov 27 18:27:43 PST 2016
Zachary Turner via llvm-commits <llvm-commits at lists.llvm.org> writes:
> Author: zturner
> Date: Mon Nov 21 16:19:25 2016
> New Revision: 287586
>
> URL: http://llvm.org/viewvc/llvm-project?rev=287586&view=rev
> Log:
> Remove LLVM_NODISCARD from StringRef.
>
> This is a bit too aggressive of a warning, as it is forces
> ANY function which returns a StringRef to have its return
> value checked. While useful on classes like llvm::Error which
> are designed to require checking, this is not the case for
> StringRef, and it is perfectly reasonable to have a function
> return a StringRef for which the return value is not checked.
I'm opposed to this change, as I'm fairly convinced that functions that
return StringRefs that aren't checked are bad API. See [my reply] in the
review for r284364 for details (and let's discuss there).
[my reply]:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20161121/407851.html
> Move LLVM_NODISCARD to each of the individual member functions
> where it makes sense instead.
>
> Modified:
> llvm/trunk/include/llvm/ADT/StringRef.h
>
> Modified: llvm/trunk/include/llvm/ADT/StringRef.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringRef.h?rev=287586&r1=287585&r2=287586&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/StringRef.h (original)
> +++ llvm/trunk/include/llvm/ADT/StringRef.h Mon Nov 21 16:19:25 2016
> @@ -44,7 +44,7 @@ namespace llvm {
> /// situations where the character data resides in some other buffer, whose
> /// lifetime extends past that of the StringRef. For this reason, it is not in
> /// general safe to store a StringRef.
> - class LLVM_NODISCARD StringRef {
> + class StringRef {
> public:
> typedef const char *iterator;
> typedef const char *const_iterator;
> @@ -123,30 +123,36 @@ namespace llvm {
>
> /// data - Get a pointer to the start of the string (which may not be null
> /// terminated).
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> const char *data() const { return Data; }
>
> /// empty - Check if the string is empty.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> bool empty() const { return Length == 0; }
>
> /// size - Get the string size.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> size_t size() const { return Length; }
>
> /// front - Get the first character in the string.
> + LLVM_NODISCARD
> char front() const {
> assert(!empty());
> return Data[0];
> }
>
> /// back - Get the last character in the string.
> + LLVM_NODISCARD
> char back() const {
> assert(!empty());
> return Data[Length-1];
> }
>
> // copy - Allocate copy in Allocator and return StringRef to it.
> + LLVM_NODISCARD
> template <typename Allocator> StringRef copy(Allocator &A) const {
> // Don't request a length 0 copy from the allocator.
> if (empty())
> @@ -158,6 +164,7 @@ namespace llvm {
>
> /// equals - Check for string equality, this is more efficient than
> /// compare() when the relative ordering of inequal strings isn't needed.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> bool equals(StringRef RHS) const {
> return (Length == RHS.Length &&
> @@ -165,12 +172,14 @@ namespace llvm {
> }
>
> /// equals_lower - Check for string equality, ignoring case.
> + LLVM_NODISCARD
> bool equals_lower(StringRef RHS) const {
> return Length == RHS.Length && compare_lower(RHS) == 0;
> }
>
> /// compare - Compare two strings; the result is -1, 0, or 1 if this string
> /// is lexicographically less than, equal to, or greater than the \p RHS.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> int compare(StringRef RHS) const {
> // Check the prefix for a mismatch.
> @@ -184,10 +193,12 @@ namespace llvm {
> }
>
> /// compare_lower - Compare two strings, ignoring case.
> + LLVM_NODISCARD
> int compare_lower(StringRef RHS) const;
>
> /// compare_numeric - Compare two strings, treating sequences of digits as
> /// numbers.
> + LLVM_NODISCARD
> int compare_numeric(StringRef RHS) const;
>
> /// \brief Determine the edit distance between this string and another
> @@ -208,10 +219,12 @@ namespace llvm {
> /// or (if \p AllowReplacements is \c true) replacements needed to
> /// transform one of the given strings into the other. If zero,
> /// the strings are identical.
> + LLVM_NODISCARD
> unsigned edit_distance(StringRef Other, bool AllowReplacements = true,
> unsigned MaxEditDistance = 0) const;
>
> /// str - Get the contents as an std::string.
> + LLVM_NODISCARD
> std::string str() const {
> if (!Data) return std::string();
> return std::string(Data, Length);
> @@ -221,6 +234,7 @@ namespace llvm {
> /// @name Operator Overloads
> /// @{
>
> + LLVM_NODISCARD
> char operator[](size_t Index) const {
> assert(Index < Length && "Invalid index!");
> return Data[Index];
> @@ -248,6 +262,7 @@ namespace llvm {
> /// @{
>
> /// Check if this string starts with the given \p Prefix.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> bool startswith(StringRef Prefix) const {
> return Length >= Prefix.Length &&
> @@ -255,16 +270,19 @@ namespace llvm {
> }
>
> /// Check if this string starts with the given \p Prefix, ignoring case.
> + LLVM_NODISCARD
> bool startswith_lower(StringRef Prefix) const;
>
> /// Check if this string ends with the given \p Suffix.
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> + LLVM_NODISCARD
> bool endswith(StringRef Suffix) const {
> return Length >= Suffix.Length &&
> compareMemory(end() - Suffix.Length, Suffix.Data, Suffix.Length) == 0;
> }
>
> /// Check if this string ends with the given \p Suffix, ignoring case.
> + LLVM_NODISCARD
> bool endswith_lower(StringRef Suffix) const;
>
> /// @}
> @@ -275,6 +293,7 @@ namespace llvm {
> ///
> /// \returns The index of the first occurrence of \p C, or npos if not
> /// found.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> size_t find(char C, size_t From = 0) const {
> size_t FindBegin = std::min(From, Length);
> @@ -290,6 +309,7 @@ namespace llvm {
> ///
> /// \returns The index of the first occurrence of \p C, or npos if not
> /// found.
> + LLVM_NODISCARD
> size_t find_lower(char C, size_t From = 0) const;
>
> /// Search for the first character satisfying the predicate \p F
> @@ -322,18 +342,21 @@ namespace llvm {
> ///
> /// \returns The index of the first occurrence of \p Str, or npos if not
> /// found.
> + LLVM_NODISCARD
> size_t find(StringRef Str, size_t From = 0) const;
>
> /// Search for the first string \p Str in the string, ignoring case.
> ///
> /// \returns The index of the first occurrence of \p Str, or npos if not
> /// found.
> + LLVM_NODISCARD
> size_t find_lower(StringRef Str, size_t From = 0) const;
>
> /// Search for the last character \p C in the string.
> ///
> /// \returns The index of the last occurrence of \p C, or npos if not
> /// found.
> + LLVM_NODISCARD
> size_t rfind(char C, size_t From = npos) const {
> From = std::min(From, Length);
> size_t i = From;
> @@ -349,22 +372,26 @@ namespace llvm {
> ///
> /// \returns The index of the last occurrence of \p C, or npos if not
> /// found.
> + LLVM_NODISCARD
> size_t rfind_lower(char C, size_t From = npos) const;
>
> /// Search for the last string \p Str in the string.
> ///
> /// \returns The index of the last occurrence of \p Str, or npos if not
> /// found.
> + LLVM_NODISCARD
> size_t rfind(StringRef Str) const;
>
> /// Search for the last string \p Str in the string, ignoring case.
> ///
> /// \returns The index of the last occurrence of \p Str, or npos if not
> /// found.
> + LLVM_NODISCARD
> size_t rfind_lower(StringRef Str) const;
>
> /// Find the first character in the string that is \p C, or npos if not
> /// found. Same as find.
> + LLVM_NODISCARD
> size_t find_first_of(char C, size_t From = 0) const {
> return find(C, From);
> }
> @@ -373,20 +400,24 @@ namespace llvm {
> /// not found.
> ///
> /// Complexity: O(size() + Chars.size())
> + LLVM_NODISCARD
> size_t find_first_of(StringRef Chars, size_t From = 0) const;
>
> /// Find the first character in the string that is not \p C or npos if not
> /// found.
> + LLVM_NODISCARD
> size_t find_first_not_of(char C, size_t From = 0) const;
>
> /// Find the first character in the string that is not in the string
> /// \p Chars, or npos if not found.
> ///
> /// Complexity: O(size() + Chars.size())
> + LLVM_NODISCARD
> size_t find_first_not_of(StringRef Chars, size_t From = 0) const;
>
> /// Find the last character in the string that is \p C, or npos if not
> /// found.
> + LLVM_NODISCARD
> size_t find_last_of(char C, size_t From = npos) const {
> return rfind(C, From);
> }
> @@ -395,30 +426,36 @@ namespace llvm {
> /// found.
> ///
> /// Complexity: O(size() + Chars.size())
> + LLVM_NODISCARD
> size_t find_last_of(StringRef Chars, size_t From = npos) const;
>
> /// Find the last character in the string that is not \p C, or npos if not
> /// found.
> + LLVM_NODISCARD
> size_t find_last_not_of(char C, size_t From = npos) const;
>
> /// Find the last character in the string that is not in \p Chars, or
> /// npos if not found.
> ///
> /// Complexity: O(size() + Chars.size())
> + LLVM_NODISCARD
> size_t find_last_not_of(StringRef Chars, size_t From = npos) const;
>
> /// Return true if the given string is a substring of *this, and false
> /// otherwise.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> bool contains(StringRef Other) const { return find(Other) != npos; }
>
> /// Return true if the given character is contained in *this, and false
> /// otherwise.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> bool contains(char C) const { return find_first_of(C) != npos; }
>
> /// Return true if the given string is a substring of *this, and false
> /// otherwise.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> bool contains_lower(StringRef Other) const {
> return find_lower(Other) != npos;
> @@ -426,6 +463,7 @@ namespace llvm {
>
> /// Return true if the given character is contained in *this, and false
> /// otherwise.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> bool contains_lower(char C) const { return find_lower(C) != npos; }
>
> @@ -434,6 +472,7 @@ namespace llvm {
> /// @{
>
> /// Return the number of occurrences of \p C in the string.
> + LLVM_NODISCARD
> size_t count(char C) const {
> size_t Count = 0;
> for (size_t i = 0, e = Length; i != e; ++i)
> @@ -453,6 +492,7 @@ namespace llvm {
> /// If the string is invalid or if only a subset of the string is valid,
> /// this returns true to signify the error. The string is considered
> /// erroneous if empty or if it overflows T.
> + LLVM_NODISCARD
> template <typename T>
> typename std::enable_if<std::numeric_limits<T>::is_signed, bool>::type
> getAsInteger(unsigned Radix, T &Result) const {
> @@ -464,6 +504,7 @@ namespace llvm {
> return false;
> }
>
> + LLVM_NODISCARD
> template <typename T>
> typename std::enable_if<!std::numeric_limits<T>::is_signed, bool>::type
> getAsInteger(unsigned Radix, T &Result) const {
> @@ -519,6 +560,7 @@ namespace llvm {
> ///
> /// APInt::fromString is superficially similar but assumes the
> /// string is well-formed in the given radix.
> + LLVM_NODISCARD
> bool getAsInteger(unsigned Radix, APInt &Result) const;
>
> /// @}
> @@ -526,9 +568,11 @@ namespace llvm {
> /// @{
>
> // Convert the given ASCII string to lowercase.
> + LLVM_NODISCARD
> std::string lower() const;
>
> /// Convert the given ASCII string to uppercase.
> + LLVM_NODISCARD
> std::string upper() const;
>
> /// @}
> @@ -544,6 +588,7 @@ namespace llvm {
> /// \param N The number of characters to included in the substring. If N
> /// exceeds the number of characters remaining in the string, the string
> /// suffix (starting with \p Start) will be returned.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> StringRef substr(size_t Start, size_t N = npos) const {
> Start = std::min(Start, Length);
> @@ -553,6 +598,7 @@ namespace llvm {
> /// Return a StringRef equal to 'this' but with only the first \p N
> /// elements remaining. If \p N is greater than the length of the
> /// string, the entire string is returned.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> StringRef take_front(size_t N = 1) const {
> if (N >= size())
> @@ -563,6 +609,7 @@ namespace llvm {
> /// Return a StringRef equal to 'this' but with only the first \p N
> /// elements remaining. If \p N is greater than the length of the
> /// string, the entire string is returned.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> StringRef take_back(size_t N = 1) const {
> if (N >= size())
> @@ -572,6 +619,7 @@ namespace llvm {
>
> /// Return the longest prefix of 'this' such that every character
> /// in the prefix satisfies the given predicate.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> StringRef take_while(function_ref<bool(char)> F) const {
> return substr(0, find_if_not(F));
> @@ -579,6 +627,7 @@ namespace llvm {
>
> /// Return the longest prefix of 'this' such that no character in
> /// the prefix satisfies the given predicate.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> StringRef take_until(function_ref<bool(char)> F) const {
> return substr(0, find_if(F));
> @@ -586,6 +635,7 @@ namespace llvm {
>
> /// Return a StringRef equal to 'this' but with the first \p N elements
> /// dropped.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> StringRef drop_front(size_t N = 1) const {
> assert(size() >= N && "Dropping more elements than exist");
> @@ -594,6 +644,7 @@ namespace llvm {
>
> /// Return a StringRef equal to 'this' but with the last \p N elements
> /// dropped.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> StringRef drop_back(size_t N = 1) const {
> assert(size() >= N && "Dropping more elements than exist");
> @@ -602,6 +653,7 @@ namespace llvm {
>
> /// Return a StringRef equal to 'this', but with all characters satisfying
> /// the given predicate dropped from the beginning of the string.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> StringRef drop_while(function_ref<bool(char)> F) const {
> return substr(find_if_not(F));
> @@ -609,6 +661,7 @@ namespace llvm {
>
> /// Return a StringRef equal to 'this', but with all characters not
> /// satisfying the given predicate dropped from the beginning of the string.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> StringRef drop_until(function_ref<bool(char)> F) const {
> return substr(find_if(F));
> @@ -649,6 +702,7 @@ namespace llvm {
> /// remaining in the string, the string suffix (starting with \p Start)
> /// will be returned. If this is less than \p Start, an empty string will
> /// be returned.
> + LLVM_NODISCARD
> LLVM_ATTRIBUTE_ALWAYS_INLINE
> StringRef slice(size_t Start, size_t End) const {
> Start = std::min(Start, Length);
> @@ -666,6 +720,7 @@ namespace llvm {
> ///
> /// \param Separator The character to split on.
> /// \returns The split substrings.
> + LLVM_NODISCARD
> std::pair<StringRef, StringRef> split(char Separator) const {
> size_t Idx = find(Separator);
> if (Idx == npos)
> @@ -683,6 +738,7 @@ namespace llvm {
> ///
> /// \param Separator - The string to split on.
> /// \return - The split substrings.
> + LLVM_NODISCARD
> std::pair<StringRef, StringRef> split(StringRef Separator) const {
> size_t Idx = find(Separator);
> if (Idx == npos)
> @@ -735,6 +791,7 @@ namespace llvm {
> ///
> /// \param Separator - The character to split on.
> /// \return - The split substrings.
> + LLVM_NODISCARD
> std::pair<StringRef, StringRef> rsplit(char Separator) const {
> size_t Idx = rfind(Separator);
> if (Idx == npos)
> @@ -744,36 +801,42 @@ namespace llvm {
>
> /// Return string with consecutive \p Char characters starting from the
> /// the left removed.
> + LLVM_NODISCARD
> StringRef ltrim(char Char) const {
> return drop_front(std::min(Length, find_first_not_of(Char)));
> }
>
> /// Return string with consecutive characters in \p Chars starting from
> /// the left removed.
> + LLVM_NODISCARD
> StringRef ltrim(StringRef Chars = " \t\n\v\f\r") const {
> return drop_front(std::min(Length, find_first_not_of(Chars)));
> }
>
> /// Return string with consecutive \p Char characters starting from the
> /// right removed.
> + LLVM_NODISCARD
> StringRef rtrim(char Char) const {
> return drop_back(Length - std::min(Length, find_last_not_of(Char) + 1));
> }
>
> /// Return string with consecutive characters in \p Chars starting from
> /// the right removed.
> + LLVM_NODISCARD
> StringRef rtrim(StringRef Chars = " \t\n\v\f\r") const {
> return drop_back(Length - std::min(Length, find_last_not_of(Chars) + 1));
> }
>
> /// Return string with consecutive \p Char characters starting from the
> /// left and right removed.
> + LLVM_NODISCARD
> StringRef trim(char Char) const {
> return ltrim(Char).rtrim(Char);
> }
>
> /// Return string with consecutive characters in \p Chars starting from
> /// the left and right removed.
> + LLVM_NODISCARD
> StringRef trim(StringRef Chars = " \t\n\v\f\r") const {
> return ltrim(Chars).rtrim(Chars);
> }
> @@ -817,6 +880,7 @@ namespace llvm {
> /// @}
>
> /// \brief Compute a hash_code for a StringRef.
> + LLVM_NODISCARD
> hash_code hash_value(StringRef S);
>
> // StringRefs can be treated like a POD type.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list