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

Declaring structs/enums in function scope #6

Closed
54k1 opened this issue Apr 13, 2021 · 2 comments
Closed

Declaring structs/enums in function scope #6

54k1 opened this issue Apr 13, 2021 · 2 comments

Comments

@54k1
Copy link
Contributor

54k1 commented Apr 13, 2021

I tried to perform the following:

fn f () {
    enum E {}
}

enum E {}

And, I got the following error message:
Error E: Name "E" is already in use at [5:5].

Should this be the behavior?
struct/enum declarations in a scope must become inaccessible when out of scope. (In this case, they are inaccessible, however, it won't let us declare another enum/struct with the same name)
The check for previously declared enum/struct is being done in Parser::check_if_declared. We should probably move it to the interpreter where the name resolution by scope is handled.

@tuqqu what do you think about this? What should be the behavior?

@tuqqu
Copy link
Owner

tuqqu commented Apr 13, 2021

Ideally, it should not be the behaviour, indeed.

Good catch actually.
The parser should not be handling enum/struct collisions. And it can't do it properly anyways.

Other name collisions are resolved on declaration in interpreter::env as they ought to be (constants, functions).
And it works as expected and this is allowed:

fn f() {
    const x = 10;
    fn y() {}
}
const x = 10;
fn y() {}

If I am not missing anything we need to add a similar check to define_struct and define_enum and remove the check_if_declared from the parser altogether.

While we are at it we can also fix the current confusing error messages of define_function and define_constant

const x = 10;
fn x() {}  //Error x: Trying to redefine function "x" at [2:3]. 

It really should be just Name "x" is already in use at [..].

@tuqqu
Copy link
Owner

tuqqu commented Apr 13, 2021

implemented in #7

@tuqqu tuqqu closed this as completed Apr 13, 2021
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