Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Declaration engine #11

Merged
merged 19 commits into from
Sep 27, 2022
Merged

Declaration engine #11

merged 19 commits into from
Sep 27, 2022

Conversation

emilyaherbert
Copy link
Contributor

@emilyaherbert emilyaherbert commented Jul 13, 2022

@emilyaherbert emilyaherbert self-assigned this Jul 13, 2022
emilyaherbert added 2 commits July 13, 2022 19:46
@emilyaherbert emilyaherbert marked this pull request as ready for review August 3, 2022 13:02
rfcs/0003-declaration-engine.md Outdated Show resolved Hide resolved
rfcs/0003-declaration-engine.md Outdated Show resolved Hide resolved
rfcs/0003-declaration-engine.md Outdated Show resolved Hide resolved
rfcs/0003-declaration-engine.md Outdated Show resolved Hide resolved
rfcs/0003-declaration-engine.md Outdated Show resolved Hide resolved
rfcs/0003-declaration-engine.md Outdated Show resolved Hide resolved
rfcs/0003-declaration-engine.md Outdated Show resolved Hide resolved
rfcs/0003-declaration-engine.md Outdated Show resolved Hide resolved
rfcs/0003-declaration-engine.md Outdated Show resolved Hide resolved
}

impl DeclarationEngine {
fn add_function(&mut self, function: TypedFunctionDeclaration) -> DeclarationDefinition {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the function has to be typed when we insert it into the declaration, doesn't that mean we maintain the status quo of having to type check (and perform inference on) everything before beginning this process?

@sezna
Copy link
Contributor

sezna commented Aug 3, 2022

Sooo excited for work on this to begin 🚀

Copy link
Contributor

@otrho otrho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you've addressed out of order declarations and dependencies here? Is that another thing the decl engine was supposed to handle?

Allowing out of order declarations and to remove the declaration sorting we have now and especially to allow recursive data structures and function calls is highly desirable.

rfcs/0003-declaration-engine.md Outdated Show resolved Hide resolved
@adlerjohn adlerjohn requested a review from a team August 5, 2022 01:38
Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one Emily!

I don't have much to add on top of the others' comments, though I think your insight about our current lack of clarity on how to perform type inference in a consistent manner being a core issue is a really key insight and makes a lot of sense! I can see the proposed DeclarationEngine going a long way to help with this.

The described approach reminds me a little of the Hindley-Milner type system and its approach to inference, though maybe a little less formal and jargon-y 🥲 Considering it was the solution adopted by ML, Haskell, Rust and I'm sure many other languages, perhaps it's worth taking a closer look and seeing if there's anything we can learn from it as "Prior Art"? I'd imagine it might have some key design insights particularly when it comes to handling trait (typeclass) constraints and polymorphic types (Vec<T>, (T, U), etc).

@sezna
Copy link
Contributor

sezna commented Aug 17, 2022

The described approach reminds me a little of the Hindley-Milner type system and its approach to inference,

I actually based our current type engine on Hindley-Milner when I wrote it, hence the function names like unify etc. We are already using the HM algorithms to perform unification and inference.

emilyaherbert and others added 16 commits August 24, 2022 14:05
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
Co-authored-by: Alex Hansen <alex@alex-hansen.com>
@emilyaherbert
Copy link
Contributor Author

There hasn't been much activity on this RFC page, largely because I have been working on this prototype: https://github.com/emilyaherbert/declaration-engine-and-collection-context-demo/tree/master/de. The prototype should 1) help explain the idea and 2) acts as a guiding light for our implementation. Trait constraints for generic functions are working in the prototype.

Also, I've basically reworked the entire RFC, so I'd love a second round of feedback. In the meantime, I will hack on a experimental branch in the Sway repo so that we can get things moving. Don't let this scare you away from iterating on the design though!

@emilyaherbert emilyaherbert requested review from sezna and a team August 26, 2022 00:08
@otrho otrho requested a review from a team August 26, 2022 01:44
@otrho otrho requested a review from a team August 29, 2022 05:49
@emilyaherbert
Copy link
Contributor Author

ping @FuelLabs/sway-compiler would we be able to merge this?

@emilyaherbert emilyaherbert enabled auto-merge (squash) September 19, 2022 21:30
@emilyaherbert emilyaherbert requested a review from a team September 20, 2022 02:28
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@emilyaherbert emilyaherbert merged commit 5ee44c0 into master Sep 27, 2022
@emilyaherbert emilyaherbert deleted the emilyaherbert/declartion-engine branch September 27, 2022 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants