FuelLabs / sway

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

Resolve type-system and unification related issues due to missing bottom type and `TypeSystem::Unknown` type annotations #5562

Open ironcev opened 8 months ago

ironcev commented 8 months ago

We have several open issues all coming from the same root cause, the fact that we miss the proper bottom type and that we are not passing the correct type annotations in TypeCheckContext. Here are some notable examples:

We had similar issues in the past, and always addressed them in isolation. This proposal calls for a systematic fix of all such issues by addressing the root cause.

Proposed steps are the following:

Introduce the proper bottom type

Remove the DeterministicallyAborts trait

DeterministicallyAborts actually compensates, or tries to compensate, for the missing bottom type. It is erroneous in its current implementation as #5438 explains, but there is no need to fix it. Once we properly introduce ! and diverging expression have it as a type, the consistent type system will automatically take care of the few places where deterministically_aborts was used.

Pass the function return type as a part of the context

@tritao already very well explained the need for this in #5518.

Check places where TypeInfo::Unknown is set as type annotation

There are parts of semantic analysis where we want TypeInfo::Unknown to be set as ctx type annotation. Such places should always be documented in a comment. There are other places where we do set it currently, either:

  1. because we were forced to due to the lacking of the proper bottom type
  2. because we have a bug and need to properly consider which type actually needs to be passed

An example for 1) is #5492. The bug shows us that if and match expression should not remove the received type annotation from the context by setting it to Unknown. But once @vaivaswatha put the proper annotations to ctx passed to those expressions we've started getting tons of issues related to the missing bottom type.

An example for 2) are #5559 and #5581.

Support Never and divergence in DCA

Currently, __revert() and revert() are treated as special cases in the DCA. Once we get the ! type, this special treatment should be replaced by the generic analysis of all functions that return !.

Also, once all diverging expressions get ! as type, we can straightforwardly fully support divergence in all expressions (e.g., #5561).

Optional: Introduce functions like unreachable and todo

Once we have ! we could offer counterparts of Rust unreachable and todo macros as std functions. If I remember well this was even requested during the std audit. The function would simply log the message and revert.

However, we should think carefully about this. One day we want to have macros and having these as macros like in Rust would be more suitable. Without macros we cannot offer at the same time todo() without arguments and todo("With argument"). But then, we can start with functions and turn to macros later on. Decision needed, but nevertheless '!' opens door to this functionality.

esdrubal commented 8 months ago

Thanks for the detailed issue, I will look into this.

esdrubal commented 8 months ago

This was closed by mistake.