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

typechecker: Type check generic functions independently of their use #1389

Open
fahrradflucht opened this issue Feb 23, 2023 · 8 comments
Open

Comments

@fahrradflucht
Copy link
Contributor

fahrradflucht commented Feb 23, 2023

It looks to me like we aren't checking generic functions "on their own". Instead, we only check them with type variables filled in at their call sites. This leads to quite strange error reporting and in some cases even C++ that doesn't compile.

Currently, writing this is not an error:

fn to_minus_42<T>(anon a: T) -> T {
    return -42
}

fn main() {
    to_minus_42(-12)
}

However, this is:

fn to_minus_42<T>(anon a: T) -> T {
    return -42
}

fn main() {
    to_minus_42(12 as! u32)
}

Also notice where it is reported:

Error: Type mismatch: expected ‘u32, but got ‘i64’
───┬─ src/main.jakt:2:5
 1fn to_minus_42<T>(anon a: T) -> T { 
 2return -42 
   │     ─────────┬
   │              ╰─ Type mismatch: expected ‘u32, but got ‘i643} 
───┴─

Lastly, consider this case, which will type check but doesn't compile:

fn unwrap<T>(anon a: Optional<T>) -> T {
    return None
}

fn main() {
    unwrap(Some(42))
}
@alimpfard
Copy link
Member

Skipping the last one as it's likely because of a typecheck weirdness regarding return types (are we actually checking them?), I don't see what's wrong with the to_minus_42 example you have - T is inferred as u32, and -42 is inferred to be an i64 as it doesn't really get a direct type hint here

@fahrradflucht
Copy link
Contributor Author

fahrradflucht commented Feb 23, 2023

Hm, interesting. Maybe I'm just primed differently by other languages, but the same would not pass type checking in:

But maybe the desire for a generic function to be "correct in abstract" is not as universal as I thought.


Edit:

Actually now that I think about it, imagine the same scenario across library boundaries. Wouldn't this be super weird? You call a generic function and suddenly there is a type error in the library?

@alimpfard
Copy link
Member

We initially did check the generic function on its own (we still do), but not everything can be resolved like that since we're somewhat closer to C++ in this; e.g. comptime expressions depending on the concrete type (reflect T.name()) or 10 as! T. So now we check the function, and ignore errors in it unless they're definitely erroneous.

@alimpfard
Copy link
Member

Re the error placement, yeah that could use some work, for instance, we could give you a trace of why the call happened.
It comes with the yolo model of generics :P

@alimpfard
Copy link
Member

As a sidenote, I do think we should check things that are actually constrained somehow, by traits or other requirements, and make them invariants inside the generic function.

@fahrradflucht
Copy link
Contributor Author

Skipping the last one as it's likely because of a typecheck weirdness regarding return types (are we actually checking them?)

Just for reference, this doesn't compile - so something is specifically wrong about returning None there, it is not that we aren't checking at all:

fn to_minus_42<T>(anon a: Optional<T>) -> T {
    return "String"
}

fn main() {
    to_minus_42(Some(42))
}

@alimpfard
Copy link
Member

Does returning None in a normal function that's supposed to return...say, i32 cause an error?

@fahrradflucht
Copy link
Contributor Author

fahrradflucht commented Feb 23, 2023

Does returning None in a normal function that's supposed to return...say, i32 cause an error?

Ah thats it 😅 Even this typechecks:

fn please_not_none(anon a: i64) -> i64 {
    return None
}

fn main() {
    please_not_none(42)
}

I created #1390 let this issue be about generic return type checking / error reporting and not this None issue.

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

No branches or pull requests

2 participants