icerpc / slicec

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

Split 'typealias' into 'type' and 'alias' #73

Closed InsertCreativityHere closed 2 years ago

InsertCreativityHere commented 2 years ago

Currently there are 2 main uses cases for the 'typealias' construct:

So, I propose that we split the typealias construct in two: type and alias

Alias

alias is purely about providing convenience to the Slice file writer

Open Questions

Type

type is purely about declaring a new type for the language mapping

Open Questions

Limitations

One limitation with custom type mappings in C#, is that they might not always be 'trait'-able. For instance if you map a type to System.DateTime, there's no way to implement a trait on this, since you have no control over this system type.

I think this is okay. In Rust and I believe Swift, this isn't an issue, you can implement a trait/protocol on anything. Even if C#, if you really need to use a trait, you can always map to your own custom type that extends or contains a DateTime. This is a limitation of C#, not of the slice construct.

InsertCreativityHere commented 2 years ago

After writing that proposal, I think it's obvious that type is a bad choice for a keyword. Too much ambiguity between type, a normal Slice type, and type this new construct.

Other ideas we had for the keyword:

bernardnormier commented 2 years ago

There is another option you didn't mention: rather than adding a new keyword, we could make cs:type an attribute for compact struct. For example:

type Time = [cs:type("System.DateTime")] [rust:type("std::time::SystemTime")] ulong;

could be written as:

[cs:type("System.DateTime")] 
[rust:type("std::time::SystemTime")]
compact struct Time
{ 
    value: ulong,
}

Advantage: no new keyword, obvious default mapping, clear that it's a new type and not an alias. Downside: makes compact struct more useful / visible and raises the question "should non-compact struct really be the default?".

InsertCreativityHere commented 2 years ago

Ah, I thought we'd ruled this option out. I think that was before we realized that compact structs worked with traits however...

It's no longer clear to me why we need to restrict this attribute to only compact structs. The only differences between compact and normal structs is that compact structs don't support tags, and hence can encoded slightly more efficiently. Correct?

If so, why does this affect it's ability to be custom-mapped? Can't we pass whatever on the wire and then let the user-provided decodeXFrom method do the custom-mapping?

pepone commented 2 years ago

Attributes are NOT allowed on aliases, and they also cannot be marked optional

We currently use cs:generic with the typealias, where would we put this if alias doesn't allow attributes, would this go to type too?

There is another option you didn't mention: rather than adding a new keyword, we could make cs:type an attribute for compact struct. For example:

Requiring a struct doesn't seems convenient if all that I want to do is mapping a collection to a custom collection that doesn't meet the cs:generic requirements. How do you see this use case working with the proposed compact struct.

bernardnormier commented 2 years ago

I think the best solution is to do the following:

  1. keep typealias as they are today (possibly rename them just alias) meaning:

    • they have a macro-like parser-only behavior - it's just a shortcut for a longer expression
    • they accept any attribute that can be specified inline on a type, such as cs:generic
  2. add the new cs:type attribute that I suggested for custom types on compact struct instead of typealias

Naturally, you can't use the cs:generic attribute on a struct, even if it has a single member.

It's no longer clear to me why we need to restrict this attribute to only compact structs.

It's because it makes little sense for a non-compact struct. The encoding/decoding methods that the user provides must match exactly the Slice encoding for the type. The Slice encoding of a compact struct is reasonably straightforward (especially if you don't have member with optional members, i.e. no bit sequence). The Slice encoding of a non-compact struct requires a decode function that handles tagged members and an encode method that encodes at least the tag end marker.

If you write:

[cs::type("DateTime"]
struct DateTime
{
    value: ulong,
}

It's much more likely that you forgot compact than that you used a non-compact struct on purpose.

InsertCreativityHere commented 2 years ago

possibly rename them just alias

If we do decide to keep typealias (current name) as is, I think that's still an ideal name, you can only alias types (not modules, exceptions, etc.) in Slice, and typealias makes this more clear than just alias.

The Slice encoding of a compact struct is reasonably straightforward

Perhaps I'm misunderstanding how we plan on supporting custom types. My thought was that for something like: (using your idea)

[cs:type("MyType")]
struct SliceStruct
{
    value: ulong
}

First we would decode a SliceStruct from a buffer, then use a user-supplied method to map a SliceStruct to a MyType.

Was your proposal that we skip this intermediate SliceStruct and users directly decode a MyType from a buffer?

bernardnormier commented 2 years ago

The custom encoding/decoding operates directly on encoder and decoder parameters.