[llvm-dev] [EXTERNAL] Re: preferred way to return expected values

Fri Oct 2 13:10:01 PDT 2020

> Is the code currently broken?
There are quite a few places which rely on this optimization (e.g. a large
vector to be moved rather than copied) but now he would silently get a copy
instead (with older compilers).

On Fri, Oct 2, 2020 at 12:51 PM David Blaikie <dblaikie at gmail.com> wrote:

> Yeah, not sure either.
>
> The discussion about minimum compatibility usually comes down to what
> version is available on long-term OS releases.
>
> On Fri, Oct 2, 2020 at 12:38 PM George Rimar <grimar at accesssoftek.com>
> wrote:
>
>> > Is the code currently broken?
>>
>>
>> I don't know. Perhaps the code is fine, I did not try to compile LLVM
>> with all that compilers
>>
>> that we claim as supported though.  I just *guess* that we don't have a
>> bot that builds LLVM with Clang 3.5,
>>
>> for example. It sounds like a bit too old version to me to use (released
>> in the middle of 2014, AFAIK).
>>
>>
>> Best regards,
>> George | Developer | Access Softek, Inc
>> ------------------------------
>> *От:* David Blaikie <dblaikie at gmail.com>
>> *Отправлено:* 2 октября 2020 г. 22:04
>> *Кому:* George Rimar
>> *Копия:* Alexander Shaposhnikov; Richard Smith; llvm-dev; Lang Hames;
>> James Henderson; avl.lapshin at gmail.com
>> *Тема:* Re: [EXTERNAL] Re: [llvm-dev] preferred way to return expected
>> values
>>
>>
>>
>> On Fri, Oct 2, 2020 at 1:48 AM George Rimar <grimar at accesssoftek.com>
>> wrote:
>>
>>> Thanks, David!
>>>
>>>
>>> Few minor additions to the topic:
>>>
>>>
>>> > I'm not sure which MSVC version on godbolt would be "MSVC 2017" that
>>> the LLVM docs refer to
>>>
>>>
>>> I've found that the minimal available version of MSVC on
>>> godbolt is "WINE MSVC 2015: x64 msvc v19.0 (WINE)".
>>>
>>> Your sample compiles fine with it: https://godbolt.org/z/hsPneK
>>>
>>>
>>> Also, I've tried with "x64 msvc v19.10 (WINE)" (
>>> https://godbolt.org/z/vaqsPY)
>>>
>>> wiki says that v19.10 corresponds to Visual Studio 2017 version 15.0 (
>>> https://en.wikipedia.org/wiki/Microsoft_Visual_C%2B%2B),
>>>
>>> i.e. it is MSVS 2017 without updates. The sample also feels fine.
>>>
>>>
>>> > Looks like we would need to bump the minimum Clang up from 3.5 to at
>>> least 3.9 to allow returns with implicit moves that include conversions.
>>>
>>>
>>> Perhaps, we could have a bot to check that LLVMs codebase is compilable with
>>> compilers we claim to support.
>>>
>> Generally good to do, yes, but someone's got to pay for/setup the
>> resources, etc.
>>
>>> I am not sure it is not a overkill though, but could help either to keep
>>> the documentation up to date or fix the code to match it.
>>>
>> Is the code currently broken?
>>
>> - Dave
>>
>>
>>>
>>> Best regards,
>>> George | Developer | Access Softek, Inc
>>> ------------------------------
>>> *От:* David Blaikie <dblaikie at gmail.com>
>>> *Отправлено:* 1 октября 2020 г. 22:00
>>> *Кому:* George Rimar
>>> *Копия:* Alexander Shaposhnikov; Richard Smith; llvm-dev; Lang Hames;
>>> James Henderson; avl.lapshin at gmail.com
>>> *Тема:* Re: [EXTERNAL] Re: [llvm-dev] preferred way to return expected
>>> values
>>>
>>>
>>>
>>> On Thu, Oct 1, 2020 at 2:08 AM George Rimar <grimar at accesssoftek.com>
>>> wrote:
>>>
>>>> FWIW, I've performed an experiment with the code below at godbolt.
>>>> (used -O2, https://godbolt.org/z/nY95nh)
>>>>
>>>> ```
>>>> #include <vector>
>>>> #include "llvm/Support/Error.h"
>>>>
>>>> llvm::Expected<std::vector<int>> foo() {
>>>>  std::vector<int> V;
>>>>  V.push_back(0);
>>>>  return V;
>>>> }
>>>> ```
>>>>
>>>
>>> I think the easiest and portable way to test this functionality would be
>>> more like:
>>>
>>> #include <memory>
>>> struct base { virtual ~base(); };
>>> struct derived : base { };
>>> std::unique_ptr<base> f() {
>>>   std::unique_ptr<derived> d;
>>>   return d;
>>> }
>>>
>>> That shows whether the compiler's treating the return of a temporary as
>>> movable, even when the types aren't an exact match.
>>>
>>> Clang 3.5 does not support this conversion: https://godbolt.org/z/5nsWM8
>>> GCC 5.1 does support it: https://godbolt.org/z/cvd3d6
>>> & I'm not sure which MSVC version on godbolt would be "MSVC 2017" that
>>> the LLVM docs refer to.
>>>
>>>
>>>>
>>>> If I understand the produced output correctly, then results are:
>>>>
>>>> gcc 7.5: creates a copy.
>>>> gcc 8.1: uses move.
>>>>
>>>> clang < 6.0: doesn't compile.
>>>>
>>>
>>> That's interesting - I wonder if LLVM's documentation is out of date,
>>> which claims the minimum required Clang is 3.5:
>>> https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library
>>>
>>>
>>>> clang >= 6.0: uses move.
>>>>
>>>> MSVS: was unable to compile, complains about "llvm/Support/Error.h"
>>>> header.
>>>> I am using MSVS 2017 locally and it calls move constructor of
>>>> Expected<> though,
>>>> so I think all MSVS >= 2017 (at least) should be fine.
>>>>
>>>
>>> May be something to do with which compiler the llvm library provided by
>>> godbolt is compiled with? which might make the above results not quite
>>> right (& why testing with the non-llvm-specific example might be clearer)
>>>
>>> Looks like we would need to bump the minimum Clang up from 3.5 to at
>>> least 3.9 to allow returns with implicit moves that include conversions.
>>> - Dave
>>>
>>>>
>>>>
>>>> Best regards,
>>>> George | Developer | Access Softek, Inc
>>>> ------------------------------
>>>> *От:* Alexander Shaposhnikov <alexander.v.shaposhnikov at gmail.com>
>>>> *Отправлено:* 28 сентября 2020 г. 22:46
>>>> *Кому:* David Blaikie
>>>> *Копия:* Richard Smith; llvm-dev; Lang Hames; George Rimar; James
>>>> Henderson; avl.lapshin at gmail.com
>>>> *Тема:* [EXTERNAL] Re: [llvm-dev] preferred way to return expected
>>>> values
>>>>
>>>> CAUTION: This email originated from outside of the organization. Do
>>>> not click links or open attachments unless you recognize the sender and
>>>> know the content is safe.  If you suspect potential phishing or spam email,
>>>> report it to ReportSpam at accesssoftek.com
>>>> Many thanks for the reply,
>>>> right, this is what the discussion is about.
>>>>
>>>>
>>>> On Mon, Sep 28, 2020 at 10:57 AM David Blaikie <dblaikie at gmail.com>
>>>> wrote:
>>>>
>>>>> To clarify, this is a discussion around whether given some move-only
>>>>> type X, implicitly convertible to Y and the code "Y func() { X x; return x;
>>>>> }" is that valid in LLVM? (and, as a corollary, if the type isn't
>>>>> move-only, is that code efficient (does it move rather than copying) - as
>>>>> in the vector example given)
>>>>>
>>>>> I /believe/ the answer is that it is not valid. I think the set of
>>>>> compilers supported includes those that do not implement this rule. (either
>>>>> due to the language version we compile with, or due to it being a DR that
>>>>> some supported compiler versions do not implement). But that's just my
>>>>> rough guess.
>>>>>
>>>>> On Sat, Sep 26, 2020 at 3:17 PM Alexander Shaposhnikov via llvm-dev <
>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>
>>>>>> Hello everyone!
>>>>>> It looks like in the LLVM codebase (including subprojects) there are
>>>>>> some inconsistencies
>>>>>> in how values are returned from functions with the following (or
>>>>>> similar) signature:
>>>>>>     Expected<std::vector<int>> createVector() {
>>>>>>         std::vector<int> V;
>>>>>>         ...
>>>>>>     }
>>>>>> It would be interesting to find out your opinion on this.
>>>>>> After some investigation I have found https://reviews.llvm.org/D70963
>>>>>> and https://reviews.llvm.org/D43322 which have some additional
>>>>>> context regarding
>>>>>> the problem. The aforementioned diffs (and the comments on them)
>>>>>> contain a lot of
>>>>>> details and the history of the problem (whether one should use the
>>>>>> cast or not).
>>>>>> If I am not mistaken a part of the problem is that compilers'
>>>>>> behaviors have changed over time, and e.g. the latest versions would use a
>>>>>> move constructor while the older ones could use a copy constructor. So the
>>>>>> question is where we stand at the moment / what is the recommended approach
>>>>>> for the new code.
>>>>>>
>>>>>> Many thanks in advance,
>>>>>> Alexander Shaposhnikov
>>>>>>
>>>>>> _______________________________________________
>>>>>> LLVM Developers mailing list
>>>>>> llvm-dev at lists.llvm.org
>>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201002/cc83e0f5/attachment.html>