Re: [[nodiscard]] and KDE coding policy

Hello everyone,


On 7/31/26 2:53 AM, Jin Liu wrote:
> Vlad Zahorodnii <[email protected]> 于2026年7月31日周五 04:22写道：
>> That being said, I still don't think that adding [[nodiscard]] to
>> functions such as getters is a good idea. It makes code more verbose,
>> with more boilerplate, with little (imho) benefit. Stray getter calls
>> are odd but the effort to avoid them is hard to justify.
> Yeah, I don't think getters need `[[nodiscard]]`, for the same reason that
> `std::min` and `std::move` don't have it:
> 1. It's very unlikely that one would ignore the return value.
> 2. Even when mistakenly ignored, there's no effect.

1. We're writing lots of code for things that aren't likely, it's a huge 
part of why we have compiler warnings, certain CI jobs, etc. at all. 
Yes, it's not likely that you're forgetting to actually do something 
with the getter call, but it does happen

2. Well, no: There is not going to be an evil side-effect from the 
invocation of the getter call, but you're also not doing the thing you 
wanted to do, which could lead to all sorts of problems. This is also 
not a theoretical problem, we have recently shipped a (small) bug 
because of such a mistake[1]. This is what led me to suggest adding 
[[nodiscard]] pretty much anywhere as long as the returned value isn't 
really just an optional thing.


So yes, I do think we should add it to getters as well, it's a useful 
sanity check. That doesn't mean we have to immediately add it to all 
existing code or worry too much about forgetting it somewhere. But it's 
a good thing to aim for.


Cheers,

Tobias


>
> So maybe we could restrict the use to:
> A) Pure functions that (due to unfortunate naming) might be mistakenly
> assumed to be mutating. E.g., `container.empty()`.
> B) Mutating functions that might be mistakenly assumed to have no
> return value. And the return value really needs to be checked.
>
> I think this is similar to the Qt rule quoted earlier (except its
> "MAY" clause 2).
>
> -Jin


[1] https://invent.kde.org/plasma/plasma-nm/-/merge_requests/597