Set UNIQUE property into index objects #849
Conversation
LaurentiuCristofor
requested review from
chuan,
senderista,
yiwen-wong and
simone-gaia
| { | ||
| case catalog::index_type_t::range: | ||
| return get_indexes()->emplace( | ||
| index_id, | ||
| std::make_shared<range_index_t, gaia_id_t>(static_cast<gaia_id_t&&>(index_id))) | ||
| std::make_shared<range_index_t, gaia_id_t, bool>( | ||
| static_cast<gaia_id_t&&>(index_id), static_cast<bool&&>(is_unique))) |
There was a problem hiding this comment.
Why you need to move a bool? (I see the same thing happens for gaia_id_t)
There was a problem hiding this comment.
I guess it's make_shared's way of taking ownership of the arguments. I've tried to not do the casts and got conversion errors about not knowing how to implicitly convert to bool&&.
This is C++ supposed to make things robust and easy to understand, I guess. ;) Very powerful, but easy to get wrong.
At least this error was easy, but I was getting a bunch of confusing errors that were actually coming from missing that include in the header file. It's only after trying to fix them several times that I realized that at the end they were complaining of not knowing what index_view_t was. But before they kept complaining about the definition of this method not matching any declaration. Now I understand that they defaulted the unknown type to something else and then they couldn't match the definition, but they should have reported the unknown type first, not the result of their assumptions. Even simple stuff is made complicated.
There was a problem hiding this comment.
Yeah, I guess that make_shared needs a "universal reference" which looks like &&, but is apparently not to be confused with an "rvalue reference", which looks exactly the same! Simplicity FTW! Even though I guess I should know this stuff, I'm just not sure how many brain cells I want to devote to this trivia.
There was a problem hiding this comment.
BTW here's a good article on universal references:
https://eli.thegreenplace.net/2014/perfect-forwarding-and-universal-references-in-c
And also an item in Effective Modern C++, but I don't know how to find it online.
There was a problem hiding this comment.
Getting on my Rust soapbox, I would never call Rust a simple language, but I don't think it has the same degree of accidental complexity as C++ (as shown here), and it has much better error messages!
There was a problem hiding this comment.
I guess it's
make_shared's way of taking ownership of the arguments. I've tried to not do the casts and got conversion errors about not knowing how to implicitly convert tobool&&.
Anyway, could you try using std::forward() here instead of the explicit casts?
There was a problem hiding this comment.
Right, perfect forwarding! I did read about that when I was messing with templates. Yes, I think that std::forward will make it much clearer. Good point.
There was a problem hiding this comment.
Changed. Thanks for the reference.
Basic functionality here is that the unique property is now set into index objects for later querying.
Some cleanup:
index_tclass which could just be inherited frombase_index_t.index_view_tobject so that we don't have to pass multiple index properties one by one.