Improve documentation on cap-lints

[behnam]

I'm looking at some docs for the --cap-lints and it's not clear what the expected behavior is.

In https://doc.rust-lang.org/rustc/lints/levels.html, we have:

This feature is used heavily by Cargo; it will pass --cap-lints allow when compiling your dependencies, so that if they have any warnings, they do not pollute the output of your build.

In rust-lang/rfcs#1193 we have:

Cargo will then pass --cap-lints allow to all upstream dependencies when compiling code.

And in #1830 we have:

This commit adds support to Cargo to pass --cap-lints allow to all upstream
dependencies instead of -A warings. This should serve the same purpose of
suppressing warnings in upstream dependencies as well as preventing widespread
breakage whenever a change to a lint is introduced in the compiler.

And the RFC has similar wordings, with an emphasis at the end:

Modifications to existing lints to emit new warnings will not get triggered, and new lints will also be entirely suppressed only for upstream dependencies.

---

Reading all the docs above, I'm not sure what "upstream dependencies" mean exactly. From the rustc doc, it seems that it's just any dependent crate.

But my guess is that Cargo doesn't want to disable such warnings when working in a workspace. So, probably "upstream" means any dependency out of current manifest's workspace.

But, setting up a simple test tells me neither of the above is right, and I'm guessing again that "upstream" would mean anything not fetched from the local machine. (Which is a bit more confusing, because I would expect the same behavior if using a remote git repo or using a local checkout of that the same repo--both being outside of my current workspace.)

---

This is the library:

my-lint-error/src/lib.rs

#[deny(missing_copy_implementations)]

pub struct Foo {
    pub field: i32
}

and this is the caller:

my-caller-app/src/main.rs

extern crate my_lint_error;

fn main() {
    println!("Hello, world!");
}

and the caller's manifest has:

[dependencies]
my-lint-error = { path="../my-lint-error/" }

The build fails with the lint error coming from the dependency:

$ cargo build --manifest-path my-caller-app/Cargo.toml -v
   Compiling my-lint-error v0.1.0 (file:///.../my-lint-error)
     Running `rustc --crate-name my_lint_error /.../my-lint-error/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=a5f75dbabcaae9e9 -C extra-filename=-a5f75dbabcaae9e9 --out-dir /.../my-caller-app/target/debug/deps -C incremental=/.../my-caller-app/target/debug/incremental -L dependency=/.../my-caller-app/target/debug/deps`
error: type could implement `Copy`; consider adding `impl Copy`
 --> /.../my-lint-error/src/lib.rs:3:1
  |
3 | / pub struct Foo {
4 | |     pub field: i32
5 | | }
  | |_^
  |
note: lint level defined here
 --> /.../my-lint-error/src/lib.rs:1:8
  |
1 | #[deny(missing_copy_implementations)]
  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

error: Could not compile `my-lint-error`.

Caused by:
  process didn't exit successfully: `rustc --crate-name my_lint_error /.../my-lint-error/src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=a5f75dbabcaae9e9 -C extra-filename=-a5f75dbabcaae9e9 --out-dir /.../my-caller-app/target/debug/deps -C incremental=/.../my-caller-app/target/debug/incremental -L dependency=/.../my-caller-app/target/debug/deps` (exit code: 1)

---

The test shows how a deny error from a library is breaking cargo build in a dependent crate. Looking at the rustc command, there's no --cap-lints passed in. Running the same command with --cap-lints allow builds the dependent without any issues. Therefore, I'm guessing there's a bug here in Cargo.

What do you think? Did I miss something in the docs?

PS. I'm on today's nightly and my ~/.cargo/config only has a bunch of aliases, so I believe my personal configuration is not causing this.

[behnam]

/cc @alexcrichton

[ehuss]

@behnam indeed, cargo changes the lint cap based on the type of dependency. Non-local dependencies (i.e. crates you cannot control) are --cap-lints=allow (unless you pass -vv in which case they become --cap-lints=warn). Path dependencies do not cap lints, as these are probably crates you are working on or have control over (such as in a workspace).

[behnam]

I see. Thanks for the explanation, @ehuss.

Now, then, first, I believe we need to make sure the docs are explaining this better. The word "upstream" is not defined clearly in the specs, and the actual compiler doc is even missing the word.

Apparently, there's also another matter here: the cap-lints flag doesn't apply to "derive" implementations. This also seems unintentional and undesired in this direction. Basically, again with the introduction of a new compiler lint rule, my crate started failing with error because of a serde_derive issue. (Example: https://travis-ci.org/open-i18n/rust-unic/jobs/422840730#L3838) I didn't see a discussion or tracker for this. Should we file one?

And, I believe there should be more docs around macro/derive behavior, as well.

What do you think?

[ehuss]

Code generated by procedural macros will be under the mercy of whatever settings you have in your crate. I don't think it's likely that the compiler can treat it differently since it gets directly inserted in your code at a relatively early phase. In general, using deny can be dangerous (see https://github.com/rust-unofficial/patterns/blob/master/anti_patterns/deny-warnings.md).

I imagine the documentation could clarify this behavior and include good practices. I'm not sure where, exactly, though. The rustc book generally doesn't document cargo behavior (I would say the existing remark is just to say "hey, here's an example of how this is used", not a detailed description). There is an open PR to significantly enhance lint controls in Cargo (#5728), and I imagine that would be the perfect opportunity to document it in depth. However, that PR seems to have stalled.

[Mark-Simulacrum]

Generally speaking derive macros are intended to be in a separate lint environment, much like edition scoping, but I don't think it's been implemented yet.

[pnkfelix]

@behnam indeed, cargo changes the lint cap based on the type of dependency. Non-local dependencies (i.e. crates you cannot control) are --cap-lints=allow (unless you pass -vv in which case they become --cap-lints=warn). Path dependencies do not cap lints, as these are probably crates you are working on or have control over (such as in a workspace).

I'd like to add a cargo -Z flag to change this behavior, if for no other reason than to make it easier to write cargo unit tests for --cap-lints behavior, since as far as I can tell the easiest kind of unit tests constructs projects that use filesystem path dependencies.

[ehuss]

easiest kind of unit tests constructs projects that use filesystem path dependencies.

I think it is actually easier to use registry dependencies. They can be added with one line of code:

Package::new("mydep", "0.1.0").publish();

[pnkfelix]

I think it is actually easier to use registry dependencies. They can be added with one line of code:

Package::new("mydep", "0.1.0").publish();

Okay, I'll go look at the tests that use this pattern and see if I can adapt it to my own needs.

[weihanglo]

See #7850 (comment) and #12115. Documentation can be resolved when -Zlints hit stable.

@rustbot label +S-needs-team-input

Laballed as such since it is blocked on -Zlints. If you find a proper place to document the behavior, welcome!

[epage]

@weihanglo how is this related to -Zlints?

[weihanglo]

I forgot. From what I can see now, not closely related to -Zlints but people have the same confusion with -Zlints. It may be good to have a place to explain them altogether.