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

rustc: reorganize driver, replace compile_upto with multiple more-obvious functions. #8077

Closed
wants to merge 1 commit into from

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Jul 27, 2013

The purpose here is to get rid of compile_upto, which pretty much always requires the user to read the source to figure out what it does. It's replaced by a sequence of obviously-named functions:

  • phase_1_parse_input(sess, cfg, input);
  • phase_2_configure_and_expand(sess, cfg, crate);
  • phase_3_run_analysis_passes(sess, expanded_crate);
  • phase_4_translate_to_llvm(sess, expanded_crate, &analysis, outputs);
  • phase_5_run_llvm_passes(sess, &trans, outputs);
  • phase_6_link_output(sess, &trans, outputs);

Each of which takes what it takes and returns what it returns, with as little variation as possible in behaviour: no "pairs of options" and "pairs of control flags". You can tell if you missed a phase because you will be missing a phase_N call to some N between 1 and 6.

It does mean that people invoking librustc from outside need to write more function calls. The benefit is that they can figure out what they're doing much more easily, and stop at any point, rather than further overloading the tangled logic of compile_upto.

@emberian
Copy link
Member

Yes, this is wonderful! Much much better.

bors added a commit that referenced this pull request Jul 27, 2013
The purpose here is to get rid of compile_upto, which pretty much always requires the user to read the source to figure out what it does. It's replaced by a sequence of obviously-named functions:

  - phase_1_parse_input(sess, cfg, input);
  - phase_2_configure_and_expand(sess, cfg, crate);
  - phase_3_run_analysis_passes(sess, expanded_crate);
  - phase_4_translate_to_llvm(sess, expanded_crate, &analysis, outputs);
  - phase_5_run_llvm_passes(sess, &trans, outputs);
  - phase_6_link_output(sess, &trans, outputs); 

Each of which takes what it takes and returns what it returns, with as little variation as possible in behaviour: no "pairs of options" and "pairs of control flags". You can tell if you missed a phase because you will be missing a `phase_N` call to some `N` between 1 and 6.

It does mean that people invoking librustc from outside need to write more function calls. The benefit is that they can _figure out what they're doing_ much more easily, and stop at any point, rather than further overloading the tangled logic of `compile_upto`.
@bors bors closed this Jul 27, 2013
@brson
Copy link
Contributor

brson commented Jul 28, 2013

Glad to see some long-needed front-end cleanup.

@huonw
Copy link
Member

huonw commented Jul 28, 2013

This appears to cause a >500MB memory use regression.

dotdash added a commit to dotdash/rust that referenced this pull request Jul 28, 2013
This fixes the recently introduced peak memory usage regression by
freeing the intermediate results as soon as they're not required
anymore instead of keeping them around for the whole compilation
process.

Refs rust-lang#8077
bors added a commit that referenced this pull request Jul 28, 2013
This fixes the recently introduced peak memory usage regression by
freeing the intermediate results as soon as they're not required
anymore instead of keeping them around for the whole compilation
process.

Refs #8077
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 6, 2021
…negatives, r=camsteffen

Fix some false negatives for [`single_char_pattern`]

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: Fix some false negatives for [`single_char_pattern`]

I noticed that clippy wasn't complaining about my usage of `split_once("x")` in a personal project so I updated the list of functions.

I had to update the test case for an unrelated issue because replace is now included in the list of functions to be linted.
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

Successfully merging this pull request may close these issues.

None yet

7 participants