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

[rs2bril] Duplicate variable names with different types compiled incorrectly. #341

Open
oflatt opened this issue Oct 22, 2024 · 6 comments

Comments

@oflatt
Copy link
Contributor

oflatt commented Oct 22, 2024

Rust files that re-use a name with a different type cause the rs2bril compiler to generate ill-typed bril.

Example:

fn main() {
  let x: f64 = 2.0;
  let x: i64 = 2;
}

@Pat-Lafon
Copy link
Contributor

What bril code would you expect this to generate?

@main() {
  x: float = const 2.0;
  x: int = const 2;
}

We could generate this, but:

❯ bril2json < dupe.bril | brilck
new type int for x conflicts with old type float

(This might be a limitation of the type checking of brilck/brilirs to be fair)

@oflatt
Copy link
Contributor Author

oflatt commented Oct 23, 2024

I wasn't sure if it's a limitation of type checking or an intended restriction.
One way to get around this is to generate fresh names for each x

@Pat-Lafon
Copy link
Contributor

Hmm, it could work, but note that fresh names are tricky because I will now need to construct a control flow graph to understand what to re-write later uses of x to. Maybe one can show in most/all well-typed rust programs that one x dominates the other?

Somewhat unrelated but a fun example of bril typing that is probably not well supported(except by brili)

@main(cond: bool) {
  br cond .there .here;
.here:
  x: int = const 2;
  jmp .end;
.there:
  x: bool = const false;
.end:
  print x;
}

@oflatt
Copy link
Contributor Author

oflatt commented Oct 24, 2024

Could we do the renaming over the rust ast?

Alternatively a lightweight solution for now that would be a lot better is to throw an error

Pat-Lafon added a commit to Pat-Lafon/bril that referenced this issue Oct 24, 2024
@Pat-Lafon
Copy link
Contributor

Hmm, I've punted and taken your solution of just throwing an error. Hopefully this isn't too limiting in terms of what rust programs are not accepted. There are probably better solutions.

@oflatt
Copy link
Contributor Author

oflatt commented Oct 25, 2024

I'm perfectly happy with that solution, thanks for putting so much effort into improving it!

Pat-Lafon added a commit to Pat-Lafon/bril that referenced this issue Nov 4, 2024
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