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

Lazy any #4343

Merged
merged 11 commits into from Jul 5, 2018
Merged

Lazy any #4343

merged 11 commits into from Jul 5, 2018

Conversation

lisitsyn
Copy link
Member

No description provided.

@lisitsyn lisitsyn requested a review from karlnapf June 23, 2018 10:48
@lisitsyn
Copy link
Member Author

@karlnapf I'm in progress yet, just to let you know something is going on ;)

Copy link
Member

@karlnapf karlnapf left a comment

Choose a reason for hiding this comment

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

not much to review yet :D

{
auto v = 9;
auto any = make_any<int>([=]() { return v; });
EXPECT_EQ(any.as<int32_t>(), v);
Copy link
Member Author

Choose a reason for hiding this comment

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

@karlnapf this thing works now, but it definitely needs more thorough testing

Copy link
Member

Choose a reason for hiding this comment

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

shall we try to test a slightly more elaborate example of registering a member function that returns a constant or something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I can add one.

@lisitsyn
Copy link
Member Author

@karlnapf an interesting question is whether we want to support the following assignments:

  • std::function<T()> = T
  • T = std::function<T()>

@karlnapf
Copy link
Member

idk, imo we only need the functions to returns stuff. But you are the guru here :D

@lisitsyn lisitsyn changed the title Lazy any [wip] Lazy any Jun 24, 2018
any = make_any<int>([=]() { return v; }), TypeMismatchException);
}

TEST(Any, lazy_member_function)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about a test case that asserts laziness?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea

Copy link
Member

Choose a reason for hiding this comment

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

Also, we need to keep in mind equals/serialization should exclude lazy computations.

Copy link
Member

Choose a reason for hiding this comment

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

i.e. we need some kind of markup. Otherwise these things will be equaled/cloned/serialized as well....or is that intended? Costs computation and I am not sure it is worth it

Copy link
Member

Choose a reason for hiding this comment

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

ah, clone_from will be a problem as it attempts to write into the member

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a bunch of questions :)

  • Serialization is tricky. It seems the 'serializator' itself should know
  • We can explicitly throw something when trying to do equals
  • Clone is cheap but it won't work correctly. It would use the original object

Copy link
Member

Choose a reason for hiding this comment

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

I think then in clone and equals, I would just skip all registered function anys

@karlnapf
Copy link
Member

karlnapf commented Jul 1, 2018

}

template <class T>
struct has_result_type
Copy link
Member

Choose a reason for hiding this comment

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

lol :D


int computed_member() const
{
return 90210;
Copy link
Member

Choose a reason for hiding this comment

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

42?

@karlnapf
Copy link
Member

karlnapf commented Jul 5, 2018

All fixed? :D

@lisitsyn
Copy link
Member Author

lisitsyn commented Jul 5, 2018

Not yet, there is a bug in has_type_fallback. And some lazyness flag visible outside is due.

@karlnapf
Copy link
Member

karlnapf commented Jul 5, 2018

add the lazyness flag you lazy flagger

@lisitsyn
Copy link
Member Author

lisitsyn commented Jul 5, 2018

I am lazy to add the lazyness

any = make_any<int>([=]() { return 222; });
EXPECT_FALSE(any.cloneable());
EXPECT_FALSE(any.visitable());
EXPECT_THROW(any.visit(nullptr), std::logic_error);
Copy link
Member

Choose a reason for hiding this comment

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

sweet!

{
SG_SDEBUG(
"Skipping clone of %s::%s of type %s.\n", this->get_name(),
tag.name().c_str(), own.type().c_str());
Copy link
Member

Choose a reason for hiding this comment

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

continue!

Copy link
Member

Choose a reason for hiding this comment

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

actually, why not move the check after the debug msg below, then you dont need to print the name and stuff again, but can just say "Skipping as not cloneable"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah but since it is here such thing might be useful anyway

"Skipping comparison of %s::%s of type %s as it is "
"non-visitable.\n",
this->get_name(), tag.name().c_str(), own.type().c_str());
continue;
Copy link
Member

Choose a reason for hiding this comment

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

same here, i would move this down and only say "skipping" to avoid duplicate code

@lisitsyn lisitsyn merged commit 80a4001 into shogun-toolbox:develop Jul 5, 2018
ktiefe pushed a commit to ktiefe/shogun that referenced this pull request Jul 30, 2019
* Add simple draft of lazy any

* Do not deref null pointer in any

* Specialize for std::function

* Add assignment tests

* Use is_functional to actually call functions

* Fix style

* Add get_value specialization for Empty

* Make has_type_fallback support std::function

* Add cloneable/visitable to filter out some Anys

* Skip non-visitable and non-cloneable anys

* Add missed continue
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.

None yet

3 participants