feat: Map support constructor initialization with MapInitialEntries#2941
feat: Map support constructor initialization with MapInitialEntries#2941pebrianz wants to merge 7 commits intoAssemblyScript:mainfrom
Conversation
- Added new type alias `MapInitialEntries<K, V>` = { key: K, value: V }[]
- Map constructor now accepts:
• An array of { key, value } objects
• Any array typed as MapInitialEntries<K,V>
- This allows populating a Map in one step with strong typing.
- Example:
let myMap = new Map<string, i32>([
{ key: "a", value: 1 },
{ key: "b", value: 2 },
]);
let init: MapInitialEntries<string, i32> = [
{ key: "x", value: 10 },
{ key: "y", value: 20 },
];
let anotherMap = new Map<string, i32>(init);
std/assembly/map.ts
Outdated
| if(entriesLength > 0) { | ||
| if(entriesLength >= this.entriesCapacity) this.bucketsMask = entriesLength; | ||
| this.rehash((this.bucketsMask << 1) | 1); | ||
|
|
||
| for(let i = 0; i < entriesLength; i++){ |
There was a problem hiding this comment.
| if(entriesLength > 0) { | |
| if(entriesLength >= this.entriesCapacity) this.bucketsMask = entriesLength; | |
| this.rehash((this.bucketsMask << 1) | 1); | |
| for(let i = 0; i < entriesLength; i++){ | |
| if (entriesLength > 0) { | |
| if (entriesLength >= this.entriesCapacity) this.bucketsMask = entriesLength; | |
| this.rehash((this.bucketsMask << 1) | 1); | |
| for (let i = 0; i < entriesLength; i++) { |
pebrianz
changed the title
|
the test rt/flags failed to compile, when pass type v128 to the map. it need simd to be enabled. Do you have any suggestion? |
|
For update all fixtures, try to run |
CountBleck
left a comment
CountBleck
left a comment
There was a problem hiding this comment.
This seems acceptable, since we don't have tuples or iterables/iterators. (If/when we do add support for tuples, we would replace this mechanism as a breaking change.) I don't know if we should be concerned about the size increase in symbol.release.wat.
See additional comments below.
std/assembly/map.ts
Outdated
|
|
||
| constructor() { | ||
| /* nop */ | ||
| constructor(initialEntries: MapInitialEntries<K,V> = []) { |
There was a problem hiding this comment.
This should be nullable, and the default argument should be null instead of []. Tip: if you wrap the entire constructor body with if (initialEntries) {, you can avoid doing initialEntries!.
(I wonder if this change could be beneficial for code that never uses this feature, by making it easier for Binaryen to optimize)
std/assembly/map.ts
Outdated
| private entriesCount: i32 = 0; | ||
|
|
||
| constructor(initialEntries: MapInitialEntries<K,V> = []) { | ||
| let entriesLength = initialEntries.length; |
There was a problem hiding this comment.
plz revert this cached entriesLength var and use instead initialEntries.length
std/assembly/map.ts
Outdated
| if (initialEntries.length >= this.entriesCapacity) this.bucketsMask = initialEntries.length; | ||
| this.rehash((this.bucketsMask << 1) | 1); | ||
|
|
||
| for (let i = 0; i < initialEntries.length; i++) { |
There was a problem hiding this comment.
plz cache initialEntries.length into var
std/assembly/map.ts
Outdated
| constructor() { | ||
| /* nop */ | ||
| constructor(initialEntries: MapInitialEntries<K,V> | null = null) { | ||
| if (initialEntries) { |
There was a problem hiding this comment.
Just use early return:
| if (initialEntries) { | |
| if (!initialEntries || !initialEntries.length) return; |
This will reduce one level of ident
…nd use cache initialEntries length
|
This PR has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions! |
|
This PR has been automatically closed due to lack of recent activity, but feel free to reopen it as long as you merge in the main branch afterwards. |
|
Just gonna keep this open so it doesn't get buried too quickly |
3 participants
MapInitialEntries<K, V>= { key: K, value: V }[]• An array of { key, value } objects
• Any array typed as MapInitialEntries<K,V>
Fixes #2940