[PATCH] gdbsupport: mark array_view::slice with [[nodiscard]]

[PATCH] gdbsupport: mark array_view::slice with [[nodiscard]]

[Simon Marchi]

From: Simon Marchi <simon.marchi@efficios.com>

I (almost) had a bug where I did:

    buffer.slice (...)

but I meant:

    buffer = buffer.slice (...)

The first one does nothing, it creates a new array_view but without
using it, it's useless.  Mark the slice methods with [[nodiscard]]
(which is standard C++17) so that error would generate a warning.

I guess that many functions could be marked as nodiscard, essentially
function that is pure (doesn't have side-effects).  But this one seems
particularly easy to mis-use.

Change-Id: Ib39a0a65a5728a3cfd68a02ae31635810baeaccb
---
 gdbsupport/array-view.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdbsupport/array-view.h b/gdbsupport/array-view.h
index ee3a3c58710c..4b519112e78f 100644
--- a/gdbsupport/array-view.h
+++ b/gdbsupport/array-view.h
@@ -185,6 +185,7 @@ class array_view
   /* Slice an array view.  */
 
   /* Return a new array view over SIZE elements starting at START.  */
+  [[nodiscard]]
   constexpr array_view<T> slice (size_type start, size_type size) const noexcept
   {
 #if defined(_GLIBCXX_DEBUG) && __cplusplus >= 201402L
@@ -195,6 +196,7 @@ class array_view
 
   /* Return a new array view over all the elements after START,
      inclusive.  */
+  [[nodiscard]]
   constexpr array_view<T> slice (size_type start) const noexcept
   {
 #if defined(_GLIBCXX_DEBUG) && __cplusplus >= 201402L

base-commit: 268109cad16c692e24a583c21ef5a8ac58cc51fe
-- 
2.42.0

Re: [PATCH] gdbsupport: mark array_view::slice with [[nodiscard]]

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> The first one does nothing, it creates a new array_view but without
Simon> using it, it's useless.  Mark the slice methods with [[nodiscard]]
Simon> (which is standard C++17) so that error would generate a warning.

Looks good to me.

Unfortunately std::span::subspan doesn't seem to have this annotation.
I wonder if it could be added.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH] gdbsupport: mark array_view::slice with [[nodiscard]]

[Simon Marchi]

On 11/3/23 14:12, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> The first one does nothing, it creates a new array_view but without
> Simon> using it, it's useless.  Mark the slice methods with [[nodiscard]]
> Simon> (which is standard C++17) so that error would generate a warning.
> 
> Looks good to me.
> 
> Unfortunately std::span::subspan doesn't seem to have this annotation.
> I wonder if it could be added.

Is it something that would be mandated by the standard, or just the
choice of each implementation?

> 
> Approved-By: Tom Tromey <tom@tromey.com>

Thanks, pushed.

Simon

Re: [PATCH] gdbsupport: mark array_view::slice with [[nodiscard]]

[Tom Tromey]

>> Unfortunately std::span::subspan doesn't seem to have this annotation.
>> I wonder if it could be added.

Simon> Is it something that would be mandated by the standard, or just the
Simon> choice of each implementation?

I asked Jonathan Wakely on irc and he said he'll add it to libstdc++.

The standard does sometimes specify [[nodiscard]], but he said that he
has been campaigning against that, because implementations are already
free to add it anywhere (they can emit any warnings they like) and they
are also free to ignore any annotation.  So specifying it is pointless.

Tom

Re: [PATCH] gdbsupport: mark array_view::slice with [[nodiscard]]

[Simon Marchi]

On 11/3/23 14:33, Tom Tromey wrote:
>>> Unfortunately std::span::subspan doesn't seem to have this annotation.
>>> I wonder if it could be added.
> 
> Simon> Is it something that would be mandated by the standard, or just the
> Simon> choice of each implementation?
> 
> I asked Jonathan Wakely on irc and he said he'll add it to libstdc++.

Wow, thanks a lot for doing that.

> The standard does sometimes specify [[nodiscard]], but he said that he
> has been campaigning against that, because implementations are already
> free to add it anywhere (they can emit any warnings they like) and they
> are also free to ignore any annotation.  So specifying it is pointless.

I think that makes sense to not specify them, they don't have an impact
on how the program behave.

Simon