[FR]: `EXPECT_THROW`, `EXPECT_NO_THROW`, `ASSERT_THROW`, and `ASSERT_NO_THROW` don't play nice with `[[nodiscard]]`

[Jacobfaib]

Does the feature exist in the most recent commit?

Tested on v1.13.0, but the same problem also exists at head:

https://github.com/google/googletest/blob/79219e26e0e36b415a5804b6b017ad6c6cd99ad8/googletest/include/gtest/internal/gtest-internal.h#L1367-L1373

where GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_() expands as

https://github.com/google/googletest/blob/79219e26e0e36b415a5804b6b017ad6c6cd99ad8/googletest/include/gtest/internal/gtest-internal.h#L1312-L1316

Why do we need this feature?

I can work around it by doing

[[nodiscard]] int a_function_that_throws();

ASSERT_THROW(static_cast<void>(a_function_that_throws()), ...)

but I feel like gtest should render this nicety for me.

Describe the proposal.

Wrap statement in static_cast<void>(statement) or std::ignore = statement. The former is probably more general.

Is the feature specific to an operating system, compiler, or build system version?

No.

[dkaszews]

I for one would rather ASSERT_NO_THROW not hide the [[nodiscard]].

The statement inside the macro can also be assignment, so you can do:

ASSERT_NO_THROW(std::ignore = a_function_that_throws());

// or
int result{};
ASSERT_NO_THROW(result = a_function_that_throws());
EXPECT_EQ(result, 1);

[Jacobfaib]

@dkaszews Fair, I an argument could be made for *_NO_THROW not to gobble the value -- I suspect it will come down to personal preference :). The main issue really is the *_THROW family, I've updated the original post to focus more on that.

In the throwing case, the return value is truly irrelevant, so gtest should handle that gracefully for us.

[dkaszews]

@Jacobfaib Sure, EXPECT_THROW effectively means ECPECT_DOES_NOT_RETURN. I would be fine with gtest silencing the [[nodiscard]] in that case.

Question is, are throwing functions with [[nodiscard]] really that common? I find it a weird combination, because I have always only seen [[nodiscard]] used either with pure nothrow functions, or functions which return error codes and therefore also never throw.

The only case I can thing of is something like std::string::substr which should be marked as [[nodiscard]] (but isn't in standard) to indicate it returns a new object instead of modifying the old one, while could still throw on out of bounds index.

[Jacobfaib]

Question is, are throwing functions with [[nodiscard]] really that common?

Very common, at least in my experience. We use [[nodiscard]] liberally in our code bases, since if the function returns something then 9/10 the user should be capturing that return value (or explicitly choose to ignore it). Whether a function returns something or throws exceptions are orthogonal concepts.

For example, you can have a factory function that validates the inputs before constructing the widget. This is both [[nodiscard]] and throwing.