From 92f8d681472ac5ff8f3323388fd05f78e70cb9c2 Mon Sep 17 00:00:00 2001 From: Jakub Kuderski Date: Sat, 9 Nov 2024 11:16:57 -0500 Subject: [PATCH 1/2] [ADT] Mark reverse and concat as nodiscard It may not be immediately obvious if these two functions modify the given ranges or return a view over them. We have seen downstream code that mistakenly assumed the given range would be mutated. Add the `[[nodiscard]]` attribute to prevent these errors. Also clarify the lack of mutation in the documentation comments. --- llvm/include/llvm/ADT/STLExtras.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h index 43c9b80edff78..4f516ccf4cd6f 100644 --- a/llvm/include/llvm/ADT/STLExtras.h +++ b/llvm/include/llvm/ADT/STLExtras.h @@ -416,7 +416,8 @@ static constexpr bool HasFreeFunctionRBegin = } // namespace detail // Returns an iterator_range over the given container which iterates in reverse. -template auto reverse(ContainerTy &&C) { +// Does not mutate the containers. +template [[nodiscard]] auto reverse(ContainerTy &&C) { if constexpr (detail::HasFreeFunctionRBegin) return make_range(adl_rbegin(C), adl_rend(C)); else @@ -1182,11 +1183,13 @@ template class concat_range { } // end namespace detail -/// Concatenated range across two or more ranges. +/// Returns a concatenated range across two or more ranges. Does not modify the +/// ranges. /// /// The desired value type must be explicitly specified. template -detail::concat_range concat(RangeTs &&... Ranges) { +[[nodiscard]] detail::concat_range +concat(RangeTs &&...Ranges) { static_assert(sizeof...(RangeTs) > 1, "Need more than one range to concatenate!"); return detail::concat_range( From c52bcdbf8bc123a859e3dd3eaba07fbaebe5263d Mon Sep 17 00:00:00 2001 From: Jakub Kuderski Date: Sat, 9 Nov 2024 11:23:20 -0500 Subject: [PATCH 2/2] Fix typo --- llvm/include/llvm/ADT/STLExtras.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h index 4f516ccf4cd6f..ace5f60b572d7 100644 --- a/llvm/include/llvm/ADT/STLExtras.h +++ b/llvm/include/llvm/ADT/STLExtras.h @@ -416,7 +416,7 @@ static constexpr bool HasFreeFunctionRBegin = } // namespace detail // Returns an iterator_range over the given container which iterates in reverse. -// Does not mutate the containers. +// Does not mutate the container. template [[nodiscard]] auto reverse(ContainerTy &&C) { if constexpr (detail::HasFreeFunctionRBegin) return make_range(adl_rbegin(C), adl_rend(C));