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

feat(forge flatten): add dead code elimination #2266

Open
sakulstra opened this issue Jul 11, 2022 · 4 comments
Open

feat(forge flatten): add dead code elimination #2266

sakulstra opened this issue Jul 11, 2022 · 4 comments
Labels
C-forge Command: forge Cmd-forge-flatten Command: forge flatten P-low Priority: low T-feature Type: feature

Comments

@sakulstra
Copy link
Contributor

Component

Forge

Describe the feature you would like

When using some libraries, the --flatten output will be gigantic as flatten will inline all imports. It would be neat if flatten would detect & remove unused imports.

When using re-export index files like here https://github.com/bgd-labs/aave-address-book/blob/main/src/AaveAddressBook.sol that makes it essentially impossible to use in foundry as the generated code will be unusable long.

Additional context

In addition to what i mentioned above i guess it might make sense to even eliminate unused code in the actually imported files. Not sure sure about this though.

@gakonst
Copy link
Member

gakonst commented Jul 11, 2022

Good catch.

@onbjerg onbjerg added C-forge Command: forge P-low Priority: low labels Jul 14, 2022
@onbjerg onbjerg added this to Foundry Jul 14, 2022
@onbjerg onbjerg moved this to Todo in Foundry Jul 14, 2022
@onbjerg onbjerg added the Cmd-forge-flatten Command: forge flatten label Aug 1, 2022
@sakulstra
Copy link
Contributor Author

@gakonst just realized the same is true for non flattened files.

When you have a file like:

// index.sol
import {A} from "a.sol";
import {B} from "b.sol";
import {C} from "c.sol";
// contract.sol
import {A, B} from "index.sol";

contract Contract {
  function test() public {
    A.noop();
  }
}

When I verify this contract it will include A, B and C although only A is used.

@gakonst
Copy link
Member

gakonst commented Oct 5, 2022

We'd need to do deeper parsing of the dependency graph for this kind of flattening I think?

@pegahcarter
Copy link
Contributor

It's probably worth an additional flag as the automatic removal of unused imports may confuse some developers.

I believe this would be a valuable feature as we'll see more utility in index files, as shown by protocols in addition to aave.

The dependency graph only grows as protocols build across many chains, requiring index files.

@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@zerosnacks zerosnacks changed the title Dead code elimination in flatten feat(forge flatten): add dead code elimination Aug 1, 2024
@zerosnacks zerosnacks removed this from the v1.0.0 milestone Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-flatten Command: forge flatten P-low Priority: low T-feature Type: feature
Projects
Status: Todo
Development

No branches or pull requests

5 participants