FuelLabs / sway

🌴 Empowering everyone to build reliable and efficient smart contracts.
https://docs.fuel.network/docs/sway/
Apache License 2.0
62.84k stars 5.35k forks source link

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

Open mitchmindtree opened 2 years ago

mitchmindtree commented 2 years ago

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.

emilyaherbert commented 2 years ago

Also related to:

sezna commented 2 years ago

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

tritao commented 1 year ago

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.

emilyaherbert commented 1 year ago

Note that this include trait constraint solving.

emilyaherbert commented 1 year ago

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:

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

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

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

anton-trunov commented 11 months ago

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)