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

Function Calls are parsed as StructExpr #25

Closed
philberty opened this issue May 17, 2020 · 11 comments · Fixed by #26
Closed

Function Calls are parsed as StructExpr #25

philberty opened this issue May 17, 2020 · 11 comments · Fixed by #26
Assignees
Labels

Comments

@philberty
Copy link
Member

When there is a statement such as:

let x = test(1);

This is parsed as:

Outer attributes: none
 let x = outer attributes: none
  StructExpr:          
   PathInExpr:      
   test(1)                            
   inner attributes:none

I am unsure if this is intentional of not but we can work around this if its intended.

@bjorn3
Copy link

bjorn3 commented May 17, 2020

It should be the exact opposite way:

struct Abc(u8);
let foo = Abc(0);

should have Abc(0) parsed as function call. For tuple structs the constructor is just a regular function. For example let _: fn(u8) -> Abc = Abc; is allowed.

@SimplyTheOther
Copy link
Member

Yep, bjorn3 is correct, the “tuple struct expression” is equivalent to parsing it as a function call to a constructor - I did not read the relevant reference page properly. As such, the tuple struct expression could be removed from the AST or could be designed to not be generated in normal parsing (and only exist after conversion from a method call after name resolution or something).

However, there remains one problem related to this - if structs and functions have separate namespaces (i.e. you can define a struct and a function in the same scope with the same name and presumably no shadowing occurs), as suggested here, then how should this constructor vs function ambiguity be dealt with? I’m not sure how rustc does it, and I don’t know if there are more complicated rules to how the syntax should be parsed that we currently don’t know about.

@philberty
Copy link
Member Author

I guess its down to the arguments passed to the function call to help determine it in that case but it doesn't handle the case where arguments are the same for the struct and call maybe calls are just given preference over structs is about the only way i can think.

@bjorn3
Copy link

bjorn3 commented May 18, 2020

struct Abc(u8);

fn Abc(_: u8) {}

fn main() {
    Abc(0);
}
error[E0428]: the name `Abc` is defined multiple times
 --> src/main.rs:3:1
  |
1 | struct Abc(u8);
  | --------------- previous definition of the value `Abc` here
2 | 
3 | fn Abc(_: u8) {}
  | ^^^^^^^^^^^^^ `Abc` redefined here
  |
  = note: `Abc` must be defined only once in the value namespace of this module

I think that reddit post was refering to something like

struct Abc { a: u8 }

fn Abc(_: u8) {}

fn main() {
    Abc(0);
}

in which case the struct constructor is not in the value namespace, so there is no ambiguity.

@philberty
Copy link
Member Author

I just tried this with rustc:

struct test {
    x: i32
}

fn test(x: i32) {
   
}

fn main() {
    let x = test(1);
}

and it gives:

warning: type `test` should have an upper camel case name
 --> test2.rs:1:8
  |
1 | struct test {
  |        ^^^^ help: convert the identifier to upper camel case: `Test`
  |
  = note: `#[warn(non_camel_case_types)]` on by default

warning: unused variable: `x`
 --> test2.rs:5:9
  |
5 | fn test(x: i32) {
  |         ^ help: consider prefixing with an underscore: `_x`
  |
  = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `x`
  --> test2.rs:10:9
   |
10 |     let x = test(1);
   |         ^ help: consider prefixing with an underscore: `_x`

warning: struct is never constructed: `test`
 --> test2.rs:1:8
  |
1 | struct test {
  |        ^^^^
  |
  = note: `#[warn(dead_code)]` on by default

So in this case the function is given preference

@bjorn3
Copy link

bjorn3 commented May 18, 2020

That is because

struct test {
    x: i32
}

fn main() {
    let x = test(1);
}

is not allowed. Only tuple structs can be used with StructName(...). Structs with named fields like test in this example can't be used this way.

@philberty
Copy link
Member Author

Good point my bad!

@NalaGinrut
Copy link
Contributor

NalaGinrut commented May 18, 2020

@philberty I think that's because we haven't done the name resolution before you start to work on type resolution. There is a similar issue I encountered in the macro expansion. BTW, it's better to make two files for type resolution and name resolution, if our single file keeps growing, my editor will halt each time when saving-and-indenting, and not good to maintain. If you don't mind, I'll rename the current rust-resolution.cc to rust-type-resolution, and work on name resolution. ;-)

@philberty
Copy link
Member Author

Yes please @NalaGinrut sorry my bad there on the naming.

@philberty
Copy link
Member Author

I think this was wrongly closed with the linking from commits

@philberty philberty reopened this Dec 1, 2020
@philberty philberty added this to the Core Datastructures milestone Dec 21, 2020
@philberty
Copy link
Member Author

this is fixed with #26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants