add get_by_view method to Map

[Guest0x0]

This PR adds a new method get_by_view to Map. The motivation is the need for querying a Map[String, V] with @string.View. Converting the view to String first requires a copy, storing @string.View directly in the map may result in memory leak. So a new API is necessary for this case.

The signature of the new API is as follows:

/// Get the value associated with the view of a key.
/// When searching the map, instead of comparing keys directly,
/// a view of keys, obtained by the parameter `view`, is used.
/// This is useful for e.g. querying a `Map` of `String` with `@string.View`.
///
/// The `view` function and the `Hash` implementation of `View` should satisfy:
/// ```
/// view(key).hash() == key.hash() 
/// ```
fn get_by_view[K, View : Hash + Eq, Value](
  self : Self[K, Value],
  key : View,
  view~ : (K) -> View
) -> Value?

The map will be searched with the hash code of key. When comparing equality, existing keys in the map will be first converted to View using view.

Note that for the @string.View case, a specialized method may be adequate. But there are other cases such as @bytes.View. So a generic implementation is used here. The view function will only be called when comparing keys with identical hash code, so it will not be invoked a lot. Therefore even if get_by_view is not inlined, the indirect call cost of view should be acceptable (since using get_by_view implies that the key is probably a large type with non-trivial comparison, such as String)

[peter-jerry-ye-code-review]

Potential hash collision handling issue in get_by_view

Category
Correctness
Code Snippet
if entry.hash == hash && view(entry.key) == key {
break Some(entry.value)
}
Recommendation
Consider adding additional collision resolution logic similar to the insert method, in case multiple keys hash to the same value but have different views
Reasoning
The current implementation only checks if the hash matches and the view matches, but doesn't handle the case where different keys might produce the same hash and view. This could lead to incorrect results in rare cases.

Test coverage could be improved

Category
Maintainability
Code Snippet
test "map::get_by_view::string_view" {
let map = { "a": 1, "b": 2, "c": 3 }
Recommendation
Add test cases for:

1. Empty string views
2. Hash collision cases
3. Other view types (e.g. bytes.View)
4. Large strings to verify performance claims
   Reasoning
   The current test only covers basic string view cases. Given the generic nature of the implementation and the performance claims in the PR description, more comprehensive testing would increase confidence in the implementation.

view function called on each comparison during probing

Category
Performance
Code Snippet
if entry.hash == hash && view(entry.key) == key {
Recommendation
Consider caching the view of the key if the probe sequence is expected to be long, or add a comment explaining why this is not necessary
Reasoning
While the PR description mentions that view conversion won't be called often due to hash filtering, in a heavily loaded map with many collisions, the view function could be called multiple times during probing. A comment or optimization might be warranted.

[coveralls]

Pull Request Test Coverage Report for Build 6660

Details

• 4 of 5 (80.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.009%) to 92.442%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
builtin/linked_hash_map.mbt	4	5	80.0%

Totals
Change from base Build 6659:	-0.009%
Covered Lines:	6140
Relevant Lines:	6642

---

💛 - Coveralls

[hackwaly]

This is one of the case which show generic params of traits is useful.

[bobzhang]

this may be somthing easier to understand :

get_by_hashcode[K, View: Eq, V](self : Self[K,V], hashcode : Int , v : View, f~ : (K) -> View) -> Value? 

Then we can build something safe on top of it, is it a premature optimization?

[Guest0x0]

this may be somthing easier to understand :

get_by_hashcode[K, View: Eq, V](self : Self[K,V], hashcode : Int , v : View, f~ : (K) -> View) -> Value? 

Then we can build something safe on top of it, is it a premature optimization?

Is this a user-facing API, or just an internal helper? In terms of implementation, get_by_hashcode seems to be just get_by_view here with the hash code computation lifted out, is there any use of get_by_hashcode except for get_by_view?