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

Fri Oct 2 13:29:54 PDT 2020

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

> (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).
>

By broken I meant are there places that wouldn't compile with Clang 3.5
because they rely on a non-copyable but move-only type being moved into a
return when there's a type mismatch/necessary implicit conversion?

The rest - yeah, I'm sure there are some missed optimization opportunities
where we could add std::move for the older compiler. I don't feel super
strongly about that - if someone cares about perf on older compilers they
can go optimize the code further, etc.

>
>
> 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/dae3d156/attachment-0001.html>