<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 29, 2016 at 4:34 PM, Stephan T. Lavavej via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">STL_MSFT marked 2 inline comments as done.<br>
STL_MSFT added a comment.<br>
<br>
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),</blockquote><div><br></div><div>Unused-variable seems pretty low value.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> but instead I'm building the tests with /<a href="http://reviews.llvm.org/W4" rel="noreferrer" target="_blank">http://reviews.llvm.org/W4</a> (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).</blockquote><div><br></div><div>*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)<br><br>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).<br><br>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.<br><br>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)<br><br>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.<br><br>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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.<br>
<span class=""><br>
<br>
================<br>
Comment at: test/std/numerics/rand/rand.device/eval.pass.cpp:33<br>
@@ -32,3 +32,3 @@<br>
     }<br>
-    catch (const std::system_error& e)<br>
+    catch (const std::system_error&)<br>
     {<br>
----------------<br>
</span><span class="">EricWF wrote:<br>
> This looks OK, but I noticed that *technically* the spec only says the exception type is derived from `std::exception`.<br>
><br>
> However if we can keep testing this exception without bothering anybody I say we do.<br>
</span>This one's problematic for MSVC for other reasons, but I have no issue with system_error here, so I haven't changed it.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D19625" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19625</a><br>
<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>