Skip to content
This repository has been archived by the owner on Mar 20, 2024. It is now read-only.

[Bug] Implement more comparison ops for vectors of struct etc. #220

Closed
brson opened this issue Jul 7, 2023 · 5 comments · Fixed by #225, #226 or #228
Closed

[Bug] Implement more comparison ops for vectors of struct etc. #220

brson opened this issue Jul 7, 2023 · 5 comments · Fixed by #225, #226 or #228
Labels
bug Something isn't working

Comments

@brson
Copy link
Collaborator

brson commented Jul 7, 2023

In #219, some of the test cases in stdlib-bcs.move are commented out with fixmes. Most of them are because some of the comparison routines for vectors are not implemented, both in the compiler and runtime. This is true for at least vector<vector<_>> and vector<struct>.

@brson brson added the bug Something isn't working label Jul 7, 2023
@nvjle
Copy link

nvjle commented Jul 7, 2023

In #219, some of the test cases in stdlib-bcs.move are commented out with fixmes. Most of them are because some of the comparison routines for vectors are not implemented, both in the compiler and runtime. This is true for at least vector<vector<_>> and vector<struct>.

FWIW, this is primarily runtime work-- the compiler side is trivial once we have the move-native routines. The existing compiler asserts are there to avoid crapping out in the runtime (as those are more difficult to diagnose on quick inspection).

Previously, to allow us to go much further in the compiler implementation and testing proper, I implemented a preliminary vec_cmp_eq in move-native. It currently only handles the common case of vector-of-primitive. But vector-of-vector or vector-of-struct, or recursion thereof, is not implemented. Nor is just plain-old-struct comparison-- that is currently emitted from the compiler as LLVM IR (again, temporary measure to further the compiler implementation proper).

@brson
Copy link
Collaborator Author

brson commented Jul 7, 2023

@nvjle is this something you are going to look at, or should i?

@nvjle
Copy link

nvjle commented Jul 7, 2023

@nvjle is this something you are going to look at, or should i?

I'd prefer if you did-- especially since it really is needed to test the recent (and coming) serialization patches. But if you don't want the context switch, let me know and I'll put something together.

@nvjle
Copy link

nvjle commented Jul 8, 2023

@nvjle is this something you are going to look at, or should i?

I'd prefer if you did-- especially since it really is needed to test the recent (and coming) serialization patches. But if you don't want the context switch, let me know and I'll put something together.

@brson, never mind above, I'll work on it.

@nvjle
Copy link

nvjle commented Jul 10, 2023

I have committed a few patches (linked) which cover all the cases we were previously missing for vector and struct comparisons. We should now be able to handle fully general comparisons, with any level of recursion among the vector, struct, and primitive types. Closing.

@nvjle nvjle closed this as completed Jul 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
2 participants