2

Github Remove dead code in `rustc_session::Options` by jyn514 · Pull Request #84...

 3 years ago
source link: https://github.com/rust-lang/rust/pull/84802
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

New issue

Remove dead code in rustc_session::Options #84802

Conversation

Copy link

Member

jyn514 commented 6 days ago •

edited by Mark-Simulacrum
  • Don't recompile the same functions for each debugging option
    This reduces the amount of items in the crate by quite a lot.
  • Remove unused parse_opt_list and parse_pathbuf_push functions
  • Remove unused macro parameters
  • Remove allow(dead_code).

Copy link

Collaborator

rust-highfive commented 6 days ago

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

Copy link

Member

Author

jyn514 commented 6 days ago

I expect this should help bootstrap time for rustc_session quite a bit (other crates shouldn't be affected).

@bors try @rust-timer queue

Copy link

Collaborator

rust-timer commented 6 days ago

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

Copy link

Contributor

bors commented 6 days ago

hourglass Trying commit 2828dec with merge 5ce9808b3b5bfb44d46efe3fe2d5d152fd51f39a...

Copy link

Member

Author

jyn514 commented 6 days ago

Err ... I guess there's not a way to cancel an ongoing try run, and I don't know a way to tell rust-timer to queue a commit that isn't the latest.

@bors try @rust-timer queue

Copy link

Collaborator

rust-timer commented 6 days ago

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

Copy link

Contributor

bors commented 6 days ago

hourglass Trying commit b19c02c with merge a64224eaa14138ddc834a78d854d070df9154409...

Copy link

Member

Mark-Simulacrum left a comment

Overall looks fine to me, presuming perf is happy.

pub const parse_wasi_exec_model: &str = "either `command` or `reactor`";

pub const parse_split_debuginfo: &str =

"one of supported split-debuginfo modes (`off` or `dsymutil`)";

&[ $( (stringify!($opt), crate::options::parse::$opt, $crate::options::desc::$parse, $desc) ),* ];

Mark-Simulacrum 6 days ago

Member

nit: one of these uses $crate, the other does not. Doesn't matter in practice as it's crate local (I think) but let's be consistent.

jyn514 6 days ago

Author

Member

Sure thing - I made this change locally, but I don't want to push since it will cancel the perf run.

diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs
index 735188768e4..cd28517bfbc 100644
--- a/compiler/rustc_session/src/options.rs
+++ b/compiler/rustc_session/src/options.rs
@@ -286,7 +286,7 @@ fn dep_tracking_hash(&self, _for_crate_hash: bool, error_format: ErrorOutputType
 
     pub type $setter_name = fn(&mut $struct_name, v: Option<&str>) -> bool;
     pub const $stat: &[(&str, $setter_name, &str, &str)] =
-        &[ $( (stringify!($opt), crate::options::parse::$opt, $crate::options::desc::$parse, $desc) ),* ];
+        &[ $( (stringify!($opt), $crate::options::parse::$opt, $crate::options::desc::$parse, $desc) ),* ];
 
     // Sometimes different options need to build a common structure.
     // That structure can kept in one of the options' fields, the others become dummy.

Copy link

Contributor

bors commented 6 days ago

sunny Try build successful - checks-actions
Build commit: a64224e (a64224eaa14138ddc834a78d854d070df9154409)

Copy link

Collaborator

rust-timer commented 6 days ago

Copy link

Collaborator

rust-timer commented 5 days ago

Finished benchmarking try commit (a64224e): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

Copy link

Member

Author

jyn514 commented 5 days ago

It looks like the time for rustc_session went ... up? confused the compiler should be doing strictly less work, maybe it's just noise?

Likely noise. Dead code doesn't usually influence performance too much as it's not generally codegen'd, and rustc itself is pretty fast. This still seems like an improvement overall to me, though, at least in terms of readability. @bors r+

Copy link

Contributor

bors commented 5 days ago

pushpin Commit b19c02c has been approved by Mark-Simulacrum

Copy link

Contributor

bors commented 5 days ago

hourglass Testing commit b19c02c with merge 89ebad5...

Copy link

Contributor

bors commented 5 days ago

sunny Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 89ebad5 to master...

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

None yet

Milestone

1.54.0

Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants

report

Recommend

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK