or you change the test so that it dereferences the pointer-to-the-vertex prior to calling the functor (or as part of calling the functor) in which case I would expect that you've set up the functor to not return an optional, but a plain value (i.e. the functor only return optional if called with a pointer, and if called with a reference, it just returns the value)
yet it is - if you expect a value to be present ;-)
we could make it a bit more elegant by defining require_pointer_t
as:
template <typename T>
constexpr bool is_pointerlike_v = std::is_pointer_v<std::remove_cv_t<std::remove_reference_t<T>>>
|| is_SmartRef_v<std::remove_cv_t<std::remove_reference_t<T>>>;
template <typename T, typename Ret>
using require_pointer_t = std::enable_if_t<is_pointerlike_v<T>,Ret>;
and then you can use is_pointerlike_v
OK, I missed one validator exception -- also fixed that one now...
Gerhard Raven (bc86ae02) at 28 Mar 10:50
remove (now) unnecessary explicit controlflow for unpacking and exp...
... and 1 more commit
oops -- reading a 'diff' can be more confusing then the underlying code. Clearly one is in the ImpactParameterChi2
and the other in ImpactParameter
-- and of course their 'structure' should look the same, they just call a different actual function ;-)
if constexpr (LHCb::Event::details::require_pointer_v<VertexType>> ) {
return vertex ? Functors::Optional{(*this)(*vertex,track_chunk )} : std::nullopt ;
} else {
using Sel::Utils::impactParameterChi2;
return impactParameterChi2( track_chunk, vertex );
}
at which point this is the same as other overload, so can be dropped altogether...
The simplest version of doing this is:
if constexpr (LHCb::Event::details::require_pointer_t<Position_t> ) {
return vertex_pos ? Functors::Optional{ (*this)(*vertex_pos,track_chunk) } : std::nullopt ;
} else {
using Sel::Utils::impactParameterSquared;
return sqrt( impactParameterSquared( vertex_pos, track_chunk ) );
}
}
as require_pointer_t
also deals with SmartRef
properly, and you should (unless there is a reason not to) use a SmartRef
as if it is a pointer, i.e. call *
on it.
(Note: I don't now whether VALUE_OR
is 'smart' enough to accept a non-optional argument, and in those cases just return the argument unmodified... but that seems like a reasonable behavior that you may need in this case)
return Functional::Optional{impactParameterChi2( track_chunk, vertex )} ;
but why is this optional, if it always returns an engaged optional, so I'd expect this branch to just be:
return impactParameterChi2( track_chunk, vertex );
using returntype = decltype( (*this)(*vertex,track_chunk ) ) ;
return vertex ? Functors::Optional{(*this)(*vertex,track_chunk )} : std::nullopt ;
which relies ?:
will try to find a common type between the two arguments, and Functors::Optional<T>
can be constructed from std::nullopt
Given that VertexType
is not a reference (as the argument is VectrexType const&
and one cannot have a reference to a reference) and also the const
is redundundant for the same reason, this is equivalent to, and thus should just be:
if constexpr (LHCb::Event::details::is_SmartRef_v<VertexType> ) {
You have:
using Sel::Utils::impactParameterChi2;
using returntype = std::optional< decltype( impactParameterChi2( track_chunk, vertex ) ) > ;
return returntype{impactParameterChi2( track_chunk, vertex )} ;
which should read simply:
using Sel::Utils::impactParameterChi2;
return Functors::Optional{impactParameterChi2( track_chunk, vertex )} ;
at that point, the arithmetic comparisons should also start to work.
Note: we use Functors::Optional
instead of std::optional
in order to make 'unengaged' optionals behave like NaN for arithmetic comparisons, ie. when comparing an unengaged optional with value, or an engaged optional of the same type, fail. `std::optional' has (in theses cases) the 'wrong' default behavior.
If the current ci-test does what I expect it to do, then yes, this could be merged soon ;-)
both DataHandle::setKey
and DataHandle::updateKey
do the exact same thing, so standardize on one of them (so that the other can be removed subsequently)
Gerhard Raven (d2493d5c) at 28 Mar 08:48
prefer setKey
over updateKey
Some of the DaVinci failures should be fixed at this point, so let's try again:
/ci-test Moore!3117 DaVinci!1056
Gerhard Raven (63f0024a) at 27 Mar 23:46
remove (now) unnecessary explicit controlflow for unpacking and exp...
... and 1 more commit
Gerhard Raven (34942362) at 27 Mar 22:18
remove (now) unnecessary explicit controlflow for unpacking and exp...
... and 1 more commit
Gerhard Raven (a8dd1898) at 27 Mar 22:16
remove (now) unnecessary explicit controlflow for unpacking and exp...
... and 1 more commit