Skip to content

Add a dead code elimination pass #1749

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

Closed
catamorphism opened this issue Feb 3, 2012 · 8 comments
Closed

Add a dead code elimination pass #1749

catamorphism opened this issue Feb 3, 2012 · 8 comments
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@catamorphism
Copy link
Contributor

Add a pass that warns about non-exported functions that are never called.

@catamorphism
Copy link
Contributor Author

as per IRC conversation w/ nmatsakis: The only trick is that many functions are "accidentally" exported due to the "export all by default" rule. One solution is to warn anyway if a function in a module with no export decls is unused (within the crate). Adding explicit export decls suppresses the warning.

@brson
Copy link
Contributor

brson commented Feb 3, 2012

See also #1702

@graydon
Copy link
Contributor

graydon commented Feb 3, 2012

We've discussed in the past inverting the "everything exported by default" rule at crate top-level. That is: only export explicit stuff for linking. The intra-crate is intended to make code less chatty and bureaucratic, but linking is often the appropriate time for bureaucracy.

Also note: if you have an explicit export list from a crate, you have a lot fewer roots to run your dead-code analysis from.

@metajack
Copy link
Contributor

Somewhat related is #2953

@Aatch
Copy link
Contributor

Aatch commented Aug 13, 2013

Visiting for triage. There have been lots of changes to privacy and similar, but I think this is still true.

@ktt3ja
Copy link
Contributor

ktt3ja commented Nov 7, 2013

I'm thinking of working on this for a class project but I'm unfamiliar with the internal of rustc. tjc has told me to focus on adding warning for now, and then work on eliminating dead code if I still have time, so I have been looking inside librustc/middle to figure out what to do. Since this issue is about adding a new pass and (I presume) the logic is similar to the liveness analysis in librustc/middle/liveness.rs, does that mean I'm supposed to create a new file in librustc/middle/ (say foo.rs) with a top-facing pub fn check_crate that does something similar to liveness.rs, and then call middle::foo::check_crate inside librustc/driver/driver.rs? Also, is there difficulty in checking for unused non-exported functions inside the existing liveness.rs rather than doing a separate pass?

(sorry if the above are stupid questions)

bors added a commit that referenced this issue Dec 8, 2013
PR for issue #1749 mainly to get some feedback and suggestion. This adds a pass that warns if a function, struct, enum, or static item is never used. For the following code,

```rust
pub static pub_static: int = 0;
static priv_static: int = 0;
static used_static: int = 0;

pub fn pub_fn() { used_fn(); }
fn priv_fn() { let unused_struct = PrivStruct; }
fn used_fn() {}

pub struct PubStruct();
struct PrivStruct();
struct UsedStruct1 { x: int }
struct UsedStruct2(int);
struct UsedStruct3();

pub enum pub_enum { foo1, bar1 }
enum priv_enum { foo2, bar2 }
enum used_enum { foo3, bar3 }

fn foo() {
	bar();
	let unused_enum = foo2;
}

fn bar() {
	foo();
}

fn main() {
	let used_struct1 = UsedStruct1 { x: 1 };
	let used_struct2 = UsedStruct2(1);
	let used_struct3 = UsedStruct3;
	let t = used_static;
	let e = foo3;
}
```

it would add the following warnings:

```rust
/home/ktt3ja/test.rs:2:0: 2:28 warning: code is never used: `priv_static`, #[warn(dead_code)] on by default
/home/ktt3ja/test.rs:2 static priv_static: int = 0;
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ktt3ja/test.rs:6:0: 6:48 warning: code is never used: `priv_fn`, #[warn(dead_code)] on by default
/home/ktt3ja/test.rs:6 fn priv_fn() { let unused_struct = PrivStruct; }
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ktt3ja/test.rs:10:0: 10:20 warning: code is never used: `PrivStruct`, #[warn(dead_code)] on by default
/home/ktt3ja/test.rs:10 struct PrivStruct();
                        ^~~~~~~~~~~~~~~~~~~~
/home/ktt3ja/test.rs:16:0: 16:29 warning: code is never used: `priv_enum`, #[warn(dead_code)] on by default
/home/ktt3ja/test.rs:16 enum priv_enum { foo2, bar2 }
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ktt3ja/test.rs:19:0: 22:1 warning: code is never used: `foo`, #[warn(dead_code)] on by default
/home/ktt3ja/test.rs:19 fn foo() {
/home/ktt3ja/test.rs:20 	bar();
/home/ktt3ja/test.rs:21 	let unused_enum = foo2;
/home/ktt3ja/test.rs:22 }
/home/ktt3ja/test.rs:24:0: 26:1 warning: code is never used: `bar`, #[warn(dead_code)] on by default
/home/ktt3ja/test.rs:24 fn bar() {
/home/ktt3ja/test.rs:25 	foo();
/home/ktt3ja/test.rs:26 }
```

Furthermore, I would like to solicit some test cases since I haven't tested extensively and I'm still unclear about some of the things in here. For example, I'm not sure how reexports would affect this and just assumed that LiveContext (which is a copy of reachable::ReachableContext) does enough work to handle it. Also, the test case above doesn't include any impl or methods, etc.
@kud1ing
Copy link

kud1ing commented Dec 9, 2013

Can be closed?

@alexcrichton
Copy link
Member

It can indeed, thanks @ktt3ja!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

8 participants