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

Type expansion #5559

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from
Draft

Type expansion #5559

wants to merge 19 commits into from

Conversation

AjaiKN
Copy link
Contributor

@AjaiKN AjaiKN commented Jun 17, 2023

First step in implementing refinement of tag unions

crates/compiler/can/src/constraint.rs Outdated Show resolved Hide resolved
crates/compiler/solve/tests/solve_expr.rs Outdated Show resolved Hide resolved
@@ -572,9 +572,37 @@ pub struct WhenBranchPattern {
pub degenerate: bool,
}

#[derive(Clone, Debug, Default)]
pub struct Refinements(VecMap<Symbol, (Variable, Variable)>);
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

What do you think about using a SmallVec here instead (there is one in roc_collections)? The hypothesis is that typically, we have 1 or 0 symbols to refine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds like a good idea!

LetAndExpandType(
Index<LetConstraint>,
Slice<Variable>,
Option<Box<Refinements>>,
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Is there a reason Refinements is boxed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I don't box it, one of the size assertions fails for Constraint:

error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
   --> crates/compiler/can/src/constraint.rs:731:1
    |
731 | roc_error_macros::assert_sizeof_aarch64!(Constraint, 3 * 8);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: source type: `Constraint` (512 bits)
    = note: target type: `[u8; 24]` (192 bits)
    = note: this error originates in the macro `static_assertions::assert_eq_size` which comes from the expansion of the macro `roc_error_macros::assert_sizeof_aarch64` (in Nightly builds, run with -Z macro-backtrace for more info)

Comment on lines +6697 to +6701
fn unique_symbol(ident_ids: &mut IdentIds, home: ModuleId) -> Symbol {
let ident_id = ident_ids.gen_unique();

Symbol::new(home, ident_id)
}
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

just fyi, there is a unique_symbol on the Env

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's what I originally did in refine_tag_union, but the borrow checker didn't like that I was both borrowing env.subs and mutably borrowing env. This unique_symbol() acts on IdentIds instead of Env so that I can mutably borrow only env.ident_ids.

Comment on lines +7 to +10
# ^ [Io Str]
Io "hi" -> Io "bye"
error -> error
# ^^^^^ [Io Str, Net Str]*
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

🔥

@github-actions
Copy link

Thank you for your contribution! Sometimes PRs end up staying open for a long time without activity, which can make the list of open PRs get long and time-consuming to review. To keep things manageable for reviewers, this bot automatically closes PRs that haven’t had activity in 60 days. This PR hasn’t had activity in 30 days, so it will be automatically closed if there is no more activity in the next 30 days. Keep in mind that PRs marked Closed are not deleted, so no matter what, the PR will still be right here in the repo. You can always access it and reopen it anytime you like!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants