-
Notifications
You must be signed in to change notification settings - Fork 449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Defer Invokable test in ranges::action #634
Conversation
Thanks very much for looking into this so quickly. I'm finding that this PR fixes my Clang but breaks my GCC 6.2.0 (coloured diagnostics in attached file) :
|
meta::compose< | ||
meta::bind_back<meta::quote<Sortable>, C, P>, | ||
meta::quote<iterator_t>>, | ||
Rng>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This constraint is identical to that in action/sort.hpp. Should we factor it out into a SortableRange
concept?
test/action/join.cpp
Outdated
#include "../simple_test.hpp" | ||
#include "../test_utils.hpp" | ||
|
||
template<class> struct undef; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've left your type printer debugging tool in the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With a more descriptive name - I always call it show_type
and make it variadic - it would be a good idea to put an intentionally undefined template in e.g. test_utils
. It would not suck to never have to type this line of code again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always call it dump
, would be nice to have this in test_utils
.
{ | ||
using namespace ranges; | ||
|
||
std::vector<std::string> v {"hello"," ","world"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#include <string>
I borrowed some ideas from @odinthenerd to make |
I'm really struggling getting this to compile on Clang 3.6 and GCC 4.8. If anybody thinks they can do it I would gladly accept help. |
FWIW, I can confirm that the PR has now fixed #632 for me on Clang 3.8.1 and GCC 6.2.0. I'd be happy to help but it's very doubtful that I'd make more progress than you. |
da45d36
to
d372008
Compare
Attn @CaseyCarter : I have rebased this branch. Be careful. |
This has been a ridiculously complicated bug fix. We really should ban @tonyelewis ;) |
Eagerly anticipating a time when I can drop gcc-4.x support. |
I really hope this "4.8" is a typo. 141 of 203 tests ICE on utility/concepts.hpp line 111 for me compiling with 4.8. |
Yes, I meant gcc-4.9. I dropped gcc-4.8 with the 0.2.0 release, and there's no going back now. |
… views to help gcc-4.9
Avoids some nasty interaction between defaulted and inherited default constructors that manifests with g++-4.9 in utility.variant and view.transform.
9862bdb
to
e963356
Compare
FINALLY. |
Apologies for being a trouble-causer. ;) |
I like the implementation of |
I picked up the technique on Stackoverflow years ago; I've seen it attributed to Xeo. In addition to being more extensible, I think it expresses intent far more clearly than ad-hoc |
Also:
action::stable_sort
andaction::view
fixes #632