editoast: use type SupportedSignalingSystem everywhere

[woshilapin]

In some places, we were converting to String with no real added value. Let’s keep HashSet<SupportedSignalingSystem> everywhere, avoiding conversions.

One of the inconvenient is the implementation of Hash for some of the struct that contains it (HashSet doesn’t implement Hash). With the help of educe, we can plug a custom function to hash such a HashSet. And we consider create a bitmask of all the present variant, using the discriminant of the variant’s enum. This should be unique, even if the order of the iteration is different.

[github-actions]

⚠️ API changes

This Pull Request introduces some changes in the API:

• please own it: notify or even prepare dedicated PR(s) to consumer projects

[hhirtz]

not sure adding more macro is a good step towards lower compilation times though... 🤔

[leovalais]

LGTM

[hhirtz]

Thank you! more typing is good, but i think this solution is overengineered

first commit message says:

The function that hashes a HashSet<SupportedSignalingSystemVariant>
is not trivial

but it is though? if you inline the discriminant functions you see it's just a 1-statement for loop and a call to hash. i don't think it calls for 4 tests with proptest... although i agree it's fun to play with, editoast isn't a playground for it

It looks like if we impl Ord for SupportedSignalingSystemVariant, we don't need the custom hashing function and the second commit becomes a lot simpler?

[woshilapin]

Thank you! more typing is good, but i think this solution is overengineered

The reason I had the discriminant function extracted is for documentation purpose. A discriminant does mean something specific in the enum ecosystem. And the function was not exposed anyway, so it stayed an internal implementation detail. That said, I’m not going to die on that hill, since we already have a comment in the implementation of the hashing function, explaining what are those magic numbers. So I inlined it.

first commit message says:

The function that hashes a HashSet<SupportedSignalingSystemVariant>
is not trivial

but it is though? if you inline the discriminant functions you see it's just a 1-statement for loop and a call to hash. i don't think it calls for 4 tests with proptest... although i agree it's fun to play with, editoast isn't a playground for it

I disagree. The complexity does not lie in the implementation itself (indeed, it’s a few lines of code), the complexity lies in the fact that this implementation does respect the properties a Hash function must enforce. So I do believe having property testing makes sense here.

It looks like if we impl Ord for SupportedSignalingSystemVariant, we don't need the custom hashing function and the second commit becomes a lot simpler?

I’m not comfortable implementing Ord on a enum where it has no reasonable semantic meaning.