Restrict usages of type variables in non-generalized contexts

[ayazhafiz]

Type variables can only be used on functions (and in number literals as
a carve-out for now). In all other cases, a type variable takes on a
single, concrete type based on later usages. This check emits errors
when this is violated.

The implementation is to check the rank of a variable after it could be
generalized. If the variable is not generalized but annotated as a type
variable, emit an error.

Needs #7450

[lukewilliamboswell]

@ayazhafiz

#7450 - has merged now.

[smores56]

Great change for Roc! Things look good besides these comments and the failing tests. Let me know if you're busy, I can try to resolve stuff for you

[ayazhafiz]

@smores56 that would be great, thanks, I probably don’t have the time to land this right now. Regarding your first comment-yes it can happen, but it leads to a type mismatch in all cases i could think of so another error takes priority over it. I wouldn’t remove the branch just because there is some ordering though. Second comment - good call, i wasn’t sure how easy it is to remove but probably sufficient to make a constant NONE set to 0. Third comment - no issue needed it’s just an inappropriate model long term. l

…

On Sun, Jan 19, 2025 at 5:47 PM Sam Mohr ***@***.***> wrote: ***@***.**** commented on this pull request. Great change for Roc! Things look good besides these comments and the failing tests. Let me know if you're busy, I can try to resolve stuff for you ------------------------------ In crates/reporting/src/error/type.rs <#7454 (comment)>: > @@ -513,6 +513,38 @@ pub fn type_problem<'b>( severity, }) } + TypeIsNotGeneralized(region, actual_type, generalizable) => { + let doc = alloc.stack([ + alloc.reflow("This type variable has a single type:"), + alloc.region(lines.convert_region(region), severity), + if !generalizable.0 { + alloc.concat([ I notice that you didn't add a test case for this, and tried to come up with one myself. I don't know if this is possible to represent in current Roc. If you think it's possible to represent this in some future Roc (or in some way today that I don't know about), can we add a test case for it? Otherwise, maybe we should not be branching on generalization. ------------------------------ In crates/compiler/types/src/subs.rs <#7454 (comment)>: > @@ -50,10 +51,24 @@ impl fmt::Debug for Mark { } } -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] -pub enum ErrorTypeContext { - None, - ExpandRanges, +bitflags! { + pub struct ErrorTypeContext : u8 { + const NONE = 1 << 0; Doesn't seem like we want to preserve NONE as its own bitflags, we get that for free when the other flags are zero. ------------------------------ In crates/compiler/solve/src/solve.rs <#7454 (comment)>: > @@ -1636,6 +1617,30 @@ fn solve( state } +fn check_named_variables_are_generalized( + env: &mut InferenceEnv<'_>, + problems: &mut Vec<TypeError>, + named_variables: &[Loc<Variable>], + generalizable: Generalizable, +) { + for loc_var in named_variables { + let is_generalized = env.subs.get_rank(loc_var.value) == Rank::GENERALIZED; + if !is_generalized { + // TODO: should be OF_PATTERN if on the LHS of a function, otherwise OF_VALUE. How important is this TODO? If it's important, we should make an issue for tracking. — Reply to this email directly, view it on GitHub <#7454 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AE6GL6X4VZ6OCRNVBRTYCND2LQTQDAVCNFSM6AAAAABUQVCLL6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNRRGEYDMMBXGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[smores56]

Fixed the comments and the failing tests, should be good to go.