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

Use local module appropriate names when printing types in warnings #4164

Open
xhalo32 opened this issue Jan 9, 2025 · 4 comments
Open

Use local module appropriate names when printing types in warnings #4164

xhalo32 opened this issue Jan 9, 2025 · 4 comments
Labels
help wanted Contributions encouraged priority:medium

Comments

@xhalo32
Copy link

xhalo32 commented Jan 9, 2025

gleam@20fb6f41b

The following code suggests incorrect type for todo

pub fn curry(f: fn(a, b) -> c) -> fn(a) -> fn(b) -> c {
    fn(a: a) {
        todo
    }
}

producing a warning which suggests

[snip]
Hint: I think its type is `fn(a) -> c`.

which is obviously incorrect as todo should be replaced with a function of type fn(b) -> c.

Investigation

I have been digging in /compiler-core/_type/pretty.rs and added tests:

    assert_string!(
        fn_(
            vec![Arc::new(Type::Var {
                type_: Arc::new(RefCell::new(TypeVar::Generic { id: 78 })),
            })],
            fn_(
                vec![Arc::new(Type::Var {
                    type_: Arc::new(RefCell::new(TypeVar::Generic { id: 2 })),
                })],
                Arc::new(Type::Named {
                    args: vec![],
                    module: "whatever".into(),
                    package: "whatever".into(),
                    name: "Bool".into(),
                    publicity: Publicity::Public,
                    inferred_variant: None,
                }),
            ),
        ),
        "fn(a) -> fn(b) -> Bool",
    );

and

    assert_eq!(
        pretty_print(fn_(vec![], fn_(vec![], int()))),
        "fn() -> fn() -> Int"
    );

which both pass, so I guess the issue is not related to the pretty printer but rather happens somewhere in the type system.

Testing

I added the following snap test which currently fails

---
source: compiler-core/src/type_/tests/warnings.rs
expression: "pub fn main() {\n          todo()\n        }"
---
----- SOURCE CODE
pub fn curry(f: fn(a, b) -> c) -> fn(a) -> fn(b) -> c {
            fn(a: a) {
                todo
            }
        }

----- WARNING
warning: Unused variable
  ┌─ /src/warning/wrn.gleam:1:14
  │
1 │ pub fn curry(f: fn(a, b) -> c) -> fn(a) -> fn(b) -> c {
  │              ^^^^^^^^^^^^^^^^ This variable is never used

Hint: You can ignore it with an underscore: `_f`.

warning: Unused variable
  ┌─ /src/warning/wrn.gleam:2:16
  │
2 │             fn(a: a) {
  │                ^^^^ This variable is never used

Hint: You can ignore it with an underscore: `_a`.

warning: Todo found
  ┌─ /src/warning/wrn.gleam:3:17
  │
3 │                 todo
  │                 ^^^^ This code is incomplete

This code will crash if it is run. Be sure to finish it before
running your program.

Hint: I think its type is `fn(b) -> c`.
-old snapshot
+new results
────────────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   29    29 │ 
   30    30 │ This code will crash if it is run. Be sure to finish it before
   31    31 │ running your program.
   32    32 │ 
   33       │-Hint: I think its type is `fn(b) -> c`.
         33 │+Hint: I think its type is `fn(a) -> b`.

Further questions

The issue might be present with panic as well as the code follows similar logic as todo's code.

@xhalo32 xhalo32 added the bug Something isn't working label Jan 9, 2025
@xhalo32 xhalo32 changed the title todo produce incorrectly typed hints todo produces incorrectly typed hints Jan 9, 2025
@lpil
Copy link
Member

lpil commented Jan 9, 2025

Hello! I think this may not be a bug and that it's that these warnings do not display types in the context of the local module, but they happen to use the same names that you are using.

If I change the names it'll be more clear:

pub fn curry(f: fn(t1, t2) -> t3) -> fn(t1) -> fn(t2) -> t3 {
  fn(a: t1) { todo }
}
warning: Todo found
  ┌─ /Users/louis/Desktop/thingy/src/thingy.gleam:4:15
  │
4 │   fn(a: t1) { todo }
  │               ^^^^ This code is incomplete

This code will crash if it is run. Be sure to finish it before
running your program.

Hint: I think its type is `fn(a) -> b`.

That said, there could be something I'm missing. Could you share the code needed to reproduce what you've got please 🙏

@xhalo32
Copy link
Author

xhalo32 commented Jan 9, 2025

Wow you are absolutely right! Thanks for the speedy response :)

This threw me off guard, never would have expected it... As I'm teaching a course involving Gleam, I will take this into account and use type variables outside the first letters of the alphabet.

I think the hint could be improved to display the type variables that are actually used or use a notation such as typevar_1, etc. to indicate that don't refer to types in the actual code.

Do you want me to close this issue or change it to a feature request?

@lpil
Copy link
Member

lpil commented Jan 9, 2025

I agree, it could be made more clear. I think the ideal would be for the warnings to print types using the appropriate local syntax for a function!

@lpil lpil added help wanted Contributions encouraged priority:medium and removed bug Something isn't working labels Jan 9, 2025
@lpil lpil changed the title todo produces incorrectly typed hints Use local module appropriate names when printing types in warnings Jan 9, 2025
@GearsDatapacks
Copy link
Member

Yes I think when making error messages context-aware, I forgot to make warning messages context-aware

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Contributions encouraged priority:medium
Projects
None yet
Development

No branches or pull requests

3 participants