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

Remove the unused &mut dependency_graph throughout sway-core #826

Merged
merged 3 commits into from
Feb 22, 2022

Conversation

mitchmindtree
Copy link
Contributor

@mitchmindtree mitchmindtree commented Feb 22, 2022

While working on #825 I noticed that that the dependency_graph is
unused throughout sway-core. I think the reason why cargo doesn't emit
any warnings about this is that at one point, it was stored in the public
TypeCheckArgument struct, so cargo believes that it is intentionally
being re-exported, however in reality we don't use it anywhere else in
the fuel/sway/forc ecosystem.


Edit: Oh also, It's not clear in the diff, but the source_map.rs file change just removes executable permissions from the file so that the file permissions match the rest of the src. I noticed it had executable permissions for some reason, but I'm quite certain that trying to run ./sway/sway-core/src/source_map.rs directly won't do anything interesting 😂

While working on #825 I noticed that that the `dependency_graph` is
unused throughout `sway-core`. I think the reason why cargo doesn't emit
any warnings about this is that at one point, it's stored in the public
`TypeCheckArgument` struct, so cargo believes that it is intentionally
being re-exported, however in reality we don't use it anywhere else in
the fuel/sway/forc ecosystem.
Copy link
Contributor

@otrho otrho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crazy. sway-core/src/source_map.rs?

@otrho
Copy link
Contributor

otrho commented Feb 22, 2022

Crazy. sway-core/src/source_map.rs?

Oh, you changed the file mode.

@mitchmindtree mitchmindtree self-assigned this Feb 22, 2022
@mitchmindtree mitchmindtree added the compiler General compiler. Should eventually become more specific as the issue is triaged label Feb 22, 2022
Copy link
Contributor

@emilyaherbert emilyaherbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM -- good catch 😄

@mitchmindtree mitchmindtree merged commit c77928c into master Feb 22, 2022
@mitchmindtree mitchmindtree deleted the mitchmindtree/unused-dep-graph branch February 22, 2022 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler General compiler. Should eventually become more specific as the issue is triaged
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants