icerpc/slicec

Merge Class and Exception grammar elements

Opened this issue · 1 comments

Class and Exception are very similar:
https://github.com/icerpc/slicec/blob/main/src/grammar/elements/class.rs
https://github.com/icerpc/slicec/blob/main/src/grammar/elements/exception.rs

And the mapping for both (in C# for now) is also extremely close (see # icerpc/icerpc-csharp#4024).

It would be much more convenient to have a single grammar element, Class, for both.

After thinking it over, I'm no longer convinced this would actually simplify things.

Remember that slicec-cs is transient. The end goal is to rewrite it in C#. So, how we represent it in Rust is irrelevant to C#.
slicec-cs is free to have a single type for both Exception and Class, independent of how the compiler stores them.

So, put slicec-cs aside. And the C# mapping. All there is to consider is slicec.
And within slicec merging these two things together actually causes more complication than it solves.


The implied implementation for this is to delete the Exception type, and add a bool: is_exception field to Class.
But, there are too many places where the distinction between classes and exceptions matters, and having to check this bool (instead of just relying on the type system) adds up.

Most importantly, Class is a type, whereas Exception is not.
Actually, Exception is very limited in Slice. They can only appear in throws clauses, or as a base exception.

So, while they do share almost all their fields, they have very different behavior/use-cases. To the point where there is actually, literally 0 overlap in functionality; ie, anywhere where you can use a class, you cannot use an exception instead, and vice-versa.


Also I just think it's cleaner and more readable to have separate types.
Otherwise with a single type, I think it would be deceptively easy to add some logic in the future, which forgets about this caveat (where some classes are exceptions) which breaks something in a subtle (and likely under-tested) way.