Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
On Sat, Jan 27, 2018 at 4:55 PM, Glen Fernandes  wrote:
> I just tried boost-outcome unit tests with clang 4.0, 5.0 on Linux
> with libstdc++ with -std=c++14 and -std=c++1z which fails with many
> errors:
>
> e.g.
[snip]

> In file included from
> libs/outcome/test/tests/../../include/boost/outcome/detail/result_storage.hpp:34:
> libs/outcome/test/tests/../../include/boost/outcome/detail/../success_failure.hpp:54:42:
> error: no template named 'conditional_t' in namespace 'std'; did you
> mean 'conditional'?
> template <class T> using devoid =
> std::conditional_t<std::is_void<T>::value, void_type, T>;
> ~~~~~^~~~~~~~~~~~~
> conditional
> /usr/bin/../lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/type_traits:1780:12:
> note: 'conditional' declared here
> struct conditional
> ^
>
>
> Then I also tried the boost-outcome unit tests with Apple clang 8.1.0
> with libc++ with -std=c++14 and -std=c++1z which fails with errors
> like:
>

I pasted the wrong error output here; the right error output is:

clang-darwin.compile.c++
bin.v2/libs/outcome/test/core-outcome.test/clang-darwin-4.2.1/debug/cxxstd-14-iso/tests/core-outcome.o
In file included from libs/outcome/test/tests/core-outcome.cpp:30:
In file included from
libs/outcome/test/tests/../../include/boost/outcome/outcome.hpp:34:
In file included from
libs/outcome/test/tests/../../include/boost/outcome/detail/outcome_exception_observers.hpp:34:
In file included from
libs/outcome/test/tests/../../include/boost/outcome/detail/result_storage.hpp:35:
libs/outcome/test/tests/../../include/boost/outcome/detail/value_storage.hpp:181:22:
error: no matching constructor for initialization of 'value_type' (aka
'boost::outcome_v2::outcome<int, std::__1::error_code,
std::exception_ptr,
boost::outcome_v2::policy::error_code_throw_as_system_error<int,
std::__1::error_code, std::exception_ptr> >')
new(&_value) value_type; // NOLINT
^
libs/outcome/test/tests/../../include/boost/outcome/detail/result_storage.hpp:270:11:
note: in instantiation of member function
'boost::outcome_v2::detail::value_storage_nontrivial<boost::outcome_v2::outcome<int,
std::__1::error_code, std::exception_ptr,
boost::outcome_v2::policy::error_code_throw_as_system_error<int,
std::__1::error_code, std::exception_ptr> >
>::value_storage_nontrivial' requested here
: _state(std::move(o._state))
^

> Is this also a known issue? Only gcc 6 and gcc 7 with lisbtdc++ on
> Linux seem to pass. The other combinations I tried (clang + lisbtdc++
> on Linux,   Apple clang + libc++) do not.
>

I spoke too soon, here gcc 7 does not pass. It ICEs:

libs/outcome/test/tests/../../include/boost/outcome/detail/result_value_observers.hpp:45:17:
in constexpr expansion of
‘((boost::outcome_v2::detail::result_value_observers<boost::outcome_v2::detail::result_storage<int,
std::errc, boost::outcome_v2::policy::error_code_throw_as_system_error<int,
std::errc, void> >, int,
boost::outcome_v2::policy::error_code_throw_as_system_error<int,
std::errc, void>
>*)this)->boost::outcome_v2::detail::result_value_observers<boost::outcome_v2::detail::result_storage<int,
std::errc, boost::outcome_v2::policy::error_code_throw_as_system_error<int,
std::errc, void> >, int,
boost::outcome_v2::policy::error_code_throw_as_system_error<int,
std::errc, void>
>::<anonymous>.boost::outcome_v2::detail::result_storage<int,
std::errc, boost::outcome_v2::policy::error_code_throw_as_system_error<int,
std::errc, void>
>::result_storage<int>(boost::outcome_v2::in_place_type_t<int>(),
<anonymous>)’
libs/outcome/test/tests/core-result.cpp:330:41: internal compiler
error: in adjust_temp_type, at cp/constexpr.c:1206
constexpr result<int, std::errc> a(5), b(std::errc::invalid_argument);
[...]
0x6db788 cxx_eval_constant_expression
../../src/gcc/cp/constexpr.c:4034
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <file:///usr/share/doc/gcc-7/README.Bugs> for instructions.

Let me know if you need more context, but it appears boost-outcome is
also broken on gcc 7.2.

Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
I wrote a .travis.yml for your boost-outcome submission to run your
unit tests against a few compilers:

The output is at: https://travis-ci.org/glenfe/ned14.outcome/builds/334222400

I updated it to make sure the clang++-libstdc++ uses a more recent libstdc++.

It looks like:
- clang++-libc++ fails because of forgetting to #include a header.
- GCC 7.2 with -std=c++17 is the one with the Internal Compiler Errors
(I didn't investigate further)
- clang++5-libstdc++ fails because it cannot find a type in std

Is there a reason why you don't have travis and appveyor for
boost-outcome? I remember your "Best Practices" document strongly
advocating that every Boost library submission should have them.

Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 27/01/2018 22:06, Glen Fernandes wrote:

> On Sat, Jan 27, 2018 at 4:55 PM, Glen Fernandes  wrote:
>> I just tried boost-outcome unit tests with clang 4.0, 5.0 on Linux
>> with libstdc++ with -std=c++14 and -std=c++1z which fails with many
>> errors:
>>
>> e.g.
> [snip]
>> In file included from
>> libs/outcome/test/tests/../../include/boost/outcome/detail/result_storage.hpp:34:
>> libs/outcome/test/tests/../../include/boost/outcome/detail/../success_failure.hpp:54:42:
>> error: no template named 'conditional_t' in namespace 'std'; did you
>> mean 'conditional'?
>> template <class T> using devoid =
>> std::conditional_t<std::is_void<T>::value, void_type, T>;
>> ~~~~~^~~~~~~~~~~~~
>> conditional
>> /usr/bin/../lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/type_traits:1780:12:
>> note: 'conditional' declared here
>> struct conditional

That's an ancient version of libstdc++ 4.8 which does not provide
sufficient C++ 14 support.

As the documentation says at
https://ned14.github.io/outcome/requirements/, you need GCC 6.3 or later.

Thus libstdc++ would also need to be 6.3 or newer.

>> Then I also tried the boost-outcome unit tests with Apple clang 8.1.0
>> with libc++ with -std=c++14 and -std=c++1z which fails with errors
>> like:

Again, as https://ned14.github.io/outcome/requirements/ says, XCode 9 or
newer is required. The implementation of C++ 14 in XCode 8 is insufficient.

>> Is this also a known issue? Only gcc 6 and gcc 7 with lisbtdc++ on
>> Linux seem to pass. The other combinations I tried (clang + lisbtdc++
>> on Linux,   Apple clang + libc++) do not.
>>
>
> I spoke too soon, here gcc 7 does not pass. It ICEs:
> [snip]
> Let me know if you need more context, but it appears boost-outcome is
> also broken on gcc 7.2.

There's a long standing memory corruption bug in GCC's constexpr
implementation which Outcome's unit tests may trigger sometimes.

The GCC maintainers know well of this bug, and efforts continue to track
it down and fix it. The problem is that it is not repeatable across
machines. I may get consistently an ICE here on my machine (I do!), but
Travis running the exact same compiler and test suite does not (also
true!). Same problem when say Jason Merrill runs a repro I send him, and
it works perfectly on his machine, yet ICEs on mine.

If you're using Outcome heavily in 100% constexpr programming, then you
are advised to use clang or - and I kid you not here - MSVC until GCC
gets its constexpr implementation fixed.

Again, none of this is Outcome's fault. These are bugs in compilers. I
would also add that in day to day programming, GCC's constexpr problems
don't really manifest. They appear only when heavily stressed in say a
test suite which runs lots of corner case scenarios that would never
happen in real life.

Equally, if when programming with Outcome and you see GCC ICEing in
constexpr, either remove the constexpr or use a different compiler.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 27/01/2018 23:22, Glen Fernandes via Boost wrote:
> I wrote a .travis.yml for your boost-outcome submission to run your
> unit tests against a few compilers:
>
> The output is at: https://travis-ci.org/glenfe/ned14.outcome/builds/334222400
>
> I updated it to make sure the clang++-libstdc++ uses a more recent libstdc++.
>
> It looks like:
> - clang++-libc++ fails because of forgetting to #include a header.

Yep, that's a bug. Fixed in develop branch. Thanks for reporting it!

> - GCC 7.2 with -std=c++17 is the one with the Internal Compiler Errors
> (I didn't investigate further)

GCC constexpr ICE bug described in previous email.

> - clang++5-libstdc++ fails because it cannot find a type in std

On C++ 17, it uses std::in_place_type<T> instead of defining a local
copy. This error suggests that either the STL being used doesn't define
an implementation, or as you say a header is missing.

Yet at
https://github.com/ned14/boost-outcome/blob/master/include/boost/outcome/detail/value_storage.hpp#L40,
it very clearly is including the correct header, and at
https://github.com/ned14/boost-outcome/blob/master/include/boost/outcome/detail/value_storage.hpp#L40
it is correctly selecting std::in_place_type on the value of __cplusplus.

So on the basis of that, I'm going to assert that the libstdc++ version
you are using is insufficiently new.

> Is there a reason why you don't have travis and appveyor for
> boost-outcome? I remember your "Best Practices" document strongly
> advocating that every Boost library submission should have them.

https://travis-ci.org/ned14/boost-outcome

https://travis-ci.org/ned14/outcome

https://ci.appveyor.com/project/ned14/outcome

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
On Sat, Jan 27, 2018 at 6:43 PM, Niall Douglas wrote:

> On 27/01/2018 23:22, Glen Fernandes via Boost wrote:
>> I wrote a .travis.yml for your boost-outcome submission to run your
>> unit tests against a few compilers:
>>
>> The output is at: https://travis-ci.org/glenfe/ned14.outcome/builds/334222400
>>
>> Is there a reason why you don't have travis and appveyor for
>> boost-outcome? I remember your "Best Practices" document strongly
>> advocating that every Boost library submission should have them.
>
> https://travis-ci.org/ned14/boost-outcome
>
> https://travis-ci.org/ned14/outcome
>
> https://ci.appveyor.com/project/ned14/outcome
>

The first one is for boost-outcome, correct? It only tests g++-6 with
c++14, not g++-7 (in 14 or 17) (where the ICE occurs), or or
clang+libc++ (where your other errors were).

Aren't the second and third for standalone outcome? That isn't being
reviewed here, correct? (i.e. Standalone outcome doens't even ICE
MSVC2017 with Vinnie's test case, but boost-outcome, the one being
submitted for review, does?).

Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
On 27 January 2018 at 17:48, Glen Fernandes via Boost <[hidden email]
> wrote:

> ... where the ICE occurs...


Any ICE is a compiler issue... so this is the wrong place to report the
(ICE-)issue...

 gcc-7.3 has just been released
<https://gcc.gnu.org/ml/gcc/2018-01/msg00197.html> fixing over 99 bugs...

degski

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
>> https://travis-ci.org/ned14/boost-outcome
>>
>> https://travis-ci.org/ned14/outcome
>>
>> https://ci.appveyor.com/project/ned14/outcome
>>
>
> The first one is for boost-outcome, correct? It only tests g++-6 with
> c++14, not g++-7 (in 14 or 17) (where the ICE occurs), or or
> clang+libc++ (where your other errors were).

The same constexpr ICE problem afflicts GCC 6. But similarly it happens
randomly across machines.

> Aren't the second and third for standalone outcome? That isn't being
> reviewed here, correct? (i.e. Standalone outcome doens't even ICE
> MSVC2017 with Vinnie's test case, but boost-outcome, the one being
> submitted for review, does?).

I didn't think it necessary to run more CIs than at present. I know they
appear to be free to us, but I felt it anti-social to be using more jobs
than is strictly necessary. And the existing CIs cover all of MSVC,
clang and GCC, with Dinkumware, libstdc++ and libc++ on Linux, Windows
and MacOS. That's a good cross section of the most common configurations.

I am surprised about the clang + libc++ missing header because the XCode
9 CI job ought to have caught that, but that happens sometimes.

boost-outcome is the same library as Outcome standalone, if you do a
side by side diff you'll see that. The only change is the use of
Boost.Config instead of SD6 feature macros, and Boost.Exception. Hence I
felt that boost-outcome didn't need much testing above making sure it
works inside Boost, which is what the CI job does.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Sat, Jan 27, 2018 at 7:01 PM, degski via Boost wrote:
> Any ICE is a compiler issue... so this is the wrong place to report the
> (ICE-)issue...

Sure. The author putting a library forward for review should feel free
to report any compiler defects to vendors that his library exposes. It
looks like Niall has done that already.

Or are you suggesting that reviewers trying to review the submission
during the review (by, among other things, running tests) shouldn't
mention to the author that they encountered an ICE?

Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
On 27 January 2018 at 18:07, Glen Fernandes via Boost <[hidden email]
> wrote:

> ... shouldn't mention to the author that they encountered an ICE?
>

Sure you can mention it, but stating the obvious, the compiler shouldn't
crash, but should instead tell you what's wrong with the code (if anything
*is* actually wrong with the code, as it ICE-es we don't know, the only
thing we do know is that gcc-7.2 and lower won't handle it at all)...

degski

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 28/01/2018 00:07, Glen Fernandes via Boost wrote:
> On Sat, Jan 27, 2018 at 7:01 PM, degski via Boost wrote:
>> Any ICE is a compiler issue... so this is the wrong place to report the
>> (ICE-)issue...
>
> Sure. The author putting a library forward for review should feel free
> to report any compiler defects to vendors that his library exposes. It
> looks like Niall has done that already.

Yep, clang, MSVC and GCC have all fixed compiler bugs found by Outcome.
It's been a good test of their compiler's C++ 14 conformance.

clang and GCC have also fixed the code optimisation problems I reported,
and Microsoft have told me it's a high priority for MSVC once their new
optimiser is debugged fully.

> Or are you suggesting that reviewers trying to review the submission
> during the review (by, among other things, running tests) shouldn't
> mention to the author that they encountered an ICE?

Oh no, it's definitely important to report ICEs in the compiler. Just
please don't judge Outcome harshly for it, just six months ago Outcome
was unusable on anything but clang. It's thanks to the compiler vendors
being so great with me that Outcome is now widely usable.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Sat, Jan 27, 2018 at 7:15 PM, degski wrote:
> Sure you can mention it, but stating the obvious, the compiler shouldn't
> crash, but should instead tell you what's wrong with the code (if anything
> *is* actually wrong with the code, as it ICE-es we don't know, the only
> thing we do know is that gcc-7.2 and lower won't handle it at all)...

Sure. It would be nice if compilers didn't have defects at all. Be
they defects which result in an ICE, or defects that result in
incorrect or sub-optimal code being generated, failure to conform to
the specification, defective implementation of certain language or
library features.

One of the things I appreciated about Boost when I came to know Boost
libraries more intimately when we started using them at Microsoft, was
that they compensated for those defects (to the point where we would
use Boost equivalents even instead of our C++ implementation's
standard library facilities).

Glen

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
> the only
> thing we do know is that gcc-7.2 and lower won't handle it at all)...

My main development environment is MSVC on Windows as it's the lowest
common denominator usually. Then it's GCC on Linux. Finally, only
occasionally, I give it a whirl on clang on FreeBSD.

Outcome may cause GCC to sometimes ICE in its unit tests, but really I
assure you that GCC is a fine compiler to write Outcome based code with.
You very rarely encounter problems in normal coding, and GCC has a
Concepts implementation which Outcome uses and makes for much more
useful compiler error messages, plus I suspect it's a bit faster to
compile as Concepts are easier than SFINAE to evaluate.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
On 27 January 2018 at 18:24, Niall Douglas via Boost <[hidden email]>
wrote:

> My main development environment is MSVC on Windows as it's the lowest
> common denominator usually.


My IDE is VS2017 as well, but Clang/LLVM is doing the job...


> Outcome may cause GCC to sometimes ICE in its unit tests...


Doesn't matter where or how it ICE-es... it simply shouldn't... Your code
might crash, due to logic errors, UB, double free etc... but the compiler
shouldn't ICE... I'm just stating (in your defense actually) that for now
we don't know who the culprit is, but it starts with gcc.

If you want more useful compiler error messages, I would recommend
wholeheartedly Clang/LLVM <https://llvm.org/builds/>... That installer does
not integrate with VS2017 (it does with VS2015 and lower), but it's trivial
to do it manually, I can send you the props and target files (and where to
put them) if you want...

degski

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
On 27 January 2018 at 18:40, degski <[hidden email]> wrote:

> If you want more useful compiler error messages, I would recommend
> wholeheartedly Clang/LLVM <https://llvm.org/builds/>...
>

From what I read, STL stated that MS is testing the VC-STL against
Clang/LLVM to sort out the bugs...

degski

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
> If you want more useful compiler error messages, I would recommend
> wholeheartedly Clang/LLVM <https://llvm.org/builds/>... That installer does
> not integrate with VS2017 (it does with VS2015 and lower), but it's trivial
> to do it manually, I can send you the props and target files (and where to
> put them) if you want...

Already running it many years now :). Also, Windows Subsystem for Linux
using the same source tree as Windows. Works lovely.

I also have a personally hacked and recompiled clang-format plugin for
Visual Studio which auto-formats on save. Very, very useful.

Modern tooling is amazing. I only just recently was having a discussion
with Rob Stewart about the merits of intentionally typing out crap
unformatted C++ and letting clang-tidy -fix rewrite it for me into
non-crap C++, and clang-format to reformat it for me. He was not
convinced, but I think he is watching that space.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [review] outcome broken on clang/libstdc++ (Linux) and Apple clang/libc++ and gcc 7.2

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
>> If you want more useful compiler error messages, I would recommend
>> wholeheartedly Clang/LLVM <https://llvm.org/builds/>...
>>
>
> From what I read, STL stated that MS is testing the VC-STL against
> Clang/LLVM to sort out the bugs...

The Dinkumware STL is, and always has been, portable across compilers
and architectures. QNX ship Dinkumware as the system STL with GCC as the
compiler for example. Anyone can buy in Dinkumware as their STL for
their whatever weird platform for the right fee. It's a commercial
product totally separate from Microsoft, who are merely one of many
customers, though probably their biggest.

Niall

--
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost