sway icon indicating copy to clipboard operation
sway copied to clipboard

Consider refactoring monomorphizing into a separate step following `type_check`?

Open mitchmindtree opened this issue 3 years ago • 6 comments

At the moment, the process of monomorphizing generics is intertwined with the type-checking process.

I'm curious whether this is out of necessity, i.e. do we need to monomorphize in order to successfully type check? Or is it more the case that we can do it at the same time so we might as well?

In the case that it's the latter, I wonder if it might be worth splitting it into a separate step that immediately follows type checking?

The type checking process is quite complex, and as a newcomer to sway-core I've found grokking how the namespace is updated to be particularly tricky (could be just me!).

Building a mental model of how the namespace is updated requires following two axes:

  1. through the creation of new scopes and populating the scope's contents and
  2. subtle inner mutation within methods like namespace.find_method_for_type due to the inner resolve_type_with_self call that may monomorphize internally.

1. is quite intuitive to follow, but 2. seems a little shoe-horned in and is making #1213 particularly finicky (though I think that PR's still worth tackling before more global mutation sites start cropping up).

I'd imagine having distinct steps might make debugging issues related to type-checking and monomorphizing a little easier too? This is purely speculation though as I'm still new to sway-core.

I'm also unsure just how much work would be involved in such a refactor. I'd imagine at the very least the Namespace::resolve_* methods, and the semantics around TypeIds would need some rethinking.


Ahh I just noticed https://github.com/FuelLabs/sway/issues/862 which seems related.

mitchmindtree avatar Apr 15 '22 03:04 mitchmindtree

Also related to:

  • #1527
  • #1557

emilyaherbert avatar May 17 '22 13:05 emilyaherbert

It is related to needing to monomorphize in order to successfully type check. Ideally the inference engine is constantly improving its inference during the compilation process, but the original canonical function declarations do not get mutated.

Do you still want to pursue this now that #1213 has gone in? It would be a pretty big type system change to no longer require "passing" type checks and just use unresolved generics until we resolve them afterwards

sezna avatar May 21 '22 12:05 sezna

Just came across this issue after having a bit of a challenge trying to grok how the entire system is working as well, so it's not just you.

Separating the steps seems like it could simplify things, and might even be a necessary step to get https://github.com/FuelLabs/sway/issues/2636 to work with full optimality, because right now when we do monomorphization we still don't know the full inferenced signature, just a partial signature with explicit generic types.

tritao avatar Sep 23 '22 18:09 tritao

Note that this include trait constraint solving.

emilyaherbert avatar Jan 31 '23 18:01 emilyaherbert

A potential 80%-baked solution: use de bruijn indices to track generic types (and self types) as a separate entity in the AST in order to separate monomoprhization.

This is what Rust does. See the references here:

  • https://rustc-dev-guide.rust-lang.org/ty.html#question-why-not-substitute-inside-the-adtdef
  • https://rustc-dev-guide.rust-lang.org/generics.html
  • https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/subst/type.SubstsRef.html
  • https://rustc-dev-guide.rust-lang.org/variance.html?highlight=self%20type#variance-and-object-types

80%-baked solution for implementing "the self type" with de bruijn indices: #3762

  • the remaining 20% will be to update the design (probably something akin to the design in #3744 as #3762 is a precursor to #3744)

80%-baked solution for implementing generic types + self types with de bruijn indices: #3744

  • the remaining 20% will be to complete the monomorphization solver and AST pipeline

This solution will subsume writing a monomorphization cache (#2636).

emilyaherbert avatar Jun 01 '23 22:06 emilyaherbert

It seems that if we push the monomorphization phase after typechecking, we'll run into issues with a lot of Sway intrinsics, because those are often "polymorphic".

We can forbid some bounded polymorphic intrinsics like __eq<T>(lhs: T, rhs: T) if their arguments come from generics, but for things like __is_reference_type<T>() -> bool it looks more complicated. Or, for instance, for __check_str_type<T>() -> u64, which throws a compile time error if its type argument T is not a string type (which we cannot do for generic functions, so the write_str stdlib function won't compile anymore: https://github.com/FuelLabs/sway/blob/159a2402ab3a4079a18329706a586a40cbfa5fc4/sway-lib-std/src/hash.sw#L39)

anton-trunov avatar Sep 06 '23 17:09 anton-trunov