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

Fri Oct 2 13:12:25 PDT 2020

(typo)
There are quite a few places which rely on this optimization (e.g. a large
vector to be moved rather than copied) but now those places would silently
create a copy instead (with older compilers).

On Fri, Oct 2, 2020 at 1:10 PM Alexander Shaposhnikov <
alexander.v.shaposhnikov at gmail.com> wrote:

> > 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/209b8d12/attachment.html>