[llvm] [ADT] Mark reverse and concat as nodiscard (PR #115611)
Jakub Kuderski via llvm-commits
llvm-commits at lists.llvm.org
Sat Nov 9 08:22:19 PST 2024
https://github.com/kuhar created https://github.com/llvm/llvm-project/pull/115611
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.
>From 92f8d681472ac5ff8f3323388fd05f78e70cb9c2 Mon Sep 17 00:00:00 2001
From: Jakub Kuderski <jakub at nod-labs.com>
Date: Sat, 9 Nov 2024 11:16:57 -0500
Subject: [PATCH] [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 43c9b80edff78e..4f516ccf4cd6f1 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 <typename ContainerTy> auto reverse(ContainerTy &&C) {
+// Does not mutate the containers.
+template <typename ContainerTy> [[nodiscard]] auto reverse(ContainerTy &&C) {
if constexpr (detail::HasFreeFunctionRBegin<ContainerTy>)
return make_range(adl_rbegin(C), adl_rend(C));
else
@@ -1182,11 +1183,13 @@ template <typename ValueT, typename... RangeTs> 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 <typename ValueT, typename... RangeTs>
-detail::concat_range<ValueT, RangeTs...> concat(RangeTs &&... Ranges) {
+[[nodiscard]] detail::concat_range<ValueT, RangeTs...>
+concat(RangeTs &&...Ranges) {
static_assert(sizeof...(RangeTs) > 1,
"Need more than one range to concatenate!");
return detail::concat_range<ValueT, RangeTs...>(
More information about the llvm-commits
mailing list