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

Fri Oct 2 12:51:27 PDT 2020

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