[traits_bag.h] Use base::ParameterPack for trait validation [chromium/src : main]
Patrick Monette (Gerrit)
Patrick Monette added 1 comment![]( "Open in Gerrit")
Related details
- Etienne Pierre-Doray
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray added 2 comments![]( "Open in Gerrit")
ValidTraits::template HasType<std::remove_cvref_t<T>>::value ||Nit: decay_t for simplicity
While there are subtle difference I don't think they matter here.
inline constexpr bool HasTypeInPack =This is the same as HasTrait() below?
Related details
- Patrick Monette
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Monette (Gerrit)
Patrick Monette added 3 comments![]( "Open in Gerrit")
PTAnL
ValidTraits::template HasType<std::remove_cvref_t<T>>::value ||Nit: decay_t for simplicity
While there are subtle difference I don't think they matter here.
I have a preference for keeping remove_cvref as it has the exact semantics I want. Given that it's isolated to a single concept, I think it's pretty readable as it is.
Do you have a strong opinion on this?
inline constexpr bool HasTypeInPack =This is the same as HasTrait() below?
Yes and no.
HasTypeInPack is the internal helper, with a precise name for what it does. It's used also for the Exclude::Filter method, while HasTrait is the public facing API, with the nice error message. The error message doesn't make sense for Exclude::Filter.
I also considered moving all helpers to an internal namespace, but wasn't sure if that was warranted given the existence of the trait_helpers namespace.
Related details
- Etienne Pierre-Doray
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Etienne Pierre-Doray (Gerrit)
Etienne Pierre-Doray voted and added 3 comments![]( "Open in Gerrit")
Votes added by Etienne Pierre-Doray
| Code-Review | +1 |
3 comments
ValidTraits::template HasType<std::remove_cvref_t<T>>::value ||Patrick MonetteNit: decay_t for simplicity
While there are subtle difference I don't think they matter here.
I have a preference for keeping remove_cvref as it has the exact semantics I want. Given that it's isolated to a single concept, I think it's pretty readable as it is.
Do you have a strong opinion on this?
Acknowledged
inline constexpr bool HasTypeInPack =Patrick MonetteThis is the same as HasTrait() below?
Yes and no.
HasTypeInPack is the internal helper, with a precise name for what it does. It's used also for the Exclude::Filter method, while HasTrait is the public facing API, with the nice error message. The error message doesn't make sense for Exclude::Filter.
I also considered moving all helpers to an internal namespace, but wasn't sure if that was warranted given the existence of the trait_helpers namespace.
Ah I see, maybe this should go in base/parameter_pack.h though?
Related details
- Patrick Monette
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Monette (Gerrit)
Patrick Monette added 2 comments![]( "Open in Gerrit")
Thanks Etienne
Gab can you take a look, and maybe owners-override if you deem appropriate. Thanks
inline constexpr bool HasTypeInPack =Patrick MonetteThis is the same as HasTrait() below?
Etienne Pierre-DorayYes and no.
HasTypeInPack is the internal helper, with a precise name for what it does. It's used also for the Exclude::Filter method, while HasTrait is the public facing API, with the nice error message. The error message doesn't make sense for Exclude::Filter.
I also considered moving all helpers to an internal namespace, but wasn't sure if that was warranted given the existence of the trait_helpers namespace.
Ah I see, maybe this should go in base/parameter_pack.h though?
It could but ultimately I don't think so. While it uses ParameterPack internally as implementation, the pack in the name refers to a variadic pack, not a base::ParameterPack.
I decided to rename it to HasTypeInVariadicPack
Related details
- Gabriel Charette
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |