RFC: Extend manifest dependencies with used

[epage]

Extend Cargo's dependency specifications so users can mark that a dependency is used, independent of what unused externs says.

Important

When responding to RFCs, try to use inline review comments (it is possible to leave an inline review comment for the entire file at the top) instead of direct comments for normal comments and keep normal comments for procedural matters like starting FCPs.

This keeps the discussion more organized.

Rendered

[bjorn3]

Maybe we could create a [peer-dependencies] section for that just like npm? This would also improve compile time by avoiding a need to block compilation of the current crate on said dependency. And cargo could get the option to warn if a peer-dependency is not used by any other crate (aka pinning a transitive dependency has no effect).

[epage]

Maybe we could create a [peer-dependencies] section for that just like npm?

There is an ecosystem cost to adding new dependencies tables. We'd have to weigh the cost against the benefits.

This would also improve compile time by avoiding a need to block compilation of the current crate on said dependency.

For the two use cases we have, compilation will already be blocked on them. Do we know of any use cases where this would be a concern?

[epage]

Reading up on peerDependencies, they sounds like target."cfg(false)".dependencies which helps with version requirements but not with feature activation.

[joshtriplett]

This looks great, and would enable us to warn about unused dependencies by default.

[Kobzol]

Is that really such a benefit? When searching for how to modify the behavior of a lint, checking how is a lint being configured or why is a given dependency not being marked as unused, I wouldn't really think of going to the dependency table. Rather I would go look at the lint configuration. Similarly to when I configure e.g. disallowed functions (disallowed_methods) for Clippy, that's also not specified on the dependency.

The way I interpret the [dependencies] table is "how to fetch and build dependencies used by my crate", possibly extended by "how to expose my dependencies to external crates", once we get public/private dependencies.

But configuring lints seems like a separate concern, that is also much more of an implementation detail of the given crate, and it IMO makes more sense for it to live in lint config, rather than here.

[epage]

I think there is a benefit. CPU caches are driven by the assumption of locality, that the same they will access memory that is close together over a close together period of time. I think locality of code is also an important consideration for us as humans to read and understand code and data. The perfect abstraction means you don't need to look at a function implementation, only its call site. Perfect abstractions don't exist but they are on a gradient. You can get away with a lower quality abstraction when the code is closer together while further away, you need a higher quality abstraction because it is harder to process and reason about because of the lack of locality.

I think the analogy with disallowed_methods doesn't work because you are applying a setting that affects a package globally while we are trying to control individual uses. We specifically call out, if we went with lint config, setting it in workspace.lints would be bad and that we likely would need a way to control which lints table the package exists in that we are referring to, to avoid being overly broad with this lint control.

That said, this isn't the only consideration here. We are weighing multiple benefits and downsides.

[epage]

Another possibility: Artifact deps feature has the lib field (better named link in my mind) which controls whether the lib is built and linked. What if we rename the lint unused_linked_dependencies and stabilize that field as a way to work around it.

I guess the questions are

• if un-linked deps (with or without an artifact) should unify features?
• does this cover all false positives?

[joshtriplett]

@epage If lint controls were to subsume this, what would it look like to say "dependencies X and Y are used so ignore the warning" with that system, while not suppressing the warning from Z?

[epage]

@epage If lint controls were to subsume this, what would it look like to say "dependencies X and Y are used so ignore the warning" with that system, while not suppressing the warning from Z?

I assume you are referring to comments of mine like:

lint control is a general problem

I have not thought through general, fine grained lint control and worry it would be too noisy in the schema to be worth it.

[ElchananHaas]

Could Cargo identify when a dependency line activates a feature on a transitive dependency? For instance, in this example the "semantic-addition" feature wouldn't be enabled by default, and this dependency line enables it.

I see that the RFC proposes other reasons for the "used" feature, but it could be better to figure out when a dependency line has an effect. In an analogy with Rust code's dead_code lint, that lint doesn't trigger on code that transitively affects the output of the program.

I think the alternative in "Link control" section is good. A dylib crate will be "unused" in all crates that depend on it. It makes more sense for the dylib to be marked as having some form of import side effect on its crate rather than make every crate importing it mark it as "used".

Are there any use cases for this that I am missing?

[epage]

Could Cargo identify when a dependency line activates a feature on a transitive dependency? For instance, in this example the "semantic-addition" feature wouldn't be enabled by default, and this dependency line enables it.

1. This would be complicated, especially to retrofit in at this point
2. This is about intent which auto-detection can't help with. If I activate a feature intentionally and some other dependency happens to do so also, is my line now unused? If add a dep with default features (foo = "1") and another dependency happens to do default-features = false, am I intentionally activating features or just naive added a dep and now it is unused?

[epage]

On a related note, there seems to be an unstable nounused extern property that might be able to be leveraged for the implementation of this.

See also rust-lang/cargo#16677.