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.

Sign up for GitHub

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

Jump to bottom

Add structs to the declaration engine #2688

Merged
merged 2 commits into from
Sep 9, 2022
Merged

Add structs to the declaration engine #2688

merged 2 commits into from
Sep 9, 2022

Conversation

tritao
Copy link
Contributor

@tritao tritao commented Sep 1, 2022 •
edited
Loading

@tritao tritao added the compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen label Sep 1, 2022
@tritao tritao self-assigned this Sep 1, 2022
@tritao tritao marked this pull request as ready for review September 1, 2022 15:56
emilyaherbert
emilyaherbert suggested changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@emilyaherbert emilyaherbert left a comment

Choose a reason for hiding this comment

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

So excited for this!

sway-core/src/semantic_analysis/ast_node/declaration.rs Outdated Show resolved Hide resolved
sway-core/src/semantic_analysis/ast_node/declaration.rs Outdated Show resolved Hide resolved
sway-core/src/semantic_analysis/ast_node/declaration.rs Show resolved Hide resolved
sway-core/src/semantic_analysis/ast_node/declaration.rs Outdated Show resolved Hide resolved
sway-core/src/semantic_analysis/ast_node/declaration.rs Outdated Show resolved Hide resolved
sway-core/src/semantic_analysis/storage_only_types.rs Outdated Show resolved Hide resolved
sway-core/src/type_system/mod.rs Outdated Show resolved Hide resolved
@tritao
Copy link
Contributor Author

tritao commented Sep 7, 2022

Rebased and addressed all comments.

emilyaherbert
emilyaherbert suggested changes Sep 8, 2022
View reviewed changes
Copy link
Contributor

@emilyaherbert emilyaherbert left a comment

Choose a reason for hiding this comment

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

More nits. Looking good though!

sway-core/src/semantic_analysis/storage_only_types.rs Outdated Show resolved Hide resolved
sway-core/src/type_system/type_binding.rs Outdated Show resolved Hide resolved
sway-lsp/src/capabilities/hover.rs Outdated Show resolved Hide resolved
sway-lsp/src/capabilities/hover.rs Outdated Show resolved Hide resolved
sway-lsp/src/core/traverse_typed_tree.rs Outdated Show resolved Hide resolved
sway-lsp/src/core/traverse_typed_tree.rs Outdated Show resolved Hide resolved
@emilyaherbert emilyaherbert requested a review from a team September 8, 2022 17:33
@tritao tritao marked this pull request as draft September 9, 2022 13:01
@tritao
Copy link
Contributor Author

tritao commented Sep 9, 2022

Rebased and addressed the nits, but temporarily in draft until #2713 is merged.

@tritao tritao marked this pull request as ready for review September 9, 2022 17:25
@emilyaherbert emilyaherbert self-requested a review September 9, 2022 17:40
mohammadfawaz
mohammadfawaz approved these changes Sep 9, 2022
View reviewed changes
Copy link
Contributor

@mohammadfawaz mohammadfawaz left a comment

Choose a reason for hiding this comment

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

:shipit:

@mohammadfawaz mohammadfawaz merged commit 719dca1 into FuelLabs:master Sep 9, 2022
Centril
Centril reviewed Sep 9, 2022
View reviewed changes
sway-core/src/type_system/type_binding.rs
errors
);

// monomorphize the copy, in place
Copy link
Contributor

Choose a reason for hiding this comment

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

So the type system is doing monomorphization?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean, but it's working the same way as it used to, though its likely this will change once we add the monomorphization cache.

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is type_check_with_ident is part of the type checker, so I am surprised that monomorphization, AIUI, is happening here, in the type checker. This, as opposed to merely type checking and then saving what the substitutions are for later monomorphization in a code-gen phase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril left review comments

@mohammadfawaz mohammadfawaz mohammadfawaz approved these changes

@emilyaherbert emilyaherbert emilyaherbert approved these changes

Assignees

@tritao tritao

Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tritao @Centril @emilyaherbert @mohammadfawaz