Add alias templates for core and util traits #1255
Conversation
| @@ -60,7 +61,7 @@ struct to_range_point | |||
| static inline void apply(Geometry& geometry, Point const& point, | |||
| signed_size_type = -1, signed_size_type = 0) | |||
| { | |||
| typename geometry::point_type<Geometry>::type copy; | |||
| geometry::point_type_t<Geometry> copy; | |||
There was a problem hiding this comment.
This first confused me.
Because, what about point_t ...
But it is correct.
From the standard:
using add_pointer_t = typename add_pointer<T>::type
using common_type_t = typename common_type<T...>::type
We have to confirm to the second form.
There was a problem hiding this comment.
Yes, the standard pattern unfortunately is what it is and for someone familiar with it point_t would probably be confusing.
On similar note. I was thinking about adding aliases for Boost.Range traits like typename boost::range_value<Range>::type in util/range.hpp. C++20 adds this as std::ranges::range_value_t<>. In Boost.Geometry there is namespace bg::range already. This seems to be the best place for these aliases even though the word range is repeated. However I don't have a strong opinion about it, I see several possibilities:
boost::geometry::range::range_value_t<>boost::geometry::range_value_t<>boost::geometry::range::value_t<>boost::geometry::ranges::range_value_t<>(probably the closest to the standard)
Do you have any opinions?
There was a problem hiding this comment.
I would adhere to the standard. So that's the last one (incl. namespace) or the first one (with our namespace).
There was a problem hiding this comment.
I also agree with the 1st or last.
| @@ -90,7 +91,7 @@ namespace dispatch | |||
| template | |||
| < | |||
| typename Geometry, | |||
| typename Tag = typename tag_cast<typename tag<Geometry>::type, multi_tag>::type | |||
| typename Tag = tag_cast_t<tag_t<Geometry>, multi_tag> | |||
There was a problem hiding this comment.
I'll finish my review later (probably tomorrow) but this all looks very good, cool improvement. Way more readable.
Thanks for all this work.
|
|
||
|
|
||
| namespace util | ||
| { | ||
|
|
||
| template <typename T> | ||
| using bare_type = remove_cptrref<T>; | ||
| using bare_type [[deprecated]] = remove_cptrref<T>; |
There was a problem hiding this comment.
I think this needs a comment saying why this is deprecated, and what is the replacement.
There was a problem hiding this comment.
Actually I was not aware of cptrref and find it hardly readable.
bare_type is clearer.
But anyway, there is std::decay too, is it equivalent? Can't we deprecate both and use that instead (if it works)?
There was a problem hiding this comment.
I've used this name because in the standard there are:
remove_const<>
remove_volatile<>
remove_reference<>
remove_pointer<>
and then there are:
remove_cv<>
remove_cvref<>
The latter first removes reference and then const volatile. Because originally bare_type was first removing reference, then pointer, then cv (because there can be a reference to pointer) I named it:
remove_cptrref<>
If we wanted to remove volatile too which we probably should do I'd call it:
remove_cvptrref
but originally volatile was not removed so I didn't do it.
AFAIU std::decay wouldn't remove pointer.
| struct add_const_if_c | ||
| { | ||
| typedef std::conditional_t | ||
| struct [[deprecated]] add_const_if_c |
There was a problem hiding this comment.
Also here, can you add a comment?
| @@ -307,7 +364,8 @@ using enable_if_geometry_collection_t = typename enable_if_geometry_collection<G | |||
| \note Also a "ring" has areal properties within Boost.Geometry | |||
| \ingroup core | |||
| */ | |||
| using util::is_areal; | |||
| template <typename T> | |||
| using is_areal [[deprecated]] = util::is_areal<T>; | |||
There was a problem hiding this comment.
Before you ask, I'll add an explanation here as well. :)
| @@ -60,7 +61,7 @@ struct to_range_point | |||
| static inline void apply(Geometry& geometry, Point const& point, | |||
| signed_size_type = -1, signed_size_type = 0) | |||
| { | |||
| typename geometry::point_type<Geometry>::type copy; | |||
| geometry::point_type_t<Geometry> copy; | |||
There was a problem hiding this comment.
I also agree with the 1st or last.
Add C++17 variable templates for traits behind #ifdef Use aliases in several algorithms
awulkiew
force-pushed
the
feature/type_aliases
branch
from
5cfbfbc to
51cf51a
Compare
|
Thanks for the reviews. Should I merge it now or wait until the release to make bugfixing easiter? |
I think it is simpler to merge it after the release. |
Plus:
Add C++17 variable templates for traits behind #ifdef
Use aliases in several algorithms