gdbsupport: mark array_view::slice with [[nodiscard]]
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
Commit Message
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(+)
base-commit: 268109cad16c692e24a583c21ef5a8ac58cc51fe
Comments
>>>>> "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
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
>> 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
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
@@ -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