google / benchmark

A microbenchmark support library
Apache License 2.0
8.6k stars 1.57k forks source link

Set -Wno-unused-variable for tests #1682

Closed oontvoo closed 8 months ago

oontvoo commented 8 months ago

We used assert() a lot in tests and that can cause build breakages in some of the opt builds (since assert() are removed)

it's not practical to sprinkle "(void)" everywhere so I think setting this warning option is the best option for now.

dmah42 commented 8 months ago

do you know why we're not seeing these as issues in the CI bots? release mode would strip the asserts, and we have both -Wall and -Werror enabled so we should be getting these warnings/errors...

LebedevRI commented 8 months ago

Because -Wall is a very small subset of all the compiler-provided warnings. You'd want -Weverything-clean, with explicit opt-outs.

dmah42 commented 8 months ago

Because -Wall is a very small subset of all the compiler-provided warnings. You'd want -Weverything-clean, with explicit opt-outs.

ah yes, the nuanced distinction between "all" and "everything" (everywhere all at once, presumably).

LebedevRI commented 8 months ago

Is this missing the CMake part?

oontvoo commented 8 months ago

Is this missing the CMake part?

Good point - will send a followup PR.

Thanks both!

dmah42 commented 8 months ago

Is this missing the CMake part?

Good point - will send a followup PR.

Thanks both!

1683 i think does it.