<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 12, 2016, at 14:42, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" class="">aaron@aaronballman.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Fri, Feb 12, 2016 at 5:36 PM, Jordan Rose <</span><a href="mailto:jordan_rose@apple.com" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">jordan_rose@apple.com</a><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">> wrote:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class="">On Feb 12, 2016, at 14:33, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" class="">aaron@aaronballman.com</a>> wrote:<br class=""><br class="">On Fri, Feb 12, 2016 at 4:58 PM, Argyrios Kyrtzidis <<a href="mailto:akyrtzi@gmail.com" class="">akyrtzi@gmail.com</a>><br class="">wrote:<br class=""><br class=""><br class="">On Feb 12, 2016, at 1:29 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" class="">aaron@aaronballman.com</a>> wrote:<br class=""><br class="">On Fri, Feb 12, 2016 at 12:40 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com" class="">jordan_rose@apple.com</a>> wrote:<br class=""><br class=""><br class="">On Feb 12, 2016, at 7:58 , Argyrios Kyrtzidis <<a href="mailto:akyrtzi@gmail.com" class="">akyrtzi@gmail.com</a>> wrote:<br class=""><br class="">On Feb 12, 2016, at 5:08 AM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com" class="">aaron@aaronballman.com</a>> wrote:<br class=""><br class="">On Thu, Feb 11, 2016 at 11:36 PM, Argyrios Kyrtzidis via llvm-commits<br class=""><<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""><br class="">Author: akirtzidis<br class="">Date: Thu Feb 11 22:36:48 2016<br class="">New Revision: 260654<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=260654&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=260654&view=rev</a><br class="">Log:<br class="">[ADT] OptionSet: ifdef out some code that seems to be crashing MSVC.<br class=""><br class="">Modified:<br class="">llvm/trunk/include/llvm/ADT/OptionSet.h<br class=""><br class="">Modified: llvm/trunk/include/llvm/ADT/OptionSet.h<br class="">URL:<br class=""><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/OptionSet.h?rev=260654&r1=260653&r2=260654&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/OptionSet.h?rev=260654&r1=260653&r2=260654&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/include/llvm/ADT/OptionSet.h (original)<br class="">+++ llvm/trunk/include/llvm/ADT/OptionSet.h Thu Feb 11 22:36:48 2016<br class="">@@ -116,6 +116,8 @@ public:<br class="">}<br class=""><br class="">private:<br class="">+#ifndef _MSC_VER<br class="">+ // This is crashing MSVC.<br class="">template <typename T><br class="">static auto _checkResultTypeOperatorOr(T t) -> decltype(t | t) { return<br class="">T(); }<br class=""><br class="">@@ -124,6 +126,7 @@ private:<br class="">static_assert(!std::is_same<decltype(_checkResultTypeOperatorOr(Flags())),<br class=""> Flags>::value,<br class=""> "operator| should produce an OptionSet");<br class="">+#endif<br class="">};<br class=""><br class=""><br class="">Are there plans to correct this for MSVC and recommit? Is there a<br class="">requirement for this to use automatic type deduction instead of using<br class="">the decltype in the is_same check, like this:<br class=""><br class="">http://coliru.stacked-crooked.com/a/94f224987d9feaee<br class=""><br class=""><br class="">+ Jordan who added that.<br class=""><br class=""><br class="">That seems perfectly fine to me—actually, it seems like an improvement. I<br class="">don't know why I did it the way I did.<br class=""><br class="">Actually, this would be an even better improvement:<br class=""><br class="">static_assert(std::is_same<decltype(Flags() | Flags()),<br class=""> OptionSet<Flags>>::value,<br class=""> "operator| should produce an OptionSet");<br class=""><br class="">…but I'm not sure if that will break any of our existing use cases!<br class=""><br class=""><br class="">It turns out that it was written that way because it had to be.<br class="">OptionSetTest.cpp has a test case using enum class, for which operator<br class="">| does not compile. Without SFINAE, there's no way to test that.<br class=""><br class="">I am able to reproduce the compiler ICE in MSVC 2013; I will see if<br class="">there's a way I can correct this so we can enable the check on Visual<br class="">Studio.<br class=""><br class=""><br class="">Thanks Aaron!<br class=""><br class="">FYI, there were also 2 other issues on MSVC:<br class=""><br class="">Compiler error in unit test:<br class="">http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/9720<br class="">Unit test failure:<br class="">http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/9721<br class=""><br class=""><br class="">I've attached a patch that addresses the static_assert issue and<br class="">updates some comments. As for the testing issues, those will have to<br class="">wait until MSVC 2013 support is dropped. The first issue is because<br class="">MSVC 2013 does not have constexpr support for std::numeric_limits. The<br class="">second issue is because is_constructible returns true despite<br class="">sizeof(intptr_t) == 4 and sizeof(long long) == 8. I don't have the<br class="">chance to look into the second one to see whether the test is<br class="">incorrect, or whether MSVC's implementation of std::is_constructible<br class="">is incorrect right now, but I think it's fine to leave the unit test<br class="">out for MSVC currently.<br class=""><br class=""><br class="">This patch is no good because it doesn't catch what the static_assert is<br class="">actually checking for, which is<br class=""><br class="">enum Flags { A = 1, B = 2 };<br class="">Flags operator|(Flags lhs, Flags rhs) {<br class="">return static_cast<Flags>(lhs | rhs);<br class="">}<br class=""><br class="">OptionSet<Flags> myFlags = A | B;<br class=""><br class=""><br class="">The idea is that if you use OptionSet<Flags>, operator| should always give<br class="">you an OptionSet, not the raw type.<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Good catch! It's strange that this does not cause any test failures,</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">however. Unfortunately, I don't have any more time I can dedicate to</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">this right now, but it gives Argyrios an idea of how to modify the</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">code to compile for MSVC 2013 without causing an ICE. I'll let them</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">take care of actually correcting it properly -- I am happy to test any</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">proposed patches. Another thread is recommending that this entire file</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">be reverted and go through the review process, so this is something</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">that we can catch at that time. Given that this is exercising</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">questionable corners of MSVC, we definitely should not have this</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">static_assert commented out if we can at all avoid it.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote></div><br class=""><div class="">At the time I couldn't figure out how to test it, since it's a static_assert. It would probably need yet <i class="">another</i> level of SFINAE.</div><div class=""><br class=""></div><div class="">Jordan</div></body></html>