icerpc / slicec

The Slice compiler library
Apache License 2.0
13 stars 5 forks source link

Merge Class and Exception grammar elements #697

Open bernardnormier opened 1 month ago

bernardnormier commented 1 month ago

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 # https://github.com/icerpc/icerpc-csharp/pull/4024).

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

InsertCreativityHere commented 5 days ago

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.