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

Suggestion when assigning enum to bit_set #4394

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

seventh-chord
Copy link
Contributor

This is something I ran into when writing some odin again recently, after not having used the language for >1 year.

Incorrect code:

MyEnum :: enum { A, B, C }
bits: bit_set[MyEnum]
bits += .A

Old error:

C:/dev/tools/Odin/examples/bad_errors.odin(39:14) Error: Invalid type 'bit_set[MyEnum]' for implicit selector expression '.A'
        bits += .A
                 ^

New error:

C:/dev/tools/Odin/examples/bad_errors.odin(39:14) Error: Cannot convert enum value to 'bit_set[MyEnum]'
        bits += .A
                 ^
        Suggestion: Did you mean '{ .A }'?

@seventh-chord
Copy link
Contributor Author

Another error message which has tripped me up is for this code:

ambiguous_call :: proc() {
    p0 :: proc(x: int) {}
    p1 :: proc(x: string) {}
    my_group :: proc{p0, p1}
    my_group(does_not_exist)
}

I get:

C:/dev/tools/Odin/examples/bad_errors.odin(23:5) Error: Ambiguous procedure group call 'my_group' that match with the given arguments
        my_group(does_not_exist)
        ^~~~~~~^
        Given argument types: (invalid type)
        p0 :: proc(x: int) at C:/dev/tools/Odin/examples/bad_errors.odin(20:5)
        p1 :: proc(x: string) at C:/dev/tools/Odin/examples/bad_errors.odin(21:5)
C:/dev/tools/Odin/examples/bad_errors.odin(23:14) Error: Undeclared name: does_not_exist
        my_group(does_not_exist)
                 ^~~~~~~~~~~~~^

But I find having the "ambiguous proc group" error first confusing, since the mistake I made in the code was just typing in a wrong variable name.

I was looking at adding an early out to check_call_arguments_proc_group if any of the arguments had type == t_invalid, but I would have to spend some more time understanding how that function works to be confident that I don't break anything else.

If you think changing this case to remove the "ambiguous proc group" error then I can look into that.

@gingerBill
Copy link
Member

Please keep these requests separate since they are quite different.

However the first one is possible to add and something I have considered before. The second one is not at all simple. The second one might be "confusing" but it's because people never read the error message.

If you can suggest something actually better for the error message itself (not removing it), then please do. I am rarely sure how to improve an error message, especially when people don't give me good suggestions.

@seventh-chord
Copy link
Contributor Author

That's fine, I'll keep thinking about the "ambiguous proc group" error.

About the first suggestion, anything in particular you'd like me to change about this PR?

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.

2 participants