rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
99.12k stars 12.8k forks source link

Diagnostic suggestions sometimes incorrectly point to external crates #88514

Open ehuss opened 3 years ago

ehuss commented 3 years ago

In some situations, rustc may provide a diagnostic suggestion in an external crate, which in general it shouldn't do. These external crates may be in cargo's registry cache, which the user should not be modifying. For example, cargo fix may stomp on changes outside of the package (which is a separate issue https://github.com/rust-lang/cargo/issues/9857). Usually rustc is good about avoiding this, so I'm not sure what is wrong in these situations.

In general, I expect rustc to not provide suggestions to modify dependencies.

The crater run found several instances of this, each for different underlying reasons (all dealing with macros).

Example 1 — unused tt

Example dependency:

// bar.rs
#[macro_export]
macro_rules! m {
    ($i:tt) => {
        $crate::m!(foo $i);
    };
    (foo $i:tt) => {
        let $i = 1;
    }
}

Example local crate:

// foo.rs
pub fn foo() {
    bar::m!(abc);
}

Resulting suggestion:

warning: unused variable: `abc`
 --> /Users/eric/Temp/crater-2021/foo/bar/src/lib.rs:7:13
  |
7 |         let $i = 1;
  |             ^^ help: if this is intentional, prefix it with an underscore: `_abc`
  |
  = note: `#[warn(unused_variables)]` on by default

A key point of this example is that the macro uses tt instead of ident. ident will not issue an unused warning.

Found in the 2021 crater run for:

Example 2 — Weird $body suggestion

The following makes a suggestion to remove unnecessary braces, but the suggestion doesn't actually remove any braces.

// Using cpython 0.2.1
use cpython::*;

fn hello(py: Python) -> PyResult<PyString> {
    Ok(PyString::new(py, "Rust says: Hello world"))
}

py_module_initializer!(example, initexample, PyInit_example, |py, m| {
    m.add(py, "hello", py_fn!(py, hello()))?;
    Ok(())
});

Suggestion:

warning: unnecessary braces around block return value
   --> /Users/eric/.cargo/registry/src/github.com-1ecc6299db9ec823/cpython-0.2.1/src/argparse.rs:386:52
    |
386 |     ( $py:expr, $iter:expr, $body:block, [] ) => { $body };
    |                                                    ^^^^^ help: remove these braces
    |
    = note: `#[warn(unused_braces)]` on by default

warning: `foo` (lib) generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.01s

Found in the 2021 crater run for:

Example 3 — Hidden JSON suggestion

In this example, the human-readable text doesn't mention the dependency, but the JSON includes a MachineApplicable suggestion.

// diesel 1.4.7
#[macro_use]
extern crate diesel;
use diesel::table;

table! {
    use diesel::sql_types::*;
    use std::fs;

    users {
        id -> Integer,
        name -> VarChar,
        favorite_color -> Nullable<VarChar>,
    }
}

This emits two duplicate diagnostics in human form:

warning: unused import: `std::fs`
 --> src/lib.rs:7:9
  |
7 |     use std::fs;
  |         ^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `std::fs`
 --> src/lib.rs:7:9
  |
7 |     use std::fs;
  |         ^^^^^^^

warning: `foo` (lib) generated 2 warnings

Buried in the JSON output is a machine-applicable change which modifies diesel:

{
    "byte_end": 9126,
    "byte_start": 9108,
    "column_end": 55,
    "column_start": 37,
    "expansion":
    {},
    "file_name": "/Users/eric/.cargo/registry/src/github.com-1ecc6299db9ec823/diesel-1.4.7/src/macros/mod.rs",
    "is_primary": true,
    "label": null,
    "line_end": 299,
    "line_start": 299,
    "suggested_replacement": "",
    "suggestion_applicability": "MachineApplicable",
    "text":
    [
        {
            "highlight_end": 55,
            "highlight_start": 37,
            "text": "            imports = [$($imports)* use $($import)::+;],"
        }
    ]
}

Found in the 2021 crater run for:

Meta

rustc --version --verbose:

rustc 1.56.0-nightly (5eacec9ec 2021-08-28)
binary: rustc
commit-hash: 5eacec9ec7e112a0de1011519a57c45586d58414
commit-date: 2021-08-28
host: x86_64-apple-darwin
release: 1.56.0-nightly
LLVM version: 13.0.0
weihanglo commented 3 years ago

The regression of "Example 1" was introduced between 1.51.0 and 1.52.0. The bisect report gave me three rollup PRs. The bug can be reproduced after this rollup and the suspect may be #82655.

I am not familiar with this part, so if anyone want to address it, just take it. (Or even better mentor me 😆, thanks)

Report from cargo-bisect-rustc ### Script for cargo-bisect-rustc ```bash #!/usr/bin/env bash # Run Example 1 cargo clean cargo fix 2>&1 | grep "Fixed" if [[ $? -gt 0 ]]; then exit 0 else exit 1 fi ``` ### Report from cargo-bisect-rustc ``` ******************************************************************************** Regression in nightly-2021-03-03 ******************************************************************************** fetching https://static.rust-lang.org/dist/2021-03-02/channel-rust-nightly-git-commit-hash.txt nightly manifest 2021-03-02: 40 B / 40 B [=======================================================================================] 100.00 % 692.44 KB/s converted 2021-03-02 to 4f20caa6258d4c74ce6b316fd347e3efe81cf557 fetching https://static.rust-lang.org/dist/2021-03-03/channel-rust-nightly-git-commit-hash.txt nightly manifest 2021-03-03: 40 B / 40 B [=======================================================================================] 100.00 % 893.43 KB/s converted 2021-03-03 to 35dbef235048f9a2939dc20effe083ca483c37ff looking for regression commit between 2021-03-02 and 2021-03-03 opening existing repository at "/Users/weihanglo/wd/contrib/rust" refreshing repository fetching (via local git) commits from 4f20caa6258d4c74ce6b316fd347e3efe81cf557 to 35dbef235048f9a2939dc20effe083ca483c37ff opening existing repository at "/Users/weihanglo/wd/contrib/rust" refreshing repository looking up first commit looking up second commit checking that commits are by bors and thus have ci artifacts... finding bors merge commits found 6 bors merge commits in the specified range commit[0] 2021-03-01UTC: Auto merge of #82663 - jyn514:rollup-xh3cb0c, r=jyn514 commit[1] 2021-03-02UTC: Auto merge of #82688 - GuillaumeGomez:rollup-b754t11, r=GuillaumeGomez commit[2] 2021-03-02UTC: Auto merge of #82634 - osa1:osa1/remove_old_fixme, r=Mark-Simulacrum commit[3] 2021-03-02UTC: Auto merge of #82698 - JohnTitor:rollup-htd533c, r=JohnTitor commit[4] 2021-03-02UTC: Auto merge of #82043 - tmiasko:may-have-side-effect, r=kennytm commit[5] 2021-03-02UTC: Auto merge of #82562 - llogiq:one-up-82248, r=oli-obk ERROR: no commits between 4f20caa6258d4c74ce6b316fd347e3efe81cf557 and 35dbef235048f9a2939dc20effe083ca483c37ff within last 167 days ----------------- ```
ehuss commented 7 months ago

This example attempts to make modifications to the standard library (if you have the source component installed).

pub fn bug_report<W: std::fmt::Write>(w: &mut W) -> std::fmt::Result {
    if true {
        writeln!(w, "`;?` here ->")?;
    } else {
        writeln!(w, "but not here")
    }
    Ok(())
}

JSON:

SEE JSON ```json { "$message_type": "diagnostic", "message": "mismatched types", "code": { "code": "E0308", "explanation": "Expected type did not match the received type.\n\nErroneous code examples:\n\n```compile_fail,E0308\nfn plus_one(x: i32) -> i32 {\n x + 1\n}\n\nplus_one(\"Not a number\");\n// ^^^^^^^^^^^^^^ expected `i32`, found `&str`\n\nif \"Not a bool\" {\n// ^^^^^^^^^^^^ expected `bool`, found `&str`\n}\n\nlet x: f32 = \"Not a float\";\n// --- ^^^^^^^^^^^^^ expected `f32`, found `&str`\n// |\n// expected due to this\n```\n\nThis error occurs when an expression was used in a place where the compiler\nexpected an expression of a different type. It can occur in several cases, the\nmost common being when calling a function and passing an argument which has a\ndifferent type than the matching type in the function declaration.\n" }, "level": "error", "spans": [ { "file_name": "/Users/eric/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/macros/mod.rs", "byte_start": 23568, "byte_end": 23617, "line_start": 670, "line_end": 670, "column_start": 9, "column_end": 58, "is_primary": true, "text": [ { "text": " $dst.write_fmt($crate::format_args_nl!($($arg)*))", "highlight_start": 9, "highlight_end": 58 } ], "label": "expected `()`, found `Result<(), Error>`", "suggested_replacement": null, "suggestion_applicability": null, "expansion": { "span": { "file_name": "lib.rs", "byte_start": 144, "byte_end": 171, "line_start": 5, "line_end": 5, "column_start": 9, "column_end": 36, "is_primary": false, "text": [ { "text": " writeln!(w, \"but not here\")", "highlight_start": 9, "highlight_end": 36 } ], "label": null, "suggested_replacement": null, "suggestion_applicability": null, "expansion": null }, "macro_decl_name": "writeln!", "def_site_span": { "file_name": "/Users/eric/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/macros/mod.rs", "byte_start": 23434, "byte_end": 23454, "line_start": 665, "line_end": 665, "column_start": 1, "column_end": 21, "is_primary": false, "text": [ { "text": "macro_rules! writeln {", "highlight_start": 1, "highlight_end": 21 } ], "label": null, "suggested_replacement": null, "suggestion_applicability": null, "expansion": null } } }, { "file_name": "lib.rs", "byte_start": 75, "byte_end": 177, "line_start": 2, "line_end": 6, "column_start": 5, "column_end": 6, "is_primary": false, "text": [ { "text": " if true {", "highlight_start": 5, "highlight_end": 14 }, { "text": " writeln!(w, \"`;?` here ->\")?;", "highlight_start": 1, "highlight_end": 38 }, { "text": " } else {", "highlight_start": 1, "highlight_end": 13 }, { "text": " writeln!(w, \"but not here\")", "highlight_start": 1, "highlight_end": 36 }, { "text": " }", "highlight_start": 1, "highlight_end": 6 } ], "label": "expected this to be `()`", "suggested_replacement": null, "suggestion_applicability": null, "expansion": null } ], "children": [ { "message": "expected unit type `()`\n found enum `Result<(), std::fmt::Error>`", "code": null, "level": "note", "spans": [], "children": [], "rendered": null }, { "message": "consider using a semicolon here", "code": null, "level": "help", "spans": [ { "file_name": "lib.rs", "byte_start": 177, "byte_end": 177, "line_start": 6, "line_end": 6, "column_start": 6, "column_end": 6, "is_primary": true, "text": [ { "text": " }", "highlight_start": 6, "highlight_end": 6 } ], "label": null, "suggested_replacement": ";", "suggestion_applicability": "MaybeIncorrect", "expansion": null } ], "children": [], "rendered": null }, { "message": "you might have meant to return this value", "code": null, "level": "help", "spans": [ { "file_name": "lib.rs", "byte_start": 144, "byte_end": 144, "line_start": 5, "line_end": 5, "column_start": 9, "column_end": 9, "is_primary": true, "text": [ { "text": " writeln!(w, \"but not here\")", "highlight_start": 9, "highlight_end": 9 } ], "label": null, "suggested_replacement": "return ", "suggestion_applicability": "MaybeIncorrect", "expansion": null }, { "file_name": "lib.rs", "byte_start": 171, "byte_end": 171, "line_start": 5, "line_end": 5, "column_start": 36, "column_end": 36, "is_primary": true, "text": [ { "text": " writeln!(w, \"but not here\")", "highlight_start": 36, "highlight_end": 36 } ], "label": null, "suggested_replacement": ";", "suggestion_applicability": "MaybeIncorrect", "expansion": null } ], "children": [], "rendered": null }, { "message": "use the `?` operator to extract the `Result<(), std::fmt::Error>` value, propagating a `Result::Err` value to the caller", "code": null, "level": "help", "spans": [ { "file_name": "/Users/eric/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/macros/mod.rs", "byte_start": 23617, "byte_end": 23617, "line_start": 670, "line_end": 670, "column_start": 58, "column_end": 58, "is_primary": true, "text": [ { "text": " $dst.write_fmt($crate::format_args_nl!($($arg)*))", "highlight_start": 58, "highlight_end": 58 } ], "label": null, "suggested_replacement": "?", "suggestion_applicability": "HasPlaceholders", "expansion": { "span": { "file_name": "lib.rs", "byte_start": 144, "byte_end": 171, "line_start": 5, "line_end": 5, "column_start": 9, "column_end": 36, "is_primary": false, "text": [ { "text": " writeln!(w, \"but not here\")", "highlight_start": 9, "highlight_end": 36 } ], "label": null, "suggested_replacement": null, "suggestion_applicability": null, "expansion": null }, "macro_decl_name": "writeln!", "def_site_span": { "file_name": "/Users/eric/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/macros/mod.rs", "byte_start": 23434, "byte_end": 23454, "line_start": 665, "line_end": 665, "column_start": 1, "column_end": 21, "is_primary": false, "text": [ { "text": "macro_rules! writeln {", "highlight_start": 1, "highlight_end": 21 } ], "label": null, "suggested_replacement": null, "suggestion_applicability": null, "expansion": null } } } ], "children": [], "rendered": null } ], "rendered": "error[E0308]: mismatched types\n --> lib.rs:5:9\n |\n2 | / if true {\n3 | | writeln!(w, \"`;?` here ->\")?;\n4 | | } else {\n5 | | writeln!(w, \"but not here\")\n | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `Result<(), Error>`\n6 | | }\n | |_____- expected this to be `()`\n |\n = note: expected unit type `()`\n found enum `Result<(), std::fmt::Error>`\n = note: this error originates in the macro `writeln` (in Nightly builds, run with -Z macro-backtrace for more info)\nhelp: consider using a semicolon here\n |\n6 | };\n | +\nhelp: you might have meant to return this value\n |\n5 | return writeln!(w, \"but not here\");\n | ++++++ +\nhelp: use the `?` operator to extract the `Result<(), std::fmt::Error>` value, propagating a `Result::Err` value to the caller\n --> /Users/eric/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/macros/mod.rs:670:58\n |\n67| $dst.write_fmt($crate::format_args_nl!($($arg)*))?\n | +\n\n" } ```

Suggests to edit lib/rustlib/src/rust/library/core/src/macros/mod.rs

rustc 1.79.0-nightly (88c2f4f5f 2024-04-02)
binary: rustc
commit-hash: 88c2f4f5f50ace5ddc7655ea311435104d3659bd
commit-date: 2024-04-02
host: aarch64-apple-darwin
release: 1.79.0-nightly
LLVM version: 18.1.2