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

foo(...)->f as b should be tail callable if both are aliases of the same type #1215

Closed
boggle opened this issue Nov 23, 2011 · 6 comments
Closed

Comments

@boggle
Copy link
Contributor

boggle commented Nov 23, 2011

Imagine a tail call { be foo() as int } where foo() returns an i32 and on the current arch it is known that integers are 32 bit wide.

In this case, it should be ok to drop the cast and tail-call foo().

The idea for this came up when rewriting std::math, cf

https://github.com/boggle/rust/blob/libmath/src/lib/math.rs

@boggle
Copy link
Contributor Author

boggle commented Nov 26, 2011

This should work for imports of functions, consts, and types, and in const definitions, too.

The general rule would be that a cast (expr: a) as b is dropped (treated as if not there while the type conversion still happens implicitly), iff the cast is valid and both types have the same shape and alignment (i.e. no actual casting code needs to be generated). This should at least hold for primitive types, tuples, function types, and record types.

In std::math it would allow to import a fn sin(n: f32) -> f32 like import f32::sin as (float)->float (as soon as casting of function types is supported) and const e: float = f32::consts::e as float which would make everything so much more concise and reduce code duplication.

@boggle
Copy link
Contributor Author

boggle commented Nov 26, 2011

Implementing this requires modifying the parser and dropping cast expressions after type checking. This either needs an extra pass or special treatment of import, const, and be, during trans.

Please give feedback on the right approach to implement this.

@brson
Copy link
Contributor

brson commented Nov 28, 2011

This depends on #217. As far as I know tail calls simply don't work in Rust.

@boggle
Copy link
Contributor Author

boggle commented Nov 28, 2011

Let's say, they would. What would be right way to approach this (or answer for import, const). Do I really need to add another pass just for this? Is there anything to watch out for when changing the ast just before trans?

@brson
Copy link
Contributor

brson commented Nov 30, 2011

I was focused on tail calls and didn't notice that this is mostly about treating machine types as ints, etc, which would obviously be nice.

I would start by just adding a new method to ty, like are_trivially_castable, then using that in the few analysis passes where this might matter (typechecking for be, const checking) and then in the corresponding places in trans (we have a lot of these little situations where the analysis and translation are completely disconnected and just make the same assumptions and rely on tests to ensure correctness).

The const case seems like a no-brainer - we can just support whatever constant scalar casting operations that the LLVMConst* functions get us, which is probably a lot more than just casts of the same machine type. Needs changes to middle::check_const and middle::trans::trans_const_expr.

Changing import like that probably requires further discussion with people other than me. On first glance I wonder if there's some other solution. Putting the 'as' keyword into a non-expression context is a big step, though the implementation would probably just be a matter of changing the parser and type checker.

@boggle
Copy link
Contributor Author

boggle commented Dec 1, 2011

Did this for "be" now, as you suggested. Consts coming up next, don't pull yet, will rebase:

https://github.com/boggle/rust/compare/castaway

boggle added a commit to boggle/rust that referenced this issue Dec 2, 2011
boggle added a commit to boggle/rust that referenced this issue Dec 2, 2011
graydon pushed a commit that referenced this issue Dec 2, 2011
graydon pushed a commit that referenced this issue Dec 2, 2011
@boggle boggle closed this as completed Dec 2, 2011
coastalwhite pushed a commit to coastalwhite/rust that referenced this issue Aug 5, 2023
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