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

Functions with an immediate return value shouldn't have an extra argument #6575

Closed
catamorphism opened this issue May 18, 2013 · 9 comments
Closed
Labels
A-codegen Area: Code generation

Comments

@catamorphism
Copy link
Contributor

as per comment in trans::type_of::type_of_fn

@thomaslee
Copy link
Contributor

@catamorphism I'm going to try to figure this one out. Any advice on where I should be looking outside of type_of::type_of_fn? I'm guessing I need to update call sites (so maybe middle::trans::callee.rs?), & I'm guessing there may be some sort of issue wrt argument indices being shifted around or something like that -- beyond that I think I'm flying blind :)

@catamorphism
Copy link
Contributor Author

Yup, you'll need to update Various Code In Trans (tm). If I were going to do this, I would just change it in one place and see what breaks -- lots of tests should fail catastrophically in that case :-) But that may not be the best way. I suspect one thing that would be just as helpful as fixing the bug is adding documentation as you go along to the code that deals with args.

@thomaslee
Copy link
Contributor

If I were going to do this, I would just change it in one place and see what breaks -- lots of tests should fail catastrophically in that case

I totally would if I could get the build to do anything but segfault with this change :) All good, I think I'm just going to have to do a bit of whack-a-mole.

@catamorphism
Copy link
Contributor Author

Yes, unfortunately the way the build works is not the best thing for testing -- there are easier test cases you could be running than the entire compiler itself, but make check will build the stage2 compiler before any test cases! :-) What I like to do instead is just try compiling run-pass test cases individually, by hand, with the stage1 compiler, until I find one that's simple enough and that exhibits the crash. Let me know if you'd like me to expand on that or if it's clear...

@thomaslee
Copy link
Contributor

Not sure I follow -- doesn't that leave me in the position of needing to build the stage1 compiler? This seems to be falling over in stage0: https://gist.github.com/thomaslee/5617386

(Must say I've not spent a whole lot of time trying to understand the bootstrapping thing -- my laziness may be my undoing here :P)

That said, I think I'm making a little progress -- just stumbled across rustc::middle::trans::common::first_real_arg, which I believe is going to be at the root of a lot of the hurtiness associated with this issue. That aside, would love clarification wrt using the stage1 compiler to test this if you still think it's relevant!

@catamorphism
Copy link
Contributor Author

Ah, I see what's confusing. If you build rustc-stage1, that will build the stage1 compiler and also build libcore and libstd with the stage1 compiler. But libcore is, like the compiler itself, a pretty challenging test case! So for a "break the whole compiler and put it together again" (I'm exaggerating somewhat) change like this, you want to find test cases that don't depend on any libraries, not even core -- or just write one, all you need is a function that returns an immediate value like an int -- and compile them with the stage1 compiler (which exists, even though the step after it, building libcore with the stage1 compiler, failed) and see what happens.

Hopefully that's a little clearer...

@thomaslee
Copy link
Contributor

Awesome, that helps a whole lot. Thanks very much!

@thomaslee
Copy link
Contributor

I've managed to get the whole toolchain through stage 1 with a few hacks with this WIP:

thomaslee@7c27273

Still seems to need some work: it's crashing hard building libcore at stage 2 & there's some nasty hacks in there to work around the fact dtors somehow get either one or both of the implicit parameters.

stage 2 borkage looks like this:

https://gist.github.com/thomaslee/5625937

Slow going, but there's some definite progress here.

thomaslee added a commit to thomaslee/rust that referenced this issue May 24, 2013
This lets us use #ifdefs to determine which stage of the build we happen
to be in, which is useful in the event we need to make changes to rustrt
that are incompatible with the code generated by stage0.

This should help pave the way to completing rust-lang#6575, which will likely
require changes to type signatures for spawn_fn & glue_fn in rustrt.
bors added a commit that referenced this issue May 29, 2013
Fix for #6575. In the trans phase, rustc emits code for a function parameter that goes completely unused in the event the return type of the function in question happens to be an immediate.

This patch modifies rustc & parts of rustrt to ensure that the vestigial parameter is no longer present in compiled code.
@pnkfelix
Copy link
Member

Fixed by commit e3d0c1e

flip1995 pushed a commit to flip1995/rust that referenced this issue Jan 15, 2021
Change env var used for testing Clippy

This changes the variable used for testing Clippy in the internal test
suite:

```
CLIPPY_TESTS -> __CLIPPY_INTERNAL_TESTS
```

`CLIPPY_TESTS` is understandably used in environments of Clippy users,
so we shouldn't use it in our test suite.

changelog: Fix oversight which caused Clippy to lint deps in some environments.

Once again fixes rust-lang/rust-clippy#3874
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation
Projects
None yet
Development

No branches or pull requests

3 participants