Remove DynType and resculpt ast::Type

[tyranron]

Follows #1247

Synopsis

In #1247 the ast::Type was made generic over the string type used for name.

pub enum Type<N = ArcStr> {
    Named(N),
    List(Box<Type<N>>, Option<usize>),
    NonNullNamed(N),
    NonNullList(Box<Type<N>>, Option<usize>),
}

In the codebase, there are numerous places where it's required to convert both ast::Type<ArcStr> and ast::Type<&str> to some singular representation stored in ValidatorContext or similar places.

However, the current ast::Type enum representation using Boxes introduces a problem that it's not possible to cheaply receive a borrowed version of ast::Type<ArcStr> as ast::Type<&str> without re-allocating all those Boxes. Furthermore, using Boxes involves redundant re-allocations on Cloneing even when &str is used.

Thus, @audunhalland came up with a temporary DynType solution to solve this problem:

pub enum DynType<'a> {
    Named(&'a str),
    List(&'a dyn AsDynType, Option<usize>),
    NonNullNamed(&'a str),
    NonNullList(&'a dyn AsDynType, Option<usize>),
}

pub trait AsDynType: fmt::Debug {
    fn as_dyn_type(&self) -> DynType<'_>;
}
impl AsDynType for ast::Type<ArcStr> { }
impl AsDynType for ast::Type<&str> {}

The problem with this solution, that it duplicates ast::Type inner logic and pollutes some potentially public APIs.

Solution

As @audunhalland kindly suggested in TODO comment:

// TODO: Ideally this type should not exist, but the reason it currently does is that `Type` has a
//       recursive design to allow arbitrary number of list wrappings.
//       The list layout could instead be modelled as a modifier so that type becomes a tuple of
//       (name, modifier).
//       If `Type` is modelled like this it becomes easier to project it as a borrowed version of
//       itself, i.e. [Type<ArcStr>] vs [Type<&str>].

This PR resculpts ast::Type in this manner:

pub enum TypeModifier {
    NonNull,
    List(Option<usize>),
}

pub(crate) type BorrowedType<'a> = Type<&'a str, &'a [TypeModifier]>;

#[derive(Clone, Debug)]
pub struct Type<N = ArcStr, M = Box<[TypeModifier]>> {
    name: N,
    modifiers: M,
}

Checklist

• Docs are updated
• Tests are updated
• CHANGELOG.md updated

[tyranron]

@audunhalland it would be nice if you provide some input of this, if you have time to spare. Thanks!

[audunhalland]

@tyranron I had a look, it's something like this I imagined.

A variation of the smallvec trick is a type that instead uses statics for commonly used modifier patterns:

audunhalland@afcf8a7

The advantage of this is that the TypeModifiers is 24 bytes while SmallVec<[TypeModifier; 2]> is 48 bytes. The TypeModifiers trick can be easily extended to cover more cases.

[tyranron]

@audunhalland the reason I've used SmallVec is not that of small-sized optimization in the first place, but rather because it's needed to be mutable to allow wrapping. Initially, I was using Box<[TypeModifier]> and realization that it would require re-allocation on every wrapping lead me to Vec (and thus SmallVec for cutting of allocations on most common types). Though, in your example, it's amortized with some a little more complicated logic for wrapping.

Thanks for the tip! It's very helpful. I'll think on it more and will try to do something of it.