refactor: codegen for __tact_dict operations #993
Conversation
* __tact_dict_get * __tact_dict_exists * __tact_dict_get_min * __tact_dict_get_next * __tact_dict_set
|
It would be nice to have exhaustivity checks when dealing with all pairs of key and value map types, perhaps @verytactical can suggest something better than |
|
So should we merge this first and then I'll rebase #941 and update replace operations accordingly? |
|
Yep, that's the plan |
There was a problem hiding this comment.
Looks good. The only thing that worries me is how can we be sure that after the code was coalesced into the compact form, no old function is missing in the compact form.
For example, just to give a hypothetical example. Suppose that in function __tact_dict_get_uint_uint, instead of line var (r, ok) = udict_get?(d, kl, k);, it had the line var (r, ok) = udict_get(d, kl, k); (i.e., without the ? symbol).
How can we be sure that we didn't miss this special case for the unit-unit combination in the new code of genTactDictGet?
I tried to check each combination one by one to check that the compact form does capture all the old functions, but this is error prone and very slow.
Is there a test that tests each one of the old functions?
Only by exhaustive testing of different map key/value types combinations in Tact. We should not actually add FunC-specific tests since we will be implementing a new backend without using FunC. |
|
On the other hand, in this particular case we can generate all those operations and compare with the removed code |
|
@jeshecdom I compared the generated contract Test {
m1: map<Int, Int>;
m2: map<Int, Int as uint42>;
m3: map<Int, Address>;
m4: map<Int, Cell>;
m5: map<Int as uint8, Int>;
m6: map<Int as uint8, Int as uint42>;
m7: map<Int as uint8, Address>;
m8: map<Int as uint8, Cell>;
m9: map<Address, Int>;
mA: map<Address, Int as uint42>;
mB: map<Address, Address>;
mC: map<Address, Cell>;
receive() {
self.m1.set(1, 42);
self.m2.set(1, 42);
self.m3.set(1, myAddress());
self.m4.set(1, emptyCell());
self.m5.set(1, 42);
self.m6.set(1, 42);
self.m7.set(1, myAddress());
self.m8.set(1, emptyCell());
self.m9.set(myAddress(), 42);
self.mA.set(myAddress(), 42);
self.mB.set(myAddress(), myAddress());
self.mC.set(myAddress(), emptyCell());
self.m1.get(1);
self.m2.get(1);
self.m3.get(1);
self.m4.get(1);
self.m5.get(1);
self.m6.get(1);
self.m7.get(1);
self.m8.get(1);
self.m9.get(myAddress());
self.mA.get(myAddress());
self.mB.get(myAddress());
self.mC.get(myAddress());
self.m1.exists(1);
self.m2.exists(1);
self.m3.exists(1);
self.m4.exists(1);
self.m5.exists(1);
self.m6.exists(1);
self.m7.exists(1);
self.m8.exists(1);
self.m9.exists(myAddress());
self.mA.exists(myAddress());
self.mB.exists(myAddress());
self.mC.exists(myAddress());
foreach (_, _ in self.m1) {}
foreach (_, _ in self.m2) {}
foreach (_, _ in self.m3) {}
foreach (_, _ in self.m4) {}
foreach (_, _ in self.m5) {}
foreach (_, _ in self.m6) {}
foreach (_, _ in self.m7) {}
foreach (_, _ in self.m8) {}
foreach (_, _ in self.m9) {}
foreach (_, _ in self.mA) {}
foreach (_, _ in self.mB) {}
foreach (_, _ in self.mC) {}
}
} |
|
In that case, I think it is good to go. Should I approve the PR? |
|
@Gusarich Thanks for checking! |
Issue
Closes #992.
Checklist