Date: Sat, 7 Oct 2023 22:10:18 +0200
Hi, some additional feedback below :)
1. The "Motivation and Scope" part is quite clear on describing use
cases for [[nodiscard]] and why we may want to discard the result
nonetheless. Yet I am not sure that the following paragraph illustrates
the thing you are trying to illustrate:
> Legacy. A function may return a status code to indicate success or
failure; then, later, its implementation evolves in a way such that it
never fails (example: pthread_once on Linux). Still, the function may
decide to keep returning the status code to preserve API and ABI
compatibility. The return value can always be safely discarded as it’s
meaningless
Do you know a compiler that would emit a warning for all unused function
results? As far as I can tell pthread_once may evolve to a never-failing
function with no impact on client code, and if it had a [[nodiscard]]
attribute then this attribute can simply be removed; it won't break the
API nor the ABI.
2. Later you write:
> having a reason can also be enforced by tooling (for instance, code
checkers could ban the usage of [[discard]] without a reason).
I would not insist on any kind of advantage of having the rationale
embedded in the attribute. Actually I think there is no way that
[[discard]] (or any other attribute) without a reason could be banned.
Forcing developers to write the rationale will only cause badly written
or empty rationales (see e.g. pre-17 static_assert). Moreover I would
argue that a well described reason written in a comment is superior to a
reason packed in a string, itself in an attribute, itself in a statement :)
3. You compare the many syntaxes to discard a value with respect to
generic code. I have some doubts on the necessity of discarding function
results in generic code. If the author of a function deemed that the
result should not be discarded, what kind of generic code could pretend
to know better and discard the result anyway?
4. Regarding std::ignore, you list the following shortcoming:
> It is relatively verbose compared to the cast to void.
It is indeed verbose compared to the cast, but [[discard]] is as much
verbose! I think you should avoid pointing shortcomings of existing
solutions that also apply to the proposed attribute :)
5. My overall opinion is that this proposal is solving a problem we
don't have. The cast to void may be an abuse of notation but it is
understandable at least as an idiom. IMHO the advantages listed in the
proposals are a bit weak, especially in regards of the cost of
introducing this attribute. Just having yet-another-way-to-do-something
means yet another thing to explain to fellow coders, because the cast to
void is not going anywhere soon. This is one thing added for zero
removed :( Moreover the whole discussion about attributes on expressions
makes it worse: this is a huge change for a feature that already exists
in another form. In my opinion it would need a more powerful attribute
to justify such a change.
Best regards
Julien
1. The "Motivation and Scope" part is quite clear on describing use
cases for [[nodiscard]] and why we may want to discard the result
nonetheless. Yet I am not sure that the following paragraph illustrates
the thing you are trying to illustrate:
> Legacy. A function may return a status code to indicate success or
failure; then, later, its implementation evolves in a way such that it
never fails (example: pthread_once on Linux). Still, the function may
decide to keep returning the status code to preserve API and ABI
compatibility. The return value can always be safely discarded as it’s
meaningless
Do you know a compiler that would emit a warning for all unused function
results? As far as I can tell pthread_once may evolve to a never-failing
function with no impact on client code, and if it had a [[nodiscard]]
attribute then this attribute can simply be removed; it won't break the
API nor the ABI.
2. Later you write:
> having a reason can also be enforced by tooling (for instance, code
checkers could ban the usage of [[discard]] without a reason).
I would not insist on any kind of advantage of having the rationale
embedded in the attribute. Actually I think there is no way that
[[discard]] (or any other attribute) without a reason could be banned.
Forcing developers to write the rationale will only cause badly written
or empty rationales (see e.g. pre-17 static_assert). Moreover I would
argue that a well described reason written in a comment is superior to a
reason packed in a string, itself in an attribute, itself in a statement :)
3. You compare the many syntaxes to discard a value with respect to
generic code. I have some doubts on the necessity of discarding function
results in generic code. If the author of a function deemed that the
result should not be discarded, what kind of generic code could pretend
to know better and discard the result anyway?
4. Regarding std::ignore, you list the following shortcoming:
> It is relatively verbose compared to the cast to void.
It is indeed verbose compared to the cast, but [[discard]] is as much
verbose! I think you should avoid pointing shortcomings of existing
solutions that also apply to the proposed attribute :)
5. My overall opinion is that this proposal is solving a problem we
don't have. The cast to void may be an abuse of notation but it is
understandable at least as an idiom. IMHO the advantages listed in the
proposals are a bit weak, especially in regards of the cost of
introducing this attribute. Just having yet-another-way-to-do-something
means yet another thing to explain to fellow coders, because the cast to
void is not going anywhere soon. This is one thing added for zero
removed :( Moreover the whole discussion about attributes on expressions
makes it worse: this is a huge change for a feature that already exists
in another form. In my opinion it would need a more powerful attribute
to justify such a change.
Best regards
Julien
Received on 2023-10-07 20:10:22