[PATCH] D19625: [libc++] Void-cast runtime-unused variables.

[David Blaikie via cfe-commits]

On Fri, Apr 29, 2016 at 4:34 PM, Stephan T. Lavavej via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> STL_MSFT marked 2 inline comments as done.
> STL_MSFT added a comment.
>
> What I'm doing is running libcxx's tests against MSVC's compiler and
> libraries. I could aggressively suppress warnings (and indeed I'm doing
> that for the noisiest, lowest-value warnings),


Unused-variable seems pretty low value.


> but instead I'm building the tests with /http://reviews.llvm.org/W4 (our
> highest supported level). It is indeed somewhat annoying to make tests
> warning-free, although it does find real issues occasionally (e.g. the
> broken assert that I reported first).


*nod* just a question of whether the work is worth it - which is mostly
totally up to you (& you've decided it's worth it for you)

My main concern is that this bar will be hard to maintain/will likely rot
over time, which reduces the value of establishing it (especially when
doing so involves adding code like the void casts in many places).

The somewhat aggressive stance I've tried to take in the past (to varying
degrees of success) is that if something can be caught by Clang then let's
make sure Clang is catching it and cleanup the codebase of any violations.
If it's not a diagnostic we'd add to Clang (because it doesn't meet the low
false positive and other quality bars) then we should just disable it
because it's not worth enforcing.

Do you need to run the test cases with warnings enabled to catch them in
the STL with MSVC? Or is it sufficient to just build a file that includes
each header? (I guess not - you need to instantiate different templates in
different ways, make sure members get instantiated, etc)

Anyway - I've no really strong opinion here, just rambling a bit in case
any of it happens to apply here - it does sound like this case might just
be sufficiently quirky as to not fit into the existing ways of approaching
diagnostic suppression/disabling/fixing in the project.

- Dave


> The real value is verifying that the headers are warning-free, which has
> the potential to catch issues in the product code. This is possibly a
> difference in philosophy between VC's STL and libc++, because I believe
> libc++ uses the "system header" behavior to avoid all warnings from system
> headers.
>
>
> ================
> Comment at: test/std/numerics/rand/rand.device/eval.pass.cpp:33
> @@ -32,3 +32,3 @@
>      }
> -    catch (const std::system_error& e)
> +    catch (const std::system_error&)
>      {
> ----------------
> EricWF wrote:
> > This looks OK, but I noticed that *technically* the spec only says the
> exception type is derived from `std::exception`.
> >
> > However if we can keep testing this exception without bothering anybody
> I say we do.
> This one's problematic for MSVC for other reasons, but I have no issue
> with system_error here, so I haven't changed it.
>
>
> http://reviews.llvm.org/D19625
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160429/fcf124c2/attachment.html>