<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, May 5, 2014 at 4:59 PM, Marshall Clow <span dir="ltr"><<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><div>On May 5, 2014, at 1:09 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div>
<br><blockquote type="cite"><div style="font-family:LucidaGrande;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, May 2, 2014 at 1:00 AM, Alp Toker<span> </span><span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span><span> </span>wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>On 02/05/2014 07:35, Richard Smith wrote:<div><div><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">On Thu, May 1, 2014 at 10:36 PM, Alp Toker <<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a><span> </span><mailto:<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>>> wrote:<br>
<br><br> <span> </span>On 02/05/2014 05:34, Richard Smith wrote:<br><br> <span> </span>IIRC there were concerns that __has_feature might not be the<br> <span> </span>right place to be surfacing these traits.<br>
<br><br> <span> </span>Right, I took some notes back when I was looking into this and my<br> <span> </span>findings were:<br><br> <span> </span>__has_feature() is intended for standardised language features and<br> <span> </span>it's prominently user-facing so not really appropriate for trait<br>
<span> </span>primitives whether or not they're feature-complete. Once an<br> <span> </span>alternative is found we should phase those out.<br><br> <span> </span>__has_extension() also suffers from being user-facing, but perhaps<br>
<span> </span>less so.<br><br> <span> </span>__has_builtin() would let us automatically list all built-in trait<br> <span> </span>primitives and I did some work to support that but it wasn't clear<br> <span> </span>what value this adds so I've put that aside.<br>
<br> <span> </span>Observation: There's no need to list "compatibility-only" or<br> <span> </span>poorly defined trait primitives in any detection macro because<br> <span> </span>they only exist for compatibility with MSVC and GCC which use<br>
<span> </span>without checking, with the possible exception of Embarcadero(?).<br><br> <span> </span>As you see this is a bit of a tangle. As a way forward the viable<br> <span> </span>options to provide a feature test for _quality_ type trait<br>
<span> </span>primitives could be one of:<br><br> <span> </span>a) A new __has_primitive()<br> <span> </span>b) Shoehorning into __has_extension() but naming the features more<br> <span> </span>clearly as primitives: e.g.<br>
<span> </span>__has_extension(primitive_is_<u></u>constructible)<br><br><br>Another option is recommending people use __is_identifier, which already works for this purpose (because we treat these tokens as keywords), but I'd be against that one since we can't mark a trait as unavailable when it's in a not-fully-working state.<br>
<br>One advantage of sticking with __has_feature or __has_extension is that we don't immediately gain a legacy deprecated interface to support and be unhappy about =) Here's what our documentation says:<br><br>"For type trait __X, __has_extension(X) indicates the presence of the type trait primitive in the compiler."<br>
<br>We could stick with that, using the is_ prefix as our indicator that the extension is a type trait (and adding these and any further type traits only to __has_extension and not to __has_feature).<br></blockquote><br></div>
</div>Yes, I think __has_extension() is the way to go.<br><br>I wonder however if the feature names need a clearer prefix given that the C++ standard library is considered part of the implementation.<br><br>The existing names makes it sound to users as though this makes sense:<br>
<br>#if __has_feature(is_<u></u>constructible) || __has_extension(is_<u></u>constructible)<br> <span> </span>std::is_constructible<T>::<u></u>value<br>#else<br> <span> </span>...<br>#endif</blockquote><div><br></div>
<div>I don't feel particularly uncomfortable about this. If users are using __has_feature without reading the documentation (maybe because they're cargo-culting something?), I think we're justified in saying they're doing it wrong. I'd prefer to keep the names the same (but to only document the __has_extension checks going forward).</div>
<div><br></div><div>Marshall: libc++ has a lot of __has_feature checks. Is there a good reason you're not using __has_extension that you know of? (__has_extension always returns true for a superset of the cases where __has_feature does.)</div>
</div></div></div></div></blockquote><div><br></div></div></div>Probably historical reasons; but I was under the impression that __has_feature was the shiny, cross-compiler way to tell if a compiler had implemented some feature. [ People use libc++ on other compilers than clang ]</div>
</div></blockquote><div><br></div><div>__has_feature is not cross-compiler, it's a Clang extension just like __has_extension. The shiny cross-compiler way is the SG10 __cpp_* macros (but it's murky whether they should map to __has_feature or __has_extension, and they're not widely supported yet, and they don't yet cover all the things you want to test, so they're probably not exactly what you want here.)</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>A quick search turned up no uses of <span style="font-family:LucidaGrande">__has_extension in libc++</span></div>
<div><br></div><div><font face="LucidaGrande">[ Apologies for belaboring the obvious, but ..]</font></div><div><font face="LucidaGrande">For libc++ to use a compiler intrinsic, I have to know if it’s available - so I need a test, and </font></div>
<div><font face="LucidaGrande">I’d like a method that works for different compilers.</font></div></div></blockquote><div><br></div><div>If that's the case, then your current __has_feature checks are not doing what you want.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><span class="HOEnZb"><font color="#888888"><div>— Marshall</div></font></span><div>
<div class="h5"><div><br></div><div><br></div><div><br><blockquote type="cite"><div style="font-family:LucidaGrande;font-size:11px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> <span> </span>Some of the traits don't work in general (and support just<br>
<span> </span>enough to get through the MS headers); I don't recall exactly<br> <span> </span>which ones, though, and I think it was more than just<br> <span> </span>__is_*_destructible. Alp, do you recall? (Maybe it was the<br>
<span> </span>__is_nothrow_* ones?)<br><br><br> <span> </span>Many of the _is_ ones are sketchy. I cooked up some tests to<br> <span> </span>compare them against GCC and while compatibility was good, overall<br>
<span> </span>usefulness wasn't clear and some of the results were sketchy. I'll<br> <span> </span>see if I can dig this up but the results may be lost (wish we had<br> <span> </span>a wiki to throw stuff like this).<br>
<br> <span> </span>Meanwhile our tests for the trait primitives have inconsistent<br> <span> </span>coverage and fixing them would be a lot of work. Fortunately the<br> <span> </span>libc++ test suite has great coverage and it might be possible to<br>
<span> </span>reuse that work..<br><br> <span> </span>Marshall, do you think a strategically placed static_assert()<br> <span> </span>could be added somewhere in your type trait tests to compare<br> <span> </span>answers from the clang builtin type trait primitives against your<br>
<span> </span>quality library implementations?<br><br><br><br> <span> </span>Do we advertise support for the __has_* ones? We should stop<br> <span> </span>doing that if we do -- they're fundamentally broken and pretty<br>
<span> </span>much useless (they exist to support libstdc++ and give wrong<br> <span> </span>answers in some cases for GCC compatibility). Eventually I<br> <span> </span>would ilke to make them synonyms for the correct traits, but<br>
<span> </span>I'm concerned that people might be relying on the GCC bugs<br> <span> </span>that we emulate for them.<br><br><br> <span> </span>My understanding of the Embarcadero usage is that they _DO_ use<br>
<span> </span>the feature detection macros. If this is indeed the case (they<br> <span> </span>should comment, do we have a contact?) then removing them from<br> <span> </span>__has_feature() is tantamount to removing them completely. I'm not<br>
<span> </span>at all opposed to that but it should be done after we get the<br> <span> </span>facts from the original authors.<br><br><br>I'm not sure whether we're talking about the same traits. Just to make sure, I mean __has_trivial_* and __has_nothrow_* (which GCC added a few years ago based on the then-C++0x type traits proposal, which got reworked substantially before C++11).<br>
</blockquote><br></div></div>Oh right. The __has ones have issues like PR16627 and ignoring members with default arguments. For the __is ones, there are various quirks and FIXMEs in SemaExprCXX.cpp which need going through. The newly added primitives not marked incomplete are good for general usage though. We should probably only expose feature checks for the non-quirky ones going on a case by case basis.<br>
<br>Alp.<br><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><br>I don't have any particular views on the __is_lvalue_expr, __is_rvalue_expr, __is_same, __is_convertible, __array_* Embarcadero traits (except that the names array_rank and array_extent are icky :-) ).<br>
<br> <span> </span>Alp.<br><br><br><br><br><br> <span> </span>On Thu, May 1, 2014 at 7:48 AM, Marshall Clow<br> <span> </span><<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a><span> </span><mailto:<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>><br>
</div> <span> </span><mailto:<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a><span> </span><mailto:<a href="mailto:mclow.lists@gmail.com" target="_blank">mclow.lists@gmail.com</a>><u></u>>><div>
<div><br> <span> </span>wrote:<br><br> <span> </span>Recently, reading<br> <span> </span>llvm/tools/clang/docs/<u></u>LanguageExtensions.rst, I<br> <span> </span>saw a list of type_trait primitives<br>
<span> </span>supported by clang.<br><br> <span> </span>One of them caught my eye: __is_nothrow_constructible — that<br> <span> </span>could be handy for use in libc++.<br><br> <span> </span>So I ran a quick test:<br>
<br> <span> </span>__is_nothrow_constructible(<u></u>int)<br> <span> </span>and it returned true.<br><br> <span> </span>Then I tried:<br> <span> </span>__has_feature(is_nothrow_<u></u>constructible)<br>
<span> </span>and it returned false.<br><br> <span> </span>Well, that’s not useful to me; I need to be able to tell<br> <span> </span>when I<br> <span> </span>can use that feature.<br>
<span> </span>So I whipped up a program to test all of the type_traits<br> <span> </span>listed in<br> <span> </span>that file (attached),<br> <span> </span>and to see if I can check (using __has_feature) whether or not<br>
<span> </span>they are implemented.<br><br> <span> </span>Turns out that __has_feature returns false for the<br> <span> </span>following type<br> <span> </span>traits:<br><br> <span> </span>__has_feature(is_interface_<u></u>class) = 0<br>
<span> </span>__has_feature(is_destructible) = 0<br> <span> </span>__has_feature(is_nothrow_<u></u>destructible) = 0<br> <span> </span>__has_feature(is_nothrow_<u></u>assignable) = 0<br>
<span> </span>__has_feature(is_nothrow_<u></u>constructible) = 0<br><br> <span> </span>__is_interface_class is a MS extension; so that’s fine.<br> <span> </span>__is_destructible and __is_nothrow_destructible are<br>
<span> </span>described as<br> <span> </span>“partially implemented”, and attempts to use them fail.<br><br> <span> </span>On the other hand:<br> <span> </span>__is_nothrow_constructible(<u></u>int) returns 1<br>
<span> </span>__is_nothrow_assignable(int&, int) returns 1<br><br> <span> </span>So these two seem to work (for at least one case)<br> <span> </span>It seems to me that we should have feature tests for these<br>
<span> </span>type<br> <span> </span>traits if we expect people to use them<br><br> <span> </span>So, I’d like to see<br> <span> </span>__has_feature(is_nothrow_<u></u>constructible)<br>
<span> </span>and __has_feature(is_nothrow_<u></u>assignable)<br><br> <span> </span>return 1<br><br><br> <span> </span>— Marshall<br><br><br><br><br> <span> </span>-- <span> </span><a href="http://www.nuanti.com/" target="_blank">http://www.nuanti.com</a><br>
<span> </span>the browser experts<br><br><br></div></div></blockquote><div><div><br>--<span> </span><br><a href="http://www.nuanti.com/" target="_blank">http://www.nuanti.com</a><br>the browser experts</div></div></blockquote>
</div></div></div></div></blockquote></div><br></div></div></div></blockquote></div><br></div></div>