Add contains() to some //base classes [chromium/src : main]
Victor Vianna (Gerrit)
Victor Vianna voted and added 3 comments![]( "Open in Gerrit")
Votes added by Victor Vianna
| Auto-Submit | +1 |
3 comments
#include "base/containers/contains.h"This one is still used, so the CL keeps it
Related details
- Daniel Cheng
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Victor Vianna (Gerrit)
Victor Vianna voted and added 2 comments![]( "Open in Gerrit")
Votes added by Victor Vianna
| Auto-Submit | +1 |
2 comments
#include "base/containers/contains.h"This one is still used, so the CL keeps it
Related details
- Daniel Cheng
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daniel Cheng (Gerrit)
Daniel Cheng added 3 comments![]( "Open in Gerrit")
constexpr bool contains(const key_type& key) const {This should support heterogeneous lookup.
small_map<std::unordered_map<int, int>> m;Please pick a container other than std::unordered_map here. I don't care strongly which one, but we have guidance that discourages new std::unordered_map use.
bool contains(const Key& key) const { return find(key) != end(); }Ditto: this was probably an oversight when we introduced variant map, but we should not persist this mistake for the new contains() method.
Related details
- Victor Vianna
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. |
Victor Vianna (Gerrit)
Victor Vianna voted and added 3 comments![]( "Open in Gerrit")
Votes added by Victor Vianna
| Auto-Submit | +1 |
3 comments
This should support heterogeneous lookup.
I just learned about heterogenous lookup, hopefully I got it right.
Please pick a container other than std::unordered_map here. I don't care strongly which one, but we have guidance that discourages new std::unordered_map use.
Done
bool contains(const Key& key) const { return find(key) != end(); }Ditto: this was probably an oversight when we introduced variant map, but we should not persist this mistake for the new contains() method.
VariantMap currently has no EqualKey template param
https://source.chromium.org/chromium/chromium/src/+/main:base/containers/variant_map.h;l=74;drc=1b049924c0f9432e31d6987e085a656c66bf94dc
Are you proposing I copy this to VariantMap?
https://source.chromium.org/chromium/chromium/src/+/main:base/containers/small_map.h;l=105-151;drc=134ef9afcaff9644ce859d61752d3ef51b924eb6
Related details
- Daniel Cheng
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. |
Daniel Cheng (Gerrit)
Daniel Cheng added 2 comments![]( "Open in Gerrit")
constexpr iterator find(const T& key) {You shouldn't change this; while we didn't hit it here, there are times when the non-template overload would be used (typically when template deduction would otherwise fail)
I'd just leave this as-is and add a TODO amd address it in a followup.
bool contains(const Key& key) const { return find(key) != end(); }Victor ViannaDitto: this was probably an oversight when we introduced variant map, but we should not persist this mistake for the new contains() method.
VariantMap currently has no EqualKey template param
https://source.chromium.org/chromium/chromium/src/+/main:base/containers/variant_map.h;l=74;drc=1b049924c0f9432e31d6987e085a656c66bf94dcAre you proposing I copy this to VariantMap?
https://source.chromium.org/chromium/chromium/src/+/main:base/containers/small_map.h;l=105-151;drc=134ef9afcaff9644ce859d61752d3ef51b924eb6
Ah... maybe it's a bit hard for `VariantMap` because one subtype is ordered and the other is not.
Let's just ignore this, since it's supposed to be removed in M145.
Related details
- Victor Vianna
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. |
Victor Vianna (Gerrit)
Victor Vianna added 2 comments![]( "Open in Gerrit")
constexpr bool contains(const key_type& key) const {
return find(key) != end();
}Do I even need this given the one below?
constexpr iterator find(const T& key) {You shouldn't change this; while we didn't hit it here, there are times when the non-template overload would be used (typically when template deduction would otherwise fail)
I'd just leave this as-is and add a TODO amd address it in a followup.
Sorry, the suggestion is not clear to me. If I don't have templated find(), how am I gonna implement contains()? Do you want me to duplicate+adapt the find's method body in contains?
If you just want a `constexpr iterator find(const key_type& key)`, I can do that in this patch already
Related details
- Daniel Cheng
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. |
Daniel Cheng (Gerrit)
Daniel Cheng added 1 comment![]( "Open in Gerrit")
constexpr iterator find(const T& key) {Victor ViannaYou shouldn't change this; while we didn't hit it here, there are times when the non-template overload would be used (typically when template deduction would otherwise fail)
I'd just leave this as-is and add a TODO amd address it in a followup.
Sorry, the suggestion is not clear to me. If I don't have templated find(), how am I gonna implement contains()? Do you want me to duplicate+adapt the find's method body in contains?
If you just want a `constexpr iterator find(const key_type& key)`, I can do that in this patch already
Oh right, `find()` is missing this and that's what `contains()` uses. It's probably easiest to duplicate it then; I don't think it's specified if the wrapped map is ordered or not.
Otherwise I would have suggested the trick that flat_tree uses: https://chromium-review.googlesource.com/c/chromium/src/+/7163836
Related details
- Victor Vianna
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. |