13

C++17 has a new attribute, [[nodiscard]].

Suppose, that I have a Result struct, which has this attribute:

struct [[nodiscard]] Result {
};

Now, if I call a function which returns Result, I got a warning if I don't check the returned Result:

Result someFunction();

int main() {
    someFunction(); // warning here, as I don't check someFunction's return value
}

This program generates:

warning: ignoring return value of function declared with 'nodiscard' attribute [-Wunused-result]

So far, so good. Now suppose, that I have a special function, for which I still want to return Result, but I don't want this warning generated, if the check is omitted:

Result someNonCriticalFunction();

int main() {
    someNonCriticalFunction(); // I don't want to generate a warning here
}

It is because, someNonCriticalFunction() does something non-critical (for example, something like printf - I bet that no-one checks printf's return value all the time); most cases, I don't care if it fails. But I still want it to return Result, as in some rare cases, I do need its Result.

Is it possible to do this somehow?


Possible solutions which I don't like:

  • I would not like calling it as (void)someNonCriticalFunction(), because this function is called a lot of times, it is awkward
  • creating a wrapper around someNonCriticalFunction(), which calls (void)someNonCriticalFunction(): I don't want to have a differently named function just because of this
  • removing [[nodiscard]] from Result, and add it to every function which returns Result
Alex Guteniev
  • 12,039
  • 2
  • 34
  • 79
geza
  • 28,403
  • 6
  • 61
  • 135
  • add another struct unimportant_result ? – xyious Nov 03 '17 at 21:02
  • 9
    It seems like you have contradictory requirements. You want to add a warning so you never ignore a result, but you want to ignore it ? – xyious Nov 03 '17 at 21:03
  • 5
    If it's a *special* case then it should really not happen much, and using a cast to `void` is acceptable (with a comment telling why). And if it does happen often then it's no longer *special* and I recommend you take some time to think about your design and requirements. – Some programmer dude Nov 03 '17 at 21:04
  • 1
    Rust does this by returning a `Result<()>` (a result type that returns an empty struct). You then just call `.unwrap()` to ignore the error. Maybe you could add an `ignore` function to your `Result` class. – Justin Nov 03 '17 at 21:05
  • 5
    You've ruled out all reasonable solutions. – Benjamin Lindley Nov 03 '17 at 21:05
  • @xyious: It's an exception. I want warnings generated always, except for this (and possible other) functions. Returned errors are almost always good to handle. But, for example, for printf, they can be ignored most of the time. – geza Nov 03 '17 at 21:06
  • @Someprogrammerdude: suppose that `printf` or `cout` returns `Result`. Would you cast their result to void all the time? – geza Nov 03 '17 at 21:07
  • But the functions you mentions aren't marked as `[[nodiscard]]`. Their results *can* be ignored. It's part of their design. Either you're not using the attribute in the way it's supposed to, or you're requirements are really contradicting in which case there is no nice solution (and no, I don't consider the solution in Barrys answer "nice" in that way, even though it is a "nice hack"). Perhaps you should mark the ***functions*** as `[[nodiscard]]` instead of the structure? – Some programmer dude Nov 03 '17 at 21:09
  • @Someprogrammerdude: suppose, that you have `Result` for any OS error. All OS functions returns this, instead of `int`. And you mark `Result` as `[[nodiscard]]`, because for 99.5% of the functions, it must be checked. And and exception is, for example, `printf`. `printf` returns `Result` too, but you don't want it generate a warning. – geza Nov 03 '17 at 21:12
  • @BenjaminLindley: maybe. If C++ had the opposite of `[[nodiscard]]`: `[[discard]]`, I could mark `someNonCriticalFunction()` with it. – geza Nov 03 '17 at 21:13
  • You're still thinking wrong IMO. If you have 39 functions that all need to have their results checked, and one that doesn't, but the single function that doesn't need to be checked is called 1000 to 1 compared to the others, is the return-type really the right place to use that attribute? Or the functions? While the single function could be seen as a special case in the declaration list, it's not a special case when it comes to the calls. – Some programmer dude Nov 03 '17 at 21:15
  • @Someprogrammerdude: I have several hundreds of functions which returns Result. I could mark them, of course. But it would be more convenient to mark `Result`, and mark the 3 exceptions I have. – geza Nov 03 '17 at 21:17
  • 1
    Also, it also increases *maintianability* and *readability* and *discoverability* to have `[[nodiscard]]` for the functions IMO. And it does make it *much* easier to handle "special cases". – Some programmer dude Nov 03 '17 at 21:18
  • @Someprogrammerdude: I disagree. I have `Result`'s all over the place. I don't want to put `[[nodiscard]]` everywhere, and it worsens readability. And maybe I forget to put `[[nodiscard]]` somewhere. It's much easier to mark 4 things (and then I can forget about it), than hundreds of them, and always remembering to put it, when I write a function which returns `Result`. – geza Nov 03 '17 at 21:22
  • 1
    What if you do a pragma push diagnostics and ignore the warning on the function signature.. then pop?? Would that work? IMO, I don't think `[[nodiscard]]` should be applied to entire classes. Rather it should be applied to things like `new`, `malloc`, `calloc`, etc.. Raw allocations where discarding the result would be a bad idea.. but then again, this is just an opinion.. – Brandon Nov 03 '17 at 21:40
  • How can we trust our own code knowing that `[[nodiscard]]` variables are sometimes discarded? Why rely on `[[nodiscard]]` if our goal is to sometimes discard? If there is truly a pragmatic reason to discard a result in few, specific, well-defined circumstances, I believe Johann Studanski's answer, below, is the most idiomatic, `grep`-able, and transparent: `std::ignore = function()`. – Escualo Nov 05 '21 at 15:43

5 Answers5

32

Why not make use of std::ignore from the <tuple> header—that would make the discard explicit:

[[nodiscard]] int MyFunction() { return 42; }

int main() 
{
    std::ignore = MyFunction();
    return 0;
}
Johann Studanski
  • 1,023
  • 12
  • 19
  • 3
    I really like this solution. It's quick to write, and obvious what it is doing. Plus, it's easy to search for when you want to fix the problem properly! – Jonathan O'Connor Nov 05 '21 at 12:28
  • 3
    The OP said he didn't want to use `(void)MyFunction()` because it's called a lot and it's "awkward", but prefixing all calls with `std::ignore =` is even more extra syntax than `(void)`, and adds a `` dependency. I think he's just got to suck it up. The Result struct is being annoying and sometimes that's life. – Billy Donahue Nov 06 '21 at 17:55
  • 4
    @BillyDonahue Of course OP is going to have to suck it up. Given that, they might remember the need to write expressive and clearly intentional code. `(void)MyFunction();` does **not** express intent and may be overlooked for its effect during a review. `std::ignore = MyFunction();` doesn't suffer from those. – sehe Nov 07 '21 at 23:27
  • 2
    @sehe `(void)` is C-compatible, well-known and idiomatic, and more generic-friendly. It can be applied to expressions that might already be `void` expressions. `std::ignore` has a tuple-related purpose and the intent is more, not less, obscure when it is used for a purpose for which it wasn't designed. – Billy Donahue Nov 08 '21 at 22:46
  • 4
    So are raw pointers, error codes, and a host of idioms that have been made extinct. If you’re goig to argue that English words aren’t expressive, go right ahead, but I’m saying your reaching. Std::ignore has the benefit of expressing intent. It’s almost funny because “to ignore” is a word that also has a purpose. I’d say you can use it with the intent to convey its meaning. – sehe Nov 08 '21 at 22:57
  • I'm not arguing that "English words aren't expressive", but I appreciate your attempt to deflect. The C++ compiler doesn't understand English. `std::ignore` is not an English word, it's a C++ identifier. It does not mean the same thing as the English verb "ignore", so using it as if it does is to mislead the reader and the compiler. – Billy Donahue May 31 '23 at 15:15
7

They say that every problem in computer science can be solved by adding another layer of indirection:

template <bool nodiscard=true>
struct Result;

template <>
struct Result<false> {
    // the actual implementation
};

template <>
struct [[nodiscard]] Result<true>
    : Result<false>
{
    using Result<false>::Result;
};

This is effectively making Result conditionally [[nodiscard]], which allows:

Result<true> someFunction();
Result<false> someNonCriticalFunction();

int main() {
    someFunction();            // warning here
    someNonCriticalFunction(); // no warning here
}

Although really, this is identical to:

  • removing [[nodiscard]] from Result, and add it to every function which returns Result

which gets my vote to begin with.

Barry
  • 286,269
  • 29
  • 621
  • 977
  • 2
    Now he's got to go to all the functions where he returns `Result`, and change them to `Result`. But if he's going to do that, he could just as well add `[[nodiscard]]` to the function (which is really what he should do, except that he's arbitrarily ruled that solution out). – Benjamin Lindley Nov 03 '17 at 21:13
  • @BenjaminLindley Oh yeah, I agree with that. See my edit, that I typed up immediately after posting and then never hit okay on :-) – Barry Nov 03 '17 at 21:36
7

I recommend the option you ruled out:

"removing [[nodiscard]] from Result, and add it to every function which returns Result."

But since you don't seem happy with it, here's another solution, using bog-standard inheritance:

struct [[nodiscard]] Result {
};

struct DiscardableResult: public Result {
};

For the functions where you can discard the result, use DiscardableResult as return type:

Result func1();
DiscardableResult func2();

func1(); // will warn
func2(); // will not warn
Nikos C.
  • 50,738
  • 9
  • 71
  • 96
  • I've just asked this, because it seems that `[[nodiscard]]` is sticky, so DiscardableResult gets the `[[nodiscard]]` attribute (I've checked with gcc). But this could be a good solution, if `[[nodiscard]]` would not be inherited. https://stackoverflow.com/questions/47104799/does-attribute-specifier-sequence-inherit – geza Nov 03 '17 at 21:43
  • @geza It warns with older GCC (6.x and older.) Clang and GCC 7.x do not warn. Unfortunately, you need a language lawyer to answer the question of whether `[[nodiscard]]` should be inherited. I don't think it should, but I'm not 100% sure. – Nikos C. Nov 03 '17 at 21:46
  • Thanks, then maybe this is the solution I'm searching for, if the standard says that attributes doesn't inherit. – geza Nov 03 '17 at 21:50
  • 1
    Btw, why do you recommend adding `[[nodiscard]]` for every function? To me, `[[nodiscard]]` belongs to `Result`, as it always stores an error condition, it always must be checked. I have several hundreds of functions which return `Result`. Marking all of them, and maintaining them afterwards is tedious. Marking `Result` is so much simpler. – geza Nov 03 '17 at 21:58
  • 4
    @geza: "*Btw, why do you recommend adding [[nodiscard]] for every function?*" If it is ever reasonable to discard the type, then the type should not be `[[nodiscard]]`. That's what it means to apply `[[nodiscard]]` to the type: that the very nature of the type itself means you should *never* discard it. – Nicol Bolas Nov 03 '17 at 22:02
  • 3
    @geza: *"as it always stores an error condition, it always must be checked"* -- Your question contradicts that. – Benjamin Lindley Nov 03 '17 at 22:03
  • @BenjaminLindley: Maybe I was ambiguous. By error condition, I mean "OK" too. And on "always" I mean almost always, except for some special functions. – geza Nov 03 '17 at 22:04
  • 4
    @geza: You're asking how to not have to check something that, supposedly, *"must always be checked"*. That's a contradiction. – Benjamin Lindley Nov 03 '17 at 22:08
  • @geza I recommend it because IMO a type does not carry any information of whether all uses of it should be discardable or not. This really depends on the context the type is used in. Which is why you ran into this issue in the first place. Some functions are OK with discarding. I'm not saying your approach is wrong though, mind you! I'm just saying that personally I'd recommend fine-grained use of `[[nodiscard]]` per function. – Nikos C. Nov 03 '17 at 22:10
  • @BenjaminLindley: Always, except when `printf` generates it. It is an exception. Really, if you have a general rule, and an exception, you would mark every general case, instead of marking the exception? – geza Nov 03 '17 at 22:11
  • @NikosC.: yes, usually that's the case, but I think my `Result` is an exception. I always need to check it, no matter where it came from, it is inherently its function. The only exception is printf. – geza Nov 03 '17 at 22:17
  • @RustyX: thanks, yes, that's an option. But then I need to add a conversion from `DiscardableResult` to `Result`, and maybe this generates some unforeseen problems. I'll try it out, if it turns out that `[[nodiscard]]` is sticky. – geza Nov 03 '17 at 22:20
  • Based on Nicol's [answer](https://stackoverflow.com/a/47105294/8157187), `[[nodiscard]]` doesn't inherit, so your answer does exactly what I needed, thanks! – geza Nov 03 '17 at 22:52
1

You can suppress the warning with another C++17 attribute, namely [[maybe_unused]]

[[nodiscard]] int MyFunction() { return 42; }

int main()
{
    [[maybe_unused]] auto v = MyFunction();
    return 0;
}

This way you also avoid the confusing dependency to std::tuple which comes with std::ignore, even CppCoreGuidelines is openly recommending to use std::ignore for ignoring [[nodiscard]] values:

Never cast to (void) to ignore a [[nodiscard]]return value. If you deliberately want to discard such a result, first think hard about whether that is really a good idea (there is usually a good reason the author of the function or of the return type used [[nodiscard]] in the first place). If you still think it's appropriate and your code reviewer agrees, use std::ignore = to turn off the warning which is simple, portable, and easy to grep.

Looking at C++ reference, officially std::ignore is only specified to be used in std::tie when unpacking tuples.

While the behavior of std::ignore outside of std::tie is not formally specified, some code guides recommend using std::ignore to avoid warnings from unused return values of [[nodiscard]] functions.

talamaki
  • 5,324
  • 1
  • 27
  • 40
0

cast the result to a (void *).

int main()
{
    (void *)someFunction(); //Warning will be gone.
}

This way you "used" your result as far as the compiler is concerned. Great for when you are using a library where nodiscard has been used and you really don't care to know the result.

  • 1
    Nice , but you may have missed this comment from the Original Poster : I would not like calling it as (void)someNonCriticalFunction(), because this function is called a lot of times, it is awkward – Gonen I Jul 28 '21 at 16:20
  • `/s/void */void/` for the proper idiom. – Thomas Eding Nov 08 '21 at 13:12