Hello,

On 07/10/2023 22:10, Julien Jorge via Std-Proposals wrote:
> 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.

It's not about a compiler emitting such warnings on any discarded
expession¹, it's about an implementation of e.g. pthreads that works on
multiple OSes, and on some of those some functions may fail so they're
marked [[nodiscard]]. But on your OS, or maybe on a recent kernel, it
doesn't fail any more; still the implementation keeps the [[nodiscard]]
around for the other cases. It's a 3rd party, so you can't go in there
and remove the tag.

¹ this would open a Pandora box, because not all discarded statements
are equal, and a compiler can't really know better than the developer.
Clang-tidy has now a warning that uses a bunch of heuristics to suggest
to add [[nodiscard]], but that's what they are, heuristics.



> 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.

This would be a matter of enforcing it as a coding guideline; not by
means of making it illegal in C++ itself. There are already coding
guidelines that are pretty aggressive in this regard.



> 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 :)

Then don't put a message, and enforce a rule where [[discard]] must have
an accompanying comment. This is just way less automatable via tooling,
that's all. I still believe in the value of code reviews...


>
> 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?

For instance generic code calling one of the functions that I describe
as OK to discard, such as calling functions that you know cannot fail
given certain inputs (and those functions are passed as template
parameters of your generic function).

Also: this answer crosses with Arthur's observation and Edward's
question. Right now, if you cast to void to actually get a void type out
of an expression (for whatever reason), you're also suppressing the
associated discarding warning. What if that warning was really
important? Wouldn't it have been better to have two different syntaxes?


> 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 :)

Fair enough, I'll add it :)

>
> 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.

But why should we "just" accept this abuse of notation? Why not asking
for a better notation?

For example, the very same notation (cast to void) has also been abused
to indicate that an automatic variable could be unused in a given
function. Now we have [[maybe_unused]] to better convey that meaning,
removing the need for this particular abuse. I'm trying to do the same
for discarding.


> 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.

And so isn't all of C++, and I'll be sure that in 20 years from now I'll
still have interfaces based on pointer+length because somebody didn't
get the memo; so why bother to add span?


> This is one thing added for zero
> removed :(

This is a logical fallacy; getting anything removed from C++ is close to
impossible, anything is going to be an addition of some sorts.


> 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.

"It's not much, but it's a start." C++11 gave us the attribute syntax
and at the same time added basically absolutely no "useful" standard
attributes.

--
Thank you everyone (and also to those who replied offlist) for your 
feedback, I've evolved the paper over the weekend (including a different 
syntax approach) and gave it a P number now. 
https://isocpp.org/files/papers/P2992R0.html
-- 
Giuseppe D'Angelo