<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:33, 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 4:58 PM, Argyrios Kyrtzidis <</span><a href="mailto:akyrtzi@gmail.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="">akyrtzi@gmail.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 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=""></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="">I've attached a patch that addresses the static_assert issue and</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="">updates some comments. As for the testing issues, those will have 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="">wait until MSVC 2013 support is dropped. The first issue is because</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="">MSVC 2013 does not have constexpr support for std::numeric_limits. 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="">second issue is because is_constructible returns true despite</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="">sizeof(intptr_t) == 4 and sizeof(long long) == 8. I don't have 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="">chance to look into the second one to see whether the test is</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="">incorrect, or whether MSVC's implementation of std::is_constructible</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="">is incorrect right now, but I think it's fine to leave the unit test</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="">out for MSVC currently.</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="">This patch is no good because it doesn't catch what the static_assert is actually checking for, which is</div><div class=""><br class=""></div><blockquote style="margin: 0 0 0 40px; border: none; padding: 0px;" class=""><div class="">enum Flags { A = 1, B = 2 };</div><div class="">Flags operator|(Flags lhs, Flags rhs) {</div><div class=""><span class="Apple-tab-span" style="white-space:pre"> </span>return static_cast<Flags>(lhs | rhs);</div><div class="">}</div><div class=""><br class=""></div><div class="">OptionSet<Flags> myFlags = A | B;</div></blockquote><br class=""><div class="">The idea is that <i class="">if</i> you use OptionSet<Flags>, operator| should always give you an OptionSet, not the raw type.</div><div class=""><br class=""></div><div class="">Jordan</div></body></html>