Skip to content
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

Making overload_t's copy constructor have priority #413

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

DarthRubik
Copy link

So currently the copy constructor for the overload_t does not take
precendent over the other templated constructor. This means a simple
function like the following fails:

void foo()
{
	auto a = hana::overload(f1,f2);
	auto b(a); //Error
}

To fix this I added a enable_if to turn off the constructor when not
applicable.........

Note: This (allegedly) fixes issue 412

So currently the copy constructor for the overload_t does not take
precendent over the other templated constructor.  This means a simple
function like the following fails:

	void foo()
	{
		auto a = hana::overload(f1,f2);
		auto b(a); //Error
	}

To fix this I added a enable_if to turn off the constructor when not
applicable.........
@DarthRubik
Copy link
Author

I am not quite sure why the automated builds failed......(I don't have much experience with appveyor or travis-ci).....when I did a local make check nothing failed........does someone have a clue as to what is going on here?

@ricejasonf
Copy link
Collaborator

ricejasonf commented Jul 1, 2018

It looks like it is detecting trailing white-space. I remember the error message for that being a little better. (err it actually had an error message for that at one time)

The Appveyor build failure looks like a config issue with MSVC.

I am new to this project, and did not realize that I needed to get
rid of trailing white space...but know I have removed the offending
space.....
@ricejasonf ricejasonf requested a review from ldionne July 10, 2018 04:23
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, with cosmetic changes. Also, please squash the two commits into one to avoid noise in the history.

@@ -0,0 +1,17 @@
#include "boost/hana.hpp"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use <> to include Hana's headers in the tests. Also, we try to include the most specific header. #include <boost/hana/functional/overload.hpp>

template <typename F_, typename ...G_>
template <typename F_, typename ...G_,
typename = std::enable_if_t<
!std::is_same<overload_t,std::decay_t<F_>>::value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Please add a space between overload_t and std::decay_t<F_>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants