From 21c75ad1ccc234b8cc2ffd23ed411b3a0da70833 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 28 Nov 2024 17:55:52 +0000 Subject: [PATCH 01/18] Do not call `extern_crate` on current trait on crate mismatch errors When we encounter an error caused by traits/types of different versions of the same crate, filter out the current crate when collecting spans to add to the context so we don't call `extern_crate` on the `DefId` of the current crate, which is meaningless and ICEs. Produced output with this filter: ``` error[E0277]: the trait bound `foo::Struct: Trait` is not satisfied --> y.rs:13:19 | 13 | check_trait::(); | ^^^^^^^^^^^ the trait `Trait` is not implemented for `foo::Struct` | note: there are multiple different versions of crate `foo` in the dependency graph --> y.rs:7:1 | 4 | extern crate foo; | ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate 5 | 6 | pub struct Struct; | ----------------- this type implements the required trait 7 | pub trait Trait {} | ^^^^^^^^^^^^^^^ this is the required trait | ::: x.rs:4:1 | 4 | pub struct Struct; | ----------------- this type doesn't implement the required trait 5 | pub trait Trait {} | --------------- this is the found trait = note: two types coming from two different versions of the same crate are different types even if they look the same = help: you can use `cargo tree` to explore your dependency tree note: required by a bound in `check_trait` --> y.rs:10:19 | 10 | fn check_trait() {} | ^^^^^ required by this bound in `check_trait` ``` Fix #133563. (cherry picked from commit 8574f374e2cc27b53c8b81dc4031c59ca3035284) --- .../traits/fulfillment_errors.rs | 4 +++ .../foo-current.rs | 14 ++++++++ .../foo-prev.rs | 6 ++++ .../rmake.rs | 32 +++++++++++++++++++ 4 files changed, 56 insertions(+) create mode 100644 tests/run-make/crate-loading-crate-depends-on-itself/foo-current.rs create mode 100644 tests/run-make/crate-loading-crate-depends-on-itself/foo-prev.rs create mode 100644 tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 4e7d7b79ff4de..20cef5e06a4cf 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -1732,6 +1732,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { span.push_span_label(self.tcx.def_span(trait_def_id), "this is the required trait"); for (sp, label) in [trait_def_id, other_trait_def_id] .iter() + // The current crate-version might depend on another version of the same crate + // (Think "semver-trick"). Do not call `extern_crate` in that case for the local + // crate as that doesn't make sense and ICEs (#133563). + .filter(|def_id| !def_id.is_local()) .filter_map(|def_id| self.tcx.extern_crate(def_id.krate)) .map(|data| { let dependency = if data.dependency_of == LOCAL_CRATE { diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/foo-current.rs b/tests/run-make/crate-loading-crate-depends-on-itself/foo-current.rs new file mode 100644 index 0000000000000..71b27cd85bfed --- /dev/null +++ b/tests/run-make/crate-loading-crate-depends-on-itself/foo-current.rs @@ -0,0 +1,14 @@ +#![crate_type = "lib"] +#![crate_name = "foo"] + +extern crate foo; + +pub struct Struct; +pub trait Trait {} +impl Trait for Struct {} + +fn check_trait() {} + +fn ice() { + check_trait::(); +} diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/foo-prev.rs b/tests/run-make/crate-loading-crate-depends-on-itself/foo-prev.rs new file mode 100644 index 0000000000000..19d3f3c972b09 --- /dev/null +++ b/tests/run-make/crate-loading-crate-depends-on-itself/foo-prev.rs @@ -0,0 +1,6 @@ +#![crate_type = "lib"] +#![crate_name = "foo"] + +pub struct Struct; +pub trait Trait {} +impl Trait for Struct {} diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs b/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs new file mode 100644 index 0000000000000..f52f3d791a713 --- /dev/null +++ b/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs @@ -0,0 +1,32 @@ +//@ only-linux +//@ ignore-wasm32 +//@ ignore-wasm64 +// ignore-tidy-linelength + +// Verify that if the current crate depends on a different version of the same crate, *and* types +// and traits of the different versions are mixed, we produce diagnostic output and not an ICE. +// #133563 + +use run_make_support::{rust_lib_name, rustc}; + +fn main() { + rustc().input("foo-prev.rs").run(); + + rustc() + .extra_filename("current") + .metadata("current") + .input("foo-current.rs") + .extern_("foo", rust_lib_name("foo")) + .run_fail() + .assert_stderr_contains(r#" +note: there are multiple different versions of crate `foo` in the dependency graph + --> foo-current.rs:7:1 + | +4 | extern crate foo; + | ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate +5 | +6 | pub struct Struct; + | ----------------- this type implements the required trait +7 | pub trait Trait {} + | ^^^^^^^^^^^^^^^ this is the required trait"#); +} From 2fb0b160c051f57a9e93c33d6ed972195a009e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 29 Nov 2024 18:31:37 +0000 Subject: [PATCH 02/18] Use rmake `diff` output in test (cherry picked from commit e97e15dea55d61d68732bc030be1a44d8a51a1e9) --- .../foo.stderr | 29 +++++++++++++++++++ .../rmake.rs | 25 ++++++++-------- 2 files changed, 41 insertions(+), 13 deletions(-) create mode 100644 tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr b/tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr new file mode 100644 index 0000000000000..9c2fcabe5ba85 --- /dev/null +++ b/tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr @@ -0,0 +1,29 @@ +error[E0277]: the trait bound `foo::Struct: Trait` is not satisfied because the trait comes from a different crate version + --> foo-current.rs:13:19 + | +13 | check_trait::(); + | ^^^^^^^^^^^ the trait `Trait` is not implemented for `foo::Struct` + | +note: there are multiple different versions of crate `foo` in the dependency graph + --> foo-current.rs:7:1 + | +4 | extern crate foo; + | ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate +5 | +6 | pub struct Struct; + | ----------------- this type implements the required trait +7 | pub trait Trait {} + | ^^^^^^^^^^^^^^^ this is the required trait + | + ::: foo-prev.rs:X:Y + | +4 | pub struct Struct; + | ----------------- this type doesn't implement the required trait +5 | pub trait Trait {} + | --------------- this is the found trait + = note: two types coming from two different versions of the same crate are different types even if they look the same + = help: you can use `cargo tree` to explore your dependency tree + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`. \ No newline at end of file diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs b/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs index f52f3d791a713..57e0cab92f1ef 100644 --- a/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs +++ b/tests/run-make/crate-loading-crate-depends-on-itself/rmake.rs @@ -7,26 +7,25 @@ // and traits of the different versions are mixed, we produce diagnostic output and not an ICE. // #133563 -use run_make_support::{rust_lib_name, rustc}; +use run_make_support::{diff, rust_lib_name, rustc}; fn main() { rustc().input("foo-prev.rs").run(); - rustc() + let out = rustc() .extra_filename("current") .metadata("current") .input("foo-current.rs") .extern_("foo", rust_lib_name("foo")) .run_fail() - .assert_stderr_contains(r#" -note: there are multiple different versions of crate `foo` in the dependency graph - --> foo-current.rs:7:1 - | -4 | extern crate foo; - | ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate -5 | -6 | pub struct Struct; - | ----------------- this type implements the required trait -7 | pub trait Trait {} - | ^^^^^^^^^^^^^^^ this is the required trait"#); + .stderr_utf8(); + + // We don't remap the path of the `foo-prev` crate, so we remap it here. + let mut lines: Vec<_> = out.lines().collect(); + for line in &mut lines { + if line.starts_with(" ::: ") { + *line = " ::: foo-prev.rs:X:Y"; + } + } + diff().expected_file("foo.stderr").actual_text("(rustc)", &lines.join("\n")).run(); } From bee5d0996b4c5f4f62ed7a68f0aff6fc745540f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 29 Nov 2024 18:43:48 +0000 Subject: [PATCH 03/18] Move the `crate-loading` test to use `diff` output (cherry picked from commit 998ff2f0cd2902e86178d35b01ba78fe4633f80b) --- .../src/external_deps/rustc.rs | 6 + .../multiple-dep-versions.stderr | 113 ++++++++++++++++++ tests/run-make/crate-loading/rmake.rs | 89 +++----------- 3 files changed, 138 insertions(+), 70 deletions(-) create mode 100644 tests/run-make/crate-loading/multiple-dep-versions.stderr diff --git a/src/tools/run-make-support/src/external_deps/rustc.rs b/src/tools/run-make-support/src/external_deps/rustc.rs index 494daeca96364..ffe10092cc28a 100644 --- a/src/tools/run-make-support/src/external_deps/rustc.rs +++ b/src/tools/run-make-support/src/external_deps/rustc.rs @@ -227,6 +227,12 @@ impl Rustc { self } + /// Normalize the line number in the stderr output + pub fn ui_testing(&mut self) -> &mut Self { + self.cmd.arg(format!("-Zui-testing")); + self + } + /// Specify the target triple, or a path to a custom target json spec file. pub fn target>(&mut self, target: S) -> &mut Self { let target = target.as_ref(); diff --git a/tests/run-make/crate-loading/multiple-dep-versions.stderr b/tests/run-make/crate-loading/multiple-dep-versions.stderr new file mode 100644 index 0000000000000..7f04b2dd64ab8 --- /dev/null +++ b/tests/run-make/crate-loading/multiple-dep-versions.stderr @@ -0,0 +1,113 @@ +error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied because the trait comes from a different crate version + --> replaced + | +LL | do_something(Type); + | ^^^^ the trait `Trait` is not implemented for `dep_2_reexport::Type` + | +note: there are multiple different versions of crate `dependency` in the dependency graph + --> replaced + | +LL | pub struct Type(pub i32); + | --------------- this type implements the required trait +LL | pub trait Trait { + | ^^^^^^^^^^^^^^^ this is the required trait + | + ::: replaced + | +LL | extern crate dep_2_reexport; + | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` +LL | extern crate dependency; + | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate + | + ::: replaced + | +LL | pub struct Type; + | --------------- this type doesn't implement the required trait +LL | pub trait Trait { + | --------------- this is the found trait + = note: two types coming from two different versions of the same crate are different types even if they look the same + = help: you can use `cargo tree` to explore your dependency tree + +error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope + --> replaced + | +LL | Type.foo(); + | ^^^ method not found in `Type` + | +note: there are multiple different versions of crate `dependency` in the dependency graph + --> replaced + | +LL | pub trait Trait { + | ^^^^^^^^^^^^^^^ this is the trait that is needed +LL | fn foo(&self); + | -------------- the method is available for `dep_2_reexport::Type` here + | + ::: replaced + | +LL | use dependency::{Trait, do_something}; + | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency` + | + ::: replaced + | +LL | pub trait Trait { + | --------------- this is the trait that was imported + +error[E0599]: no function or associated item named `bar` found for struct `dep_2_reexport::Type` in the current scope + --> replaced + | +LL | Type::bar(); + | ^^^ function or associated item not found in `Type` + | +note: there are multiple different versions of crate `dependency` in the dependency graph + --> replaced + | +LL | pub trait Trait { + | ^^^^^^^^^^^^^^^ this is the trait that is needed +LL | fn foo(&self); +LL | fn bar(); + | --------- the associated function is available for `dep_2_reexport::Type` here + | + ::: replaced + | +LL | use dependency::{Trait, do_something}; + | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency` + | + ::: replaced + | +LL | pub trait Trait { + | --------------- this is the trait that was imported + +error[E0277]: the trait bound `OtherType: Trait` is not satisfied because the trait comes from a different crate version + --> replaced + | +LL | do_something(OtherType); + | ^^^^^^^^^ the trait `Trait` is not implemented for `OtherType` + | +note: there are multiple different versions of crate `dependency` in the dependency graph + --> replaced + | +LL | pub trait Trait { + | ^^^^^^^^^^^^^^^ this is the required trait + | + ::: replaced + | +LL | extern crate dep_2_reexport; + | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` +LL | extern crate dependency; + | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate + | + ::: replaced + | +LL | pub struct OtherType; + | -------------------- this type doesn't implement the required trait + | + ::: replaced + | +LL | pub trait Trait { + | --------------- this is the found trait + = help: you can use `cargo tree` to explore your dependency tree + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0277, E0599. +For more information about an error, try `rustc --explain E0277`. \ No newline at end of file diff --git a/tests/run-make/crate-loading/rmake.rs b/tests/run-make/crate-loading/rmake.rs index 2d5913c4bcb09..6ad456e3e3e57 100644 --- a/tests/run-make/crate-loading/rmake.rs +++ b/tests/run-make/crate-loading/rmake.rs @@ -3,7 +3,7 @@ //@ ignore-wasm64 // ignore-tidy-linelength -use run_make_support::{rust_lib_name, rustc}; +use run_make_support::{diff, rust_lib_name, rustc}; fn main() { rustc().input("multiple-dep-versions-1.rs").run(); @@ -13,77 +13,26 @@ fn main() { .extern_("dependency", rust_lib_name("dependency2")) .run(); - rustc() + let out = rustc() .input("multiple-dep-versions.rs") .extern_("dependency", rust_lib_name("dependency")) .extern_("dep_2_reexport", rust_lib_name("foo")) + .ui_testing() .run_fail() - .assert_stderr_contains(r#"error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied because the trait comes from a different crate version - --> multiple-dep-versions.rs:7:18 - | -7 | do_something(Type); - | ^^^^ the trait `Trait` is not implemented for `dep_2_reexport::Type` - | -note: there are multiple different versions of crate `dependency` in the dependency graph"#) - .assert_stderr_contains(r#" -3 | pub struct Type(pub i32); - | --------------- this type implements the required trait -4 | pub trait Trait { - | ^^^^^^^^^^^^^^^ this is the required trait -"#) - .assert_stderr_contains(r#" -1 | extern crate dep_2_reexport; - | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` -2 | extern crate dependency; - | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate"#) - .assert_stderr_contains(r#" -3 | pub struct Type; - | --------------- this type doesn't implement the required trait -4 | pub trait Trait { - | --------------- this is the found trait - = note: two types coming from two different versions of the same crate are different types even if they look the same - = help: you can use `cargo tree` to explore your dependency tree"#) - .assert_stderr_contains(r#"error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope - --> multiple-dep-versions.rs:8:10 - | -8 | Type.foo(); - | ^^^ method not found in `Type` - | -note: there are multiple different versions of crate `dependency` in the dependency graph"#) - .assert_stderr_contains(r#" -4 | pub trait Trait { - | ^^^^^^^^^^^^^^^ this is the trait that is needed -5 | fn foo(&self); - | -------------- the method is available for `dep_2_reexport::Type` here - | - ::: multiple-dep-versions.rs:4:18 - | -4 | use dependency::{Trait, do_something}; - | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#) - .assert_stderr_contains(r#" -4 | pub trait Trait { - | --------------- this is the trait that was imported"#) - .assert_stderr_contains(r#" -error[E0599]: no function or associated item named `bar` found for struct `dep_2_reexport::Type` in the current scope - --> multiple-dep-versions.rs:9:11 - | -9 | Type::bar(); - | ^^^ function or associated item not found in `Type` - | -note: there are multiple different versions of crate `dependency` in the dependency graph"#) - .assert_stderr_contains(r#" -4 | pub trait Trait { - | ^^^^^^^^^^^^^^^ this is the trait that is needed -5 | fn foo(&self); -6 | fn bar(); - | --------- the associated function is available for `dep_2_reexport::Type` here - | - ::: multiple-dep-versions.rs:4:18 - | -4 | use dependency::{Trait, do_something}; - | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#) - .assert_stderr_contains( - r#" -6 | pub struct OtherType; - | -------------------- this type doesn't implement the required trait"#); + .stderr_utf8(); + + // We don't remap all the paths, so we remap it here. + let mut lines: Vec<_> = out.lines().collect(); + for line in &mut lines { + if line.starts_with(" --> ") { + *line = " --> replaced"; + } + if line.starts_with(" ::: ") { + *line = " ::: replaced"; + } + } + diff() + .expected_file("multiple-dep-versions.stderr") + .actual_text("(rustc)", &lines.join("\n")) + .run(); } From 31236a39f86ad8128323afd0cead010227444c2a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 13 Dec 2024 16:19:34 +0100 Subject: [PATCH 04/18] Correctly handle comments in attributes in doctests source code (cherry picked from commit de16ed35a326041f619de882dfcead1d02623328) --- src/librustdoc/doctest/make.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustdoc/doctest/make.rs b/src/librustdoc/doctest/make.rs index 3ae60938749ec..f484c98a0d30e 100644 --- a/src/librustdoc/doctest/make.rs +++ b/src/librustdoc/doctest/make.rs @@ -520,6 +520,8 @@ fn handle_attr(mod_attr_pending: &mut String, source_info: &mut SourceInfo, edit mod_attr_pending.clear(); } else if mod_attr_pending.ends_with('\\') { mod_attr_pending.push('n'); + } else { + mod_attr_pending.push_str("\n"); } } From e09de50ce0ded8b290098874f9d778f521b8b0cc Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 13 Dec 2024 16:20:00 +0100 Subject: [PATCH 05/18] Add ui regression test for #134221 (cherry picked from commit 9c4a61ff52a635ef96bd92a2ff1fad4a8bb2ce73) --- .../doctest/comment-in-attr-134221.rs | 23 ++++++++++++ .../doctest/comment-in-attr-134221.stdout | 36 +++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 tests/rustdoc-ui/doctest/comment-in-attr-134221.rs create mode 100644 tests/rustdoc-ui/doctest/comment-in-attr-134221.stdout diff --git a/tests/rustdoc-ui/doctest/comment-in-attr-134221.rs b/tests/rustdoc-ui/doctest/comment-in-attr-134221.rs new file mode 100644 index 0000000000000..fa8cd106659b1 --- /dev/null +++ b/tests/rustdoc-ui/doctest/comment-in-attr-134221.rs @@ -0,0 +1,23 @@ +// Regression test for . +// It checks that even if there are comments in the attributes, the attributes +// will still be generated correctly (and therefore fail in this test). + +//@ compile-flags:--test --test-args --test-threads=1 +//@ failure-status: 101 +//@ normalize-stdout-test: "tests/rustdoc-ui/doctest" -> "$$DIR" +//@ normalize-stdout-test: "finished in \d+\.\d+s" -> "finished in $$TIME" +//@ normalize-stdout-test: ".rs:\d+:\d+" -> ".rs:$$LINE:$$COL" + +/*! +```rust +#![feature( + foo, // +)] +``` + +```rust +#![feature( + foo, +)] +``` +*/ diff --git a/tests/rustdoc-ui/doctest/comment-in-attr-134221.stdout b/tests/rustdoc-ui/doctest/comment-in-attr-134221.stdout new file mode 100644 index 0000000000000..728bfdb010f4f --- /dev/null +++ b/tests/rustdoc-ui/doctest/comment-in-attr-134221.stdout @@ -0,0 +1,36 @@ + +running 2 tests +test $DIR/comment-in-attr-134221.rs - (line 11) ... FAILED +test $DIR/comment-in-attr-134221.rs - (line 17) ... FAILED + +failures: + +---- $DIR/comment-in-attr-134221.rs - (line 11) stdout ---- +error[E0635]: unknown feature `foo` + --> $DIR/comment-in-attr-134221.rs:$LINE:$COL + | +LL | foo, // + | ^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0635`. +Couldn't compile the test. +---- $DIR/comment-in-attr-134221.rs - (line 17) stdout ---- +error[E0635]: unknown feature `foo` + --> $DIR/comment-in-attr-134221.rs:$LINE:$COL + | +LL | foo, + | ^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0635`. +Couldn't compile the test. + +failures: + $DIR/comment-in-attr-134221.rs - (line 11) + $DIR/comment-in-attr-134221.rs - (line 17) + +test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME + From 32ee5bc6a33092f4b642631e425ddbeeec96fb08 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 16 Dec 2024 13:59:41 +0100 Subject: [PATCH 06/18] Also handle cases where attributes are unclosed (cherry picked from commit 23839853425e8c0c80d0aadb32bf5b4ba1bdf64b) --- src/librustdoc/doctest/make.rs | 56 +++++++++++++------ .../doctest/comment-in-attr-134221.rs | 4 ++ .../doctest/comment-in-attr-134221.stdout | 18 +++++- 3 files changed, 60 insertions(+), 18 deletions(-) diff --git a/src/librustdoc/doctest/make.rs b/src/librustdoc/doctest/make.rs index f484c98a0d30e..66549afe5a1e8 100644 --- a/src/librustdoc/doctest/make.rs +++ b/src/librustdoc/doctest/make.rs @@ -51,8 +51,17 @@ impl DocTestBuilder { !lang_str.compile_fail && !lang_str.test_harness && !lang_str.standalone_crate }); - let SourceInfo { crate_attrs, maybe_crate_attrs, crates, everything_else } = - partition_source(source, edition); + let Some(SourceInfo { crate_attrs, maybe_crate_attrs, crates, everything_else }) = + partition_source(source, edition) + else { + return Self::invalid( + String::new(), + String::new(), + String::new(), + source.to_string(), + test_id, + ); + }; // Uses librustc_ast to parse the doctest and find if there's a main fn and the extern // crate already is included. @@ -77,18 +86,7 @@ impl DocTestBuilder { else { // If the parser panicked due to a fatal error, pass the test code through unchanged. // The error will be reported during compilation. - return Self { - supports_color: false, - has_main_fn: false, - crate_attrs, - maybe_crate_attrs, - crates, - everything_else, - already_has_extern_crate: false, - test_id, - failed_ast: true, - can_be_merged: false, - }; + return Self::invalid(crate_attrs, maybe_crate_attrs, crates, everything_else, test_id); }; // If the AST returned an error, we don't want this doctest to be merged with the // others. Same if it contains `#[feature]` or `#[no_std]`. @@ -113,6 +111,27 @@ impl DocTestBuilder { } } + fn invalid( + crate_attrs: String, + maybe_crate_attrs: String, + crates: String, + everything_else: String, + test_id: Option, + ) -> Self { + Self { + supports_color: false, + has_main_fn: false, + crate_attrs, + maybe_crate_attrs, + crates, + everything_else, + already_has_extern_crate: false, + test_id, + failed_ast: true, + can_be_merged: false, + } + } + /// Transforms a test into code that can be compiled into a Rust binary, and returns the number of /// lines before the test code begins. pub(crate) fn generate_unique_doctest( @@ -533,7 +552,7 @@ struct SourceInfo { everything_else: String, } -fn partition_source(s: &str, edition: Edition) -> SourceInfo { +fn partition_source(s: &str, edition: Edition) -> Option { #[derive(Copy, Clone, PartialEq)] enum PartitionState { Attrs, @@ -608,11 +627,16 @@ fn partition_source(s: &str, edition: Edition) -> SourceInfo { } } + if !mod_attr_pending.is_empty() { + debug!("invalid doctest code: {s:?}"); + return None; + } + source_info.everything_else = source_info.everything_else.trim().to_string(); debug!("crate_attrs:\n{}{}", source_info.crate_attrs, source_info.maybe_crate_attrs); debug!("crates:\n{}", source_info.crates); debug!("after:\n{}", source_info.everything_else); - source_info + Some(source_info) } diff --git a/tests/rustdoc-ui/doctest/comment-in-attr-134221.rs b/tests/rustdoc-ui/doctest/comment-in-attr-134221.rs index fa8cd106659b1..3689ebe166ad4 100644 --- a/tests/rustdoc-ui/doctest/comment-in-attr-134221.rs +++ b/tests/rustdoc-ui/doctest/comment-in-attr-134221.rs @@ -20,4 +20,8 @@ foo, )] ``` + +```rust +#![ +``` */ diff --git a/tests/rustdoc-ui/doctest/comment-in-attr-134221.stdout b/tests/rustdoc-ui/doctest/comment-in-attr-134221.stdout index 728bfdb010f4f..aa1b27d1f0bd9 100644 --- a/tests/rustdoc-ui/doctest/comment-in-attr-134221.stdout +++ b/tests/rustdoc-ui/doctest/comment-in-attr-134221.stdout @@ -1,7 +1,8 @@ -running 2 tests +running 3 tests test $DIR/comment-in-attr-134221.rs - (line 11) ... FAILED test $DIR/comment-in-attr-134221.rs - (line 17) ... FAILED +test $DIR/comment-in-attr-134221.rs - (line 23) ... FAILED failures: @@ -26,11 +27,24 @@ LL | foo, error: aborting due to 1 previous error For more information about this error, try `rustc --explain E0635`. +Couldn't compile the test. +---- $DIR/comment-in-attr-134221.rs - (line 23) stdout ---- +error: this file contains an unclosed delimiter + --> $DIR/comment-in-attr-134221.rs:$LINE:$COL + | +LL | #![ + | -^ + | | + | unclosed delimiter + +error: aborting due to 1 previous error + Couldn't compile the test. failures: $DIR/comment-in-attr-134221.rs - (line 11) $DIR/comment-in-attr-134221.rs - (line 17) + $DIR/comment-in-attr-134221.rs - (line 23) -test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME +test result: FAILED. 0 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME From dc1a9f275c0caaf8c080c7cc6f9809a44db4ac2a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 16 Dec 2024 17:59:07 +0100 Subject: [PATCH 07/18] Remove unneeded handling of backlines in doctest attributes (cherry picked from commit c367cc3ef5648d5695fdb795cc66edbff88b4ce9) --- src/librustdoc/doctest/make.rs | 2 -- .../doctest/comment-in-attr-134221-2.rs | 15 +++++++++ .../doctest/comment-in-attr-134221-2.stdout | 31 +++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 tests/rustdoc-ui/doctest/comment-in-attr-134221-2.rs create mode 100644 tests/rustdoc-ui/doctest/comment-in-attr-134221-2.stdout diff --git a/src/librustdoc/doctest/make.rs b/src/librustdoc/doctest/make.rs index 66549afe5a1e8..a188bc8ebd912 100644 --- a/src/librustdoc/doctest/make.rs +++ b/src/librustdoc/doctest/make.rs @@ -537,8 +537,6 @@ fn handle_attr(mod_attr_pending: &mut String, source_info: &mut SourceInfo, edit push_to.push('\n'); // If it's complete, then we can clear the pending content. mod_attr_pending.clear(); - } else if mod_attr_pending.ends_with('\\') { - mod_attr_pending.push('n'); } else { mod_attr_pending.push_str("\n"); } diff --git a/tests/rustdoc-ui/doctest/comment-in-attr-134221-2.rs b/tests/rustdoc-ui/doctest/comment-in-attr-134221-2.rs new file mode 100644 index 0000000000000..8cdd665ff69d2 --- /dev/null +++ b/tests/rustdoc-ui/doctest/comment-in-attr-134221-2.rs @@ -0,0 +1,15 @@ +//@ compile-flags:--test --test-args --test-threads=1 +//@ failure-status: 101 +//@ normalize-stdout-test: "tests/rustdoc-ui/doctest" -> "$$DIR" +//@ normalize-stdout-test: "finished in \d+\.\d+s" -> "finished in $$TIME" +//@ normalize-stdout-test: ".rs:\d+:\d+" -> ".rs:$$LINE:$$COL" + +//! ``` +#![doc = "#![all\ +ow(unused)]"] +//! ``` +//! +//! ``` +#![doc = r#"#![all\ +ow(unused)]"#] +//! ``` diff --git a/tests/rustdoc-ui/doctest/comment-in-attr-134221-2.stdout b/tests/rustdoc-ui/doctest/comment-in-attr-134221-2.stdout new file mode 100644 index 0000000000000..0baff3df14431 --- /dev/null +++ b/tests/rustdoc-ui/doctest/comment-in-attr-134221-2.stdout @@ -0,0 +1,31 @@ + +running 2 tests +test $DIR/comment-in-attr-134221-2.rs - (line 11) ... FAILED +test $DIR/comment-in-attr-134221-2.rs - (line 7) ... ok + +failures: + +---- $DIR/comment-in-attr-134221-2.rs - (line 11) stdout ---- +error: unknown start of token: \ + --> $DIR/comment-in-attr-134221-2.rs:$LINE:$COL + | +LL | #![all\ + | ^ + +error: expected one of `(`, `::`, `=`, `[`, `]`, or `{`, found `ow` + --> $DIR/comment-in-attr-134221-2.rs:$LINE:$COL + | +LL | #![all\ + | - expected one of `(`, `::`, `=`, `[`, `]`, or `{` +LL | ow(unused)] + | ^^ unexpected token + +error: aborting due to 2 previous errors + +Couldn't compile the test. + +failures: + $DIR/comment-in-attr-134221-2.rs - (line 11) + +test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME + From 4171701e394f5f6aa26fedb3fb2e9908e242db5d Mon Sep 17 00:00:00 2001 From: "Tim (Theemathas) Chirananthavat" Date: Sun, 15 Dec 2024 12:37:14 +0700 Subject: [PATCH 08/18] Correctly document is_null CTFE behavior. The "panic in const if CTFE doesn't know the answer" behavior was discussed to be the desired behavior in #74939, and is currently how the function actually behaves. I intentionally wrote this documentation to allow for the possibility that a panic might not occur even if the pointer is out of bounds, because of #133700 and other potential changes in the future. (cherry picked from commit 93889172bc6fdb085bccf15e201a7c03d1bdc8e3) --- library/core/src/ptr/const_ptr.rs | 19 +++++++++++-------- library/core/src/ptr/mut_ptr.rs | 19 +++++++++++-------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index dfe905544af97..81c8251d497bb 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -12,14 +12,17 @@ impl *const T { /// Therefore, two pointers that are null may still not compare equal to /// each other. /// - /// ## Behavior during const evaluation - /// - /// When this function is used during const evaluation, it may return `false` for pointers - /// that turn out to be null at runtime. Specifically, when a pointer to some memory - /// is offset beyond its bounds in such a way that the resulting pointer is null, - /// the function will still return `false`. There is no way for CTFE to know - /// the absolute position of that memory, so we cannot tell if the pointer is - /// null or not. + /// # Panics during const evaluation + /// + /// If this method is used during const evaluation, and `self` is a pointer + /// that is offset beyond the bounds of the memory it initially pointed to, + /// then there might not be enough information to determine whether the + /// pointer is null. This is because the absolute address in memory is not + /// known at compile time. If the nullness of the pointer cannot be + /// determined, this method will panic. + /// + /// In-bounds pointers are never null, so the method will never panic for + /// such pointers. /// /// # Examples /// diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index 5ed0b39f33b76..9fce097e7b493 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -12,14 +12,17 @@ impl *mut T { /// Therefore, two pointers that are null may still not compare equal to /// each other. /// - /// ## Behavior during const evaluation - /// - /// When this function is used during const evaluation, it may return `false` for pointers - /// that turn out to be null at runtime. Specifically, when a pointer to some memory - /// is offset beyond its bounds in such a way that the resulting pointer is null, - /// the function will still return `false`. There is no way for CTFE to know - /// the absolute position of that memory, so we cannot tell if the pointer is - /// null or not. + /// # Panics during const evaluation + /// + /// If this method is used during const evaluation, and `self` is a pointer + /// that is offset beyond the bounds of the memory it initially pointed to, + /// then there might not be enough information to determine whether the + /// pointer is null. This is because the absolute address in memory is not + /// known at compile time. If the nullness of the pointer cannot be + /// determined, this method will panic. + /// + /// In-bounds pointers are never null, so the method will never panic for + /// such pointers. /// /// # Examples /// From 9d0471549526f8e17c9a81f3dcc26f7d4ceee03f Mon Sep 17 00:00:00 2001 From: "Tim (Theemathas) Chirananthavat" Date: Sat, 21 Dec 2024 16:32:47 +0700 Subject: [PATCH 09/18] Document CTFE behavior of methods that call is_null (cherry picked from commit e6efbb210b037b7e921eac6db5ec79d3c241e2b4) --- library/core/src/ptr/const_ptr.rs | 21 ++++++++++++++++ library/core/src/ptr/mut_ptr.rs | 41 +++++++++++++++++++++++++++++++ library/core/src/ptr/non_null.rs | 7 ++++++ 3 files changed, 69 insertions(+) diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 81c8251d497bb..43eeb58d1570c 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -258,6 +258,13 @@ impl *const T { /// When calling this method, you have to ensure that *either* the pointer is null *or* /// the pointer is [convertible to a reference](crate::ptr#pointer-to-reference-conversion). /// + /// # Panics during const evaluation + /// + /// This method will panic during const evaluation if the pointer cannot be + /// determined to be null or not. See [`is_null`] for more information. + /// + /// [`is_null`]: #method.is_null + /// /// # Examples /// /// ``` @@ -335,6 +342,13 @@ impl *const T { /// When calling this method, you have to ensure that *either* the pointer is null *or* /// the pointer is [convertible to a reference](crate::ptr#pointer-to-reference-conversion). /// + /// # Panics during const evaluation + /// + /// This method will panic during const evaluation if the pointer cannot be + /// determined to be null or not. See [`is_null`] for more information. + /// + /// [`is_null`]: #method.is_null + /// /// # Examples /// /// ``` @@ -1595,6 +1609,13 @@ impl *const [T] { /// /// [valid]: crate::ptr#safety /// [allocated object]: crate::ptr#allocated-object + /// + /// # Panics during const evaluation + /// + /// This method will panic during const evaluation if the pointer cannot be + /// determined to be null or not. See [`is_null`] for more information. + /// + /// [`is_null`]: #method.is_null #[inline] #[unstable(feature = "ptr_as_uninit", issue = "75402")] #[rustc_const_unstable(feature = "ptr_as_uninit", issue = "75402")] diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index 9fce097e7b493..4a2a1123f635e 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -247,6 +247,13 @@ impl *mut T { /// When calling this method, you have to ensure that *either* the pointer is null *or* /// the pointer is [convertible to a reference](crate::ptr#pointer-to-reference-conversion). /// + /// # Panics during const evaluation + /// + /// This method will panic during const evaluation if the pointer cannot be + /// determined to be null or not. See [`is_null`] for more information. + /// + /// [`is_null`]: #method.is_null-1 + /// /// # Examples /// /// ``` @@ -331,6 +338,13 @@ impl *mut T { /// Note that because the created reference is to `MaybeUninit`, the /// source pointer can point to uninitialized memory. /// + /// # Panics during const evaluation + /// + /// This method will panic during const evaluation if the pointer cannot be + /// determined to be null or not. See [`is_null`] for more information. + /// + /// [`is_null`]: #method.is_null-1 + /// /// # Examples /// /// ``` @@ -595,6 +609,12 @@ impl *mut T { /// the pointer is null *or* /// the pointer is [convertible to a reference](crate::ptr#pointer-to-reference-conversion). /// + /// # Panics during const evaluation + /// + /// This method will panic during const evaluation if the pointer cannot be + /// determined to be null or not. See [`is_null`] for more information. + /// + /// [`is_null`]: #method.is_null-1 /// /// # Examples /// @@ -678,6 +698,13 @@ impl *mut T { /// /// When calling this method, you have to ensure that *either* the pointer is null *or* /// the pointer is [convertible to a reference](crate::ptr#pointer-to-reference-conversion). + /// + /// # Panics during const evaluation + /// + /// This method will panic during const evaluation if the pointer cannot be + /// determined to be null or not. See [`is_null`] for more information. + /// + /// [`is_null`]: #method.is_null-1 #[inline] #[unstable(feature = "ptr_as_uninit", issue = "75402")] #[rustc_const_unstable(feature = "ptr_as_uninit", issue = "75402")] @@ -1950,6 +1977,13 @@ impl *mut [T] { /// /// [valid]: crate::ptr#safety /// [allocated object]: crate::ptr#allocated-object + /// + /// # Panics during const evaluation + /// + /// This method will panic during const evaluation if the pointer cannot be + /// determined to be null or not. See [`is_null`] for more information. + /// + /// [`is_null`]: #method.is_null-1 #[inline] #[unstable(feature = "ptr_as_uninit", issue = "75402")] #[rustc_const_unstable(feature = "ptr_as_uninit", issue = "75402")] @@ -2002,6 +2036,13 @@ impl *mut [T] { /// /// [valid]: crate::ptr#safety /// [allocated object]: crate::ptr#allocated-object + /// + /// # Panics during const evaluation + /// + /// This method will panic during const evaluation if the pointer cannot be + /// determined to be null or not. See [`is_null`] for more information. + /// + /// [`is_null`]: #method.is_null-1 #[inline] #[unstable(feature = "ptr_as_uninit", issue = "75402")] #[rustc_const_unstable(feature = "ptr_as_uninit", issue = "75402")] diff --git a/library/core/src/ptr/non_null.rs b/library/core/src/ptr/non_null.rs index 0fb5880fd1a0e..e56d17d4f6f58 100644 --- a/library/core/src/ptr/non_null.rs +++ b/library/core/src/ptr/non_null.rs @@ -202,6 +202,13 @@ impl NonNull { /// Creates a new `NonNull` if `ptr` is non-null. /// + /// # Panics during const evaluation + /// + /// This method will panic during const evaluation if the pointer cannot be + /// determined to be null or not. See [`is_null`] for more information. + /// + /// [`is_null`]: ../primitive.pointer.html#method.is_null-1 + /// /// # Examples /// /// ``` From 3113133cb7321abd41c9af2a74af1d75f75d60ab Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 18 Dec 2024 21:54:50 +0000 Subject: [PATCH 10/18] Add a failing test (cherry picked from commit 2e57394d8004b155c6f74ca4e2a1106dedfcccc4) --- ...hod_1.ElaborateDrops.after.panic-abort.mir | 183 ++++++++++++++++++ ...od_1.ElaborateDrops.after.panic-unwind.mir | 183 ++++++++++++++++++ tests/mir-opt/tail_expr_drop_order_unwind.rs | 36 ++++ 3 files changed, 402 insertions(+) create mode 100644 tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir create mode 100644 tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir create mode 100644 tests/mir-opt/tail_expr_drop_order_unwind.rs diff --git a/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir new file mode 100644 index 0000000000000..54bedfdc0af1e --- /dev/null +++ b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir @@ -0,0 +1,183 @@ +// MIR for `method_1` after ElaborateDrops + +fn method_1(_1: Guard) -> () { + debug g => _1; + let mut _0: (); + let mut _2: std::result::Result; + let mut _3: &Guard; + let _4: &Guard; + let _5: Guard; + let mut _6: &Guard; + let mut _7: isize; + let _8: OtherDrop; + let _9: (); + let mut _10: bool; + let mut _11: bool; + let mut _12: isize; + let mut _13: isize; + let mut _14: isize; + scope 1 { + debug other_drop => _8; + } + scope 2 { + debug err => _9; + } + + bb0: { + _11 = const false; + _10 = const false; + StorageLive(_2); + StorageLive(_3); + StorageLive(_4); + StorageLive(_5); + StorageLive(_6); + _6 = &_1; + _5 = ::clone(move _6) -> [return: bb1, unwind: bb16]; + } + + bb1: { + _11 = const true; + StorageDead(_6); + _4 = &_5; + _3 = &(*_4); + _2 = method_2(move _3) -> [return: bb2, unwind: bb14]; + } + + bb2: { + _10 = const true; + StorageDead(_3); + PlaceMention(_2); + _7 = discriminant(_2); + switchInt(move _7) -> [0: bb5, 1: bb4, otherwise: bb3]; + } + + bb3: { + unreachable; + } + + bb4: { + StorageLive(_9); + _9 = copy ((_2 as Err).0: ()); + _0 = const (); + StorageDead(_9); + goto -> bb7; + } + + bb5: { + StorageLive(_8); + _8 = move ((_2 as Ok).0: OtherDrop); + _0 = const (); + drop(_8) -> [return: bb6, unwind: bb11]; + } + + bb6: { + StorageDead(_8); + goto -> bb7; + } + + bb7: { + backward incompatible drop(_2); + backward incompatible drop(_5); + goto -> bb24; + } + + bb8: { + drop(_5) -> [return: bb9, unwind: bb16]; + } + + bb9: { + _11 = const false; + StorageDead(_5); + StorageDead(_4); + _10 = const false; + StorageDead(_2); + drop(_1) -> [return: bb10, unwind: bb17]; + } + + bb10: { + return; + } + + bb11 (cleanup): { + goto -> bb28; + } + + bb12 (cleanup): { + drop(_5) -> [return: bb13, unwind terminate(cleanup)]; + } + + bb13 (cleanup): { + goto -> bb15; + } + + bb14 (cleanup): { + drop(_5) -> [return: bb15, unwind terminate(cleanup)]; + } + + bb15 (cleanup): { + goto -> bb30; + } + + bb16 (cleanup): { + drop(_1) -> [return: bb17, unwind terminate(cleanup)]; + } + + bb17 (cleanup): { + resume; + } + + bb18: { + goto -> bb8; + } + + bb19 (cleanup): { + goto -> bb15; + } + + bb20 (cleanup): { + goto -> bb15; + } + + bb21: { + goto -> bb18; + } + + bb22: { + goto -> bb18; + } + + bb23 (cleanup): { + goto -> bb15; + } + + bb24: { + _12 = discriminant(_2); + switchInt(move _12) -> [0: bb21, otherwise: bb22]; + } + + bb25 (cleanup): { + _13 = discriminant(_2); + switchInt(move _13) -> [0: bb19, otherwise: bb23]; + } + + bb26 (cleanup): { + goto -> bb12; + } + + bb27 (cleanup): { + goto -> bb12; + } + + bb28 (cleanup): { + _14 = discriminant(_2); + switchInt(move _14) -> [0: bb26, otherwise: bb27]; + } + + bb29 (cleanup): { + drop(_5) -> [return: bb16, unwind terminate(cleanup)]; + } + + bb30 (cleanup): { + switchInt(copy _11) -> [0: bb16, otherwise: bb29]; + } +} diff --git a/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir new file mode 100644 index 0000000000000..54bedfdc0af1e --- /dev/null +++ b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir @@ -0,0 +1,183 @@ +// MIR for `method_1` after ElaborateDrops + +fn method_1(_1: Guard) -> () { + debug g => _1; + let mut _0: (); + let mut _2: std::result::Result; + let mut _3: &Guard; + let _4: &Guard; + let _5: Guard; + let mut _6: &Guard; + let mut _7: isize; + let _8: OtherDrop; + let _9: (); + let mut _10: bool; + let mut _11: bool; + let mut _12: isize; + let mut _13: isize; + let mut _14: isize; + scope 1 { + debug other_drop => _8; + } + scope 2 { + debug err => _9; + } + + bb0: { + _11 = const false; + _10 = const false; + StorageLive(_2); + StorageLive(_3); + StorageLive(_4); + StorageLive(_5); + StorageLive(_6); + _6 = &_1; + _5 = ::clone(move _6) -> [return: bb1, unwind: bb16]; + } + + bb1: { + _11 = const true; + StorageDead(_6); + _4 = &_5; + _3 = &(*_4); + _2 = method_2(move _3) -> [return: bb2, unwind: bb14]; + } + + bb2: { + _10 = const true; + StorageDead(_3); + PlaceMention(_2); + _7 = discriminant(_2); + switchInt(move _7) -> [0: bb5, 1: bb4, otherwise: bb3]; + } + + bb3: { + unreachable; + } + + bb4: { + StorageLive(_9); + _9 = copy ((_2 as Err).0: ()); + _0 = const (); + StorageDead(_9); + goto -> bb7; + } + + bb5: { + StorageLive(_8); + _8 = move ((_2 as Ok).0: OtherDrop); + _0 = const (); + drop(_8) -> [return: bb6, unwind: bb11]; + } + + bb6: { + StorageDead(_8); + goto -> bb7; + } + + bb7: { + backward incompatible drop(_2); + backward incompatible drop(_5); + goto -> bb24; + } + + bb8: { + drop(_5) -> [return: bb9, unwind: bb16]; + } + + bb9: { + _11 = const false; + StorageDead(_5); + StorageDead(_4); + _10 = const false; + StorageDead(_2); + drop(_1) -> [return: bb10, unwind: bb17]; + } + + bb10: { + return; + } + + bb11 (cleanup): { + goto -> bb28; + } + + bb12 (cleanup): { + drop(_5) -> [return: bb13, unwind terminate(cleanup)]; + } + + bb13 (cleanup): { + goto -> bb15; + } + + bb14 (cleanup): { + drop(_5) -> [return: bb15, unwind terminate(cleanup)]; + } + + bb15 (cleanup): { + goto -> bb30; + } + + bb16 (cleanup): { + drop(_1) -> [return: bb17, unwind terminate(cleanup)]; + } + + bb17 (cleanup): { + resume; + } + + bb18: { + goto -> bb8; + } + + bb19 (cleanup): { + goto -> bb15; + } + + bb20 (cleanup): { + goto -> bb15; + } + + bb21: { + goto -> bb18; + } + + bb22: { + goto -> bb18; + } + + bb23 (cleanup): { + goto -> bb15; + } + + bb24: { + _12 = discriminant(_2); + switchInt(move _12) -> [0: bb21, otherwise: bb22]; + } + + bb25 (cleanup): { + _13 = discriminant(_2); + switchInt(move _13) -> [0: bb19, otherwise: bb23]; + } + + bb26 (cleanup): { + goto -> bb12; + } + + bb27 (cleanup): { + goto -> bb12; + } + + bb28 (cleanup): { + _14 = discriminant(_2); + switchInt(move _14) -> [0: bb26, otherwise: bb27]; + } + + bb29 (cleanup): { + drop(_5) -> [return: bb16, unwind terminate(cleanup)]; + } + + bb30 (cleanup): { + switchInt(copy _11) -> [0: bb16, otherwise: bb29]; + } +} diff --git a/tests/mir-opt/tail_expr_drop_order_unwind.rs b/tests/mir-opt/tail_expr_drop_order_unwind.rs new file mode 100644 index 0000000000000..b67b358087502 --- /dev/null +++ b/tests/mir-opt/tail_expr_drop_order_unwind.rs @@ -0,0 +1,36 @@ +// skip-filecheck +// EMIT_MIR_FOR_EACH_PANIC_STRATEGY +// EMIT_MIR tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.mir + +#![deny(tail_expr_drop_order)] + +use std::backtrace::Backtrace; + +#[derive(Clone)] +struct Guard; +impl Drop for Guard { + fn drop(&mut self) { + println!("Drop!"); + } +} + +#[derive(Clone)] +struct OtherDrop; +impl Drop for OtherDrop { + fn drop(&mut self) { + println!("Drop!"); + } +} + +fn method_1(g: Guard) { + match method_2(&g.clone()) { + Ok(other_drop) => { + // repro needs something else being dropped too. + }, + Err(err) => {}, + } +} + +fn method_2(_: &Guard) -> Result { + panic!("Method 2 panics!"); +} From 432bd9de3259e5cd73b0e798a05ddffe8eea7759 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 18 Dec 2024 21:57:20 +0000 Subject: [PATCH 11/18] Separate DropKind::ForLint (cherry picked from commit 5e079011eafbb1d5fc779c14c7a29d4a620574f9) --- compiler/rustc_mir_build/src/build/scope.rs | 108 +++++++++++------- ...hod_1.ElaborateDrops.after.panic-abort.mir | 74 ++++-------- ...od_1.ElaborateDrops.after.panic-unwind.mir | 74 ++++-------- tests/mir-opt/tail_expr_drop_order_unwind.rs | 4 +- 4 files changed, 121 insertions(+), 139 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 636e47b7ad2f2..4530df00c723b 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -151,15 +151,13 @@ struct DropData { /// Whether this is a value Drop or a StorageDead. kind: DropKind, - - /// Whether this is a backwards-incompatible drop lint - backwards_incompatible_lint: bool, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub(crate) enum DropKind { Value, Storage, + ForLint, } #[derive(Debug)] @@ -248,7 +246,7 @@ impl Scope { /// use of optimizations in the MIR coroutine transform. fn needs_cleanup(&self) -> bool { self.drops.iter().any(|drop| match drop.kind { - DropKind::Value => true, + DropKind::Value | DropKind::ForLint => true, DropKind::Storage => false, }) } @@ -277,12 +275,8 @@ impl DropTree { // represents the block in the tree that should be jumped to once all // of the required drops have been performed. let fake_source_info = SourceInfo::outermost(DUMMY_SP); - let fake_data = DropData { - source_info: fake_source_info, - local: Local::MAX, - kind: DropKind::Storage, - backwards_incompatible_lint: false, - }; + let fake_data = + DropData { source_info: fake_source_info, local: Local::MAX, kind: DropKind::Storage }; let drops = IndexVec::from_raw(vec![DropNode { data: fake_data, next: DropIdx::MAX }]); Self { drops, entry_points: Vec::new(), existing_drops_map: FxHashMap::default() } } @@ -411,6 +405,27 @@ impl DropTree { }; cfg.terminate(block, drop_node.data.source_info, terminator); } + DropKind::ForLint => { + let stmt = Statement { + source_info: drop_node.data.source_info, + kind: StatementKind::BackwardIncompatibleDropHint { + place: Box::new(drop_node.data.local.into()), + reason: BackwardIncompatibleDropReason::Edition2024, + }, + }; + cfg.push(block, stmt); + let target = blocks[drop_node.next].unwrap(); + if target != block { + // Diagnostics don't use this `Span` but debuginfo + // might. Since we don't want breakpoints to be placed + // here, especially when this is on an unwind path, we + // use `DUMMY_SP`. + let source_info = + SourceInfo { span: DUMMY_SP, ..drop_node.data.source_info }; + let terminator = TerminatorKind::Goto { target }; + cfg.terminate(block, source_info, terminator); + } + } // Root nodes don't correspond to a drop. DropKind::Storage if drop_idx == ROOT_NODE => {} DropKind::Storage => { @@ -770,12 +785,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let local = place.as_local().unwrap_or_else(|| bug!("projection in tail call args")); - Some(DropData { - source_info, - local, - kind: DropKind::Value, - backwards_incompatible_lint: false, - }) + Some(DropData { source_info, local, kind: DropKind::Value }) } Operand::Constant(_) => None, }) @@ -822,6 +832,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }); block = next; } + DropKind::ForLint => { + self.cfg.push(block, Statement { + source_info, + kind: StatementKind::BackwardIncompatibleDropHint { + place: Box::new(local.into()), + reason: BackwardIncompatibleDropReason::Edition2024, + }, + }); + } DropKind::Storage => { // Only temps and vars need their storage dead. assert!(local.index() > self.arg_count); @@ -1021,7 +1040,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { drop_kind: DropKind, ) { let needs_drop = match drop_kind { - DropKind::Value => { + DropKind::Value | DropKind::ForLint => { if !self.local_decls[local].ty.needs_drop(self.tcx, self.typing_env()) { return; } @@ -1101,7 +1120,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { source_info: SourceInfo { span: scope_end, scope: scope.source_scope }, local, kind: drop_kind, - backwards_incompatible_lint: false, }); return; @@ -1135,8 +1153,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { scope.drops.push(DropData { source_info: SourceInfo { span: scope_end, scope: scope.source_scope }, local, - kind: DropKind::Value, - backwards_incompatible_lint: true, + kind: DropKind::ForLint, }); return; @@ -1430,25 +1447,38 @@ fn build_scope_drops<'tcx>( continue; } - if drop_data.backwards_incompatible_lint { - cfg.push(block, Statement { - source_info, - kind: StatementKind::BackwardIncompatibleDropHint { - place: Box::new(local.into()), - reason: BackwardIncompatibleDropReason::Edition2024, - }, - }); - } else { - unwind_drops.add_entry_point(block, unwind_to); - let next = cfg.start_new_block(); - cfg.terminate(block, source_info, TerminatorKind::Drop { - place: local.into(), - target: next, - unwind: UnwindAction::Continue, - replace: false, - }); - block = next; + unwind_drops.add_entry_point(block, unwind_to); + let next = cfg.start_new_block(); + cfg.terminate(block, source_info, TerminatorKind::Drop { + place: local.into(), + target: next, + unwind: UnwindAction::Continue, + replace: false, + }); + block = next; + } + DropKind::ForLint => { + // If the operand has been moved, and we are not on an unwind + // path, then don't generate the drop. (We only take this into + // account for non-unwind paths so as not to disturb the + // caching mechanism.) + if scope.moved_locals.iter().any(|&o| o == local) { + continue; + } + + if storage_dead_on_unwind { + debug_assert_eq!(unwind_drops.drops[unwind_to].data.local, drop_data.local); + debug_assert_eq!(unwind_drops.drops[unwind_to].data.kind, drop_data.kind); + unwind_to = unwind_drops.drops[unwind_to].next; } + + cfg.push(block, Statement { + source_info, + kind: StatementKind::BackwardIncompatibleDropHint { + place: Box::new(local.into()), + reason: BackwardIncompatibleDropReason::Edition2024, + }, + }); } DropKind::Storage => { if storage_dead_on_unwind { @@ -1500,7 +1530,7 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { unwind_indices.push(unwind_indices[drop_node.next]); } } - DropKind::Value => { + DropKind::Value | DropKind::ForLint => { let unwind_drop = self .scopes .unwind_drops diff --git a/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir index 54bedfdc0af1e..e9bbe30bd774f 100644 --- a/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir +++ b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-abort.mir @@ -12,10 +12,9 @@ fn method_1(_1: Guard) -> () { let _8: OtherDrop; let _9: (); let mut _10: bool; - let mut _11: bool; + let mut _11: isize; let mut _12: isize; let mut _13: isize; - let mut _14: isize; scope 1 { debug other_drop => _8; } @@ -24,7 +23,6 @@ fn method_1(_1: Guard) -> () { } bb0: { - _11 = const false; _10 = const false; StorageLive(_2); StorageLive(_3); @@ -32,15 +30,14 @@ fn method_1(_1: Guard) -> () { StorageLive(_5); StorageLive(_6); _6 = &_1; - _5 = ::clone(move _6) -> [return: bb1, unwind: bb16]; + _5 = ::clone(move _6) -> [return: bb1, unwind: bb13]; } bb1: { - _11 = const true; StorageDead(_6); _4 = &_5; _3 = &(*_4); - _2 = method_2(move _3) -> [return: bb2, unwind: bb14]; + _2 = method_2(move _3) -> [return: bb2, unwind: bb12]; } bb2: { @@ -78,20 +75,19 @@ fn method_1(_1: Guard) -> () { bb7: { backward incompatible drop(_2); backward incompatible drop(_5); - goto -> bb24; + goto -> bb21; } bb8: { - drop(_5) -> [return: bb9, unwind: bb16]; + drop(_5) -> [return: bb9, unwind: bb13]; } bb9: { - _11 = const false; StorageDead(_5); StorageDead(_4); _10 = const false; StorageDead(_2); - drop(_1) -> [return: bb10, unwind: bb17]; + drop(_1) -> [return: bb10, unwind: bb14]; } bb10: { @@ -99,7 +95,7 @@ fn method_1(_1: Guard) -> () { } bb11 (cleanup): { - goto -> bb28; + goto -> bb25; } bb12 (cleanup): { @@ -107,77 +103,57 @@ fn method_1(_1: Guard) -> () { } bb13 (cleanup): { - goto -> bb15; + drop(_1) -> [return: bb14, unwind terminate(cleanup)]; } bb14 (cleanup): { - drop(_5) -> [return: bb15, unwind terminate(cleanup)]; + resume; } - bb15 (cleanup): { - goto -> bb30; + bb15: { + goto -> bb8; } bb16 (cleanup): { - drop(_1) -> [return: bb17, unwind terminate(cleanup)]; + goto -> bb12; } bb17 (cleanup): { - resume; + goto -> bb12; } bb18: { - goto -> bb8; + goto -> bb15; } - bb19 (cleanup): { + bb19: { goto -> bb15; } bb20 (cleanup): { - goto -> bb15; + goto -> bb12; } bb21: { - goto -> bb18; + _11 = discriminant(_2); + switchInt(move _11) -> [0: bb18, otherwise: bb19]; } - bb22: { - goto -> bb18; - } - - bb23 (cleanup): { - goto -> bb15; - } - - bb24: { + bb22 (cleanup): { _12 = discriminant(_2); - switchInt(move _12) -> [0: bb21, otherwise: bb22]; + switchInt(move _12) -> [0: bb16, otherwise: bb20]; } - bb25 (cleanup): { - _13 = discriminant(_2); - switchInt(move _13) -> [0: bb19, otherwise: bb23]; - } - - bb26 (cleanup): { + bb23 (cleanup): { goto -> bb12; } - bb27 (cleanup): { + bb24 (cleanup): { goto -> bb12; } - bb28 (cleanup): { - _14 = discriminant(_2); - switchInt(move _14) -> [0: bb26, otherwise: bb27]; - } - - bb29 (cleanup): { - drop(_5) -> [return: bb16, unwind terminate(cleanup)]; - } - - bb30 (cleanup): { - switchInt(copy _11) -> [0: bb16, otherwise: bb29]; + bb25 (cleanup): { + _13 = discriminant(_2); + switchInt(move _13) -> [0: bb23, otherwise: bb24]; } } diff --git a/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir index 54bedfdc0af1e..e9bbe30bd774f 100644 --- a/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir +++ b/tests/mir-opt/tail_expr_drop_order_unwind.method_1.ElaborateDrops.after.panic-unwind.mir @@ -12,10 +12,9 @@ fn method_1(_1: Guard) -> () { let _8: OtherDrop; let _9: (); let mut _10: bool; - let mut _11: bool; + let mut _11: isize; let mut _12: isize; let mut _13: isize; - let mut _14: isize; scope 1 { debug other_drop => _8; } @@ -24,7 +23,6 @@ fn method_1(_1: Guard) -> () { } bb0: { - _11 = const false; _10 = const false; StorageLive(_2); StorageLive(_3); @@ -32,15 +30,14 @@ fn method_1(_1: Guard) -> () { StorageLive(_5); StorageLive(_6); _6 = &_1; - _5 = ::clone(move _6) -> [return: bb1, unwind: bb16]; + _5 = ::clone(move _6) -> [return: bb1, unwind: bb13]; } bb1: { - _11 = const true; StorageDead(_6); _4 = &_5; _3 = &(*_4); - _2 = method_2(move _3) -> [return: bb2, unwind: bb14]; + _2 = method_2(move _3) -> [return: bb2, unwind: bb12]; } bb2: { @@ -78,20 +75,19 @@ fn method_1(_1: Guard) -> () { bb7: { backward incompatible drop(_2); backward incompatible drop(_5); - goto -> bb24; + goto -> bb21; } bb8: { - drop(_5) -> [return: bb9, unwind: bb16]; + drop(_5) -> [return: bb9, unwind: bb13]; } bb9: { - _11 = const false; StorageDead(_5); StorageDead(_4); _10 = const false; StorageDead(_2); - drop(_1) -> [return: bb10, unwind: bb17]; + drop(_1) -> [return: bb10, unwind: bb14]; } bb10: { @@ -99,7 +95,7 @@ fn method_1(_1: Guard) -> () { } bb11 (cleanup): { - goto -> bb28; + goto -> bb25; } bb12 (cleanup): { @@ -107,77 +103,57 @@ fn method_1(_1: Guard) -> () { } bb13 (cleanup): { - goto -> bb15; + drop(_1) -> [return: bb14, unwind terminate(cleanup)]; } bb14 (cleanup): { - drop(_5) -> [return: bb15, unwind terminate(cleanup)]; + resume; } - bb15 (cleanup): { - goto -> bb30; + bb15: { + goto -> bb8; } bb16 (cleanup): { - drop(_1) -> [return: bb17, unwind terminate(cleanup)]; + goto -> bb12; } bb17 (cleanup): { - resume; + goto -> bb12; } bb18: { - goto -> bb8; + goto -> bb15; } - bb19 (cleanup): { + bb19: { goto -> bb15; } bb20 (cleanup): { - goto -> bb15; + goto -> bb12; } bb21: { - goto -> bb18; + _11 = discriminant(_2); + switchInt(move _11) -> [0: bb18, otherwise: bb19]; } - bb22: { - goto -> bb18; - } - - bb23 (cleanup): { - goto -> bb15; - } - - bb24: { + bb22 (cleanup): { _12 = discriminant(_2); - switchInt(move _12) -> [0: bb21, otherwise: bb22]; + switchInt(move _12) -> [0: bb16, otherwise: bb20]; } - bb25 (cleanup): { - _13 = discriminant(_2); - switchInt(move _13) -> [0: bb19, otherwise: bb23]; - } - - bb26 (cleanup): { + bb23 (cleanup): { goto -> bb12; } - bb27 (cleanup): { + bb24 (cleanup): { goto -> bb12; } - bb28 (cleanup): { - _14 = discriminant(_2); - switchInt(move _14) -> [0: bb26, otherwise: bb27]; - } - - bb29 (cleanup): { - drop(_5) -> [return: bb16, unwind terminate(cleanup)]; - } - - bb30 (cleanup): { - switchInt(copy _11) -> [0: bb16, otherwise: bb29]; + bb25 (cleanup): { + _13 = discriminant(_2); + switchInt(move _13) -> [0: bb23, otherwise: bb24]; } } diff --git a/tests/mir-opt/tail_expr_drop_order_unwind.rs b/tests/mir-opt/tail_expr_drop_order_unwind.rs index b67b358087502..065e08c340963 100644 --- a/tests/mir-opt/tail_expr_drop_order_unwind.rs +++ b/tests/mir-opt/tail_expr_drop_order_unwind.rs @@ -26,8 +26,8 @@ fn method_1(g: Guard) { match method_2(&g.clone()) { Ok(other_drop) => { // repro needs something else being dropped too. - }, - Err(err) => {}, + } + Err(err) => {} } } From 8018b401dacf25aa92b6bd196b2cf9c4a20a4c6f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 19 Dec 2024 13:53:35 +0000 Subject: [PATCH 12/18] explain how `build_scope_drops` works (cherry picked from commit b5350610608b724a3f152ccd889a78f82860ed69) --- compiler/rustc_mir_build/src/build/scope.rs | 40 +++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 4530df00c723b..7e7e38832e8c2 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -1396,12 +1396,23 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } /// Builds drops for `pop_scope` and `leave_top_scope`. +/// +/// # Parameters +/// +/// * `unwind_drops`, the drop tree data structure storing what needs to be cleaned up if unwind occurs +/// * `scope`, describes the drops that will occur on exiting the scope in regular execution +/// * `block`, the block to branch to once drops are complete (assuming no unwind occurs) +/// * `unwind_to`, describes the drops that would occur at this point in the code if a +/// panic occurred (a subset of the drops in `scope`, since we sometimes elide StorageDead and other +/// instructions on unwinding) +/// * `storage_dead_on_unwind`, if true, then we should emit `StorageDead` even when unwinding +/// * `arg_count`, number of MIR local variables corresponding to fn arguments (used to assert that we don't drop those) fn build_scope_drops<'tcx>( cfg: &mut CFG<'tcx>, unwind_drops: &mut DropTree, scope: &Scope, - mut block: BasicBlock, - mut unwind_to: DropIdx, + block: BasicBlock, + unwind_to: DropIdx, storage_dead_on_unwind: bool, arg_count: usize, ) -> BlockAnd<()> { @@ -1425,6 +1436,18 @@ fn build_scope_drops<'tcx>( // statement. For other functions we don't worry about StorageDead. The // drops for the unwind path should have already been generated by // `diverge_cleanup_gen`. + + // `unwind_to` indicates what needs to be dropped should unwinding occur. + // This is a subset of what needs to be dropped when exiting the scope. + // As we unwind the scope, we will also move `unwind_to` backwards to match, + // so that we can use it should a destructor panic. + let mut unwind_to = unwind_to; + + // The block that we should jump to after drops complete. We start by building the final drop (`drops[n]` + // in the diagram above) and then build the drops (e.g., `drop[1]`, `drop[0]`) that come before it. + // block begins as the successor of `drops[n]` and then becomes `drops[n]` so that `drops[n-1]` + // will branch to `drops[n]`. + let mut block = block; for drop_data in scope.drops.iter().rev() { let source_info = drop_data.source_info; @@ -1435,6 +1458,9 @@ fn build_scope_drops<'tcx>( // `unwind_to` should drop the value that we're about to // schedule. If dropping this value panics, then we continue // with the *next* value on the unwind path. + // + // We adjust this BEFORE we create the drop (e.g., `drops[n]`) + // because `drops[n]` should unwind to `drops[n-1]`. debug_assert_eq!(unwind_drops.drops[unwind_to].data.local, drop_data.local); debug_assert_eq!(unwind_drops.drops[unwind_to].data.kind, drop_data.kind); unwind_to = unwind_drops.drops[unwind_to].next; @@ -1466,6 +1492,11 @@ fn build_scope_drops<'tcx>( continue; } + // As in the `DropKind::Storage` case below: + // normally lint-related drops are not emitted for unwind, + // so we can just leave `unwind_to` unmodified, but in some + // cases we emit things ALSO on the unwind path, so we need to adjust + // `unwind_to` in that case. if storage_dead_on_unwind { debug_assert_eq!(unwind_drops.drops[unwind_to].data.local, drop_data.local); debug_assert_eq!(unwind_drops.drops[unwind_to].data.kind, drop_data.kind); @@ -1481,6 +1512,11 @@ fn build_scope_drops<'tcx>( }); } DropKind::Storage => { + // Ordinarily, storage-dead nodes are not emitted on unwind, so we don't + // need to adjust `unwind_to` on this path. However, in some specific cases + // we *do* emit storage-dead nodes on the unwind path, and in that case now that + // the storage-dead has completed, we need to adjust the `unwind_to` pointer + // so that any future drops we emit will not register storage-dead. if storage_dead_on_unwind { debug_assert_eq!(unwind_drops.drops[unwind_to].data.local, drop_data.local); debug_assert_eq!(unwind_drops.drops[unwind_to].data.kind, drop_data.kind); From 6fe7742fdfb9e1a18cfa2f6b197c1a10f9fd29a7 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 19 Dec 2024 14:32:25 +0000 Subject: [PATCH 13/18] pacify merciless fmt (cherry picked from commit 6564403641afde8bf445914ec2996fe7219289ab) --- compiler/rustc_mir_build/src/build/scope.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 7e7e38832e8c2..e352db2585f67 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -1396,9 +1396,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } /// Builds drops for `pop_scope` and `leave_top_scope`. -/// +/// /// # Parameters -/// +/// /// * `unwind_drops`, the drop tree data structure storing what needs to be cleaned up if unwind occurs /// * `scope`, describes the drops that will occur on exiting the scope in regular execution /// * `block`, the block to branch to once drops are complete (assuming no unwind occurs) @@ -1436,14 +1436,14 @@ fn build_scope_drops<'tcx>( // statement. For other functions we don't worry about StorageDead. The // drops for the unwind path should have already been generated by // `diverge_cleanup_gen`. - + // `unwind_to` indicates what needs to be dropped should unwinding occur. // This is a subset of what needs to be dropped when exiting the scope. // As we unwind the scope, we will also move `unwind_to` backwards to match, // so that we can use it should a destructor panic. let mut unwind_to = unwind_to; - // The block that we should jump to after drops complete. We start by building the final drop (`drops[n]` + // The block that we should jump to after drops complete. We start by building the final drop (`drops[n]` // in the diagram above) and then build the drops (e.g., `drop[1]`, `drop[0]`) that come before it. // block begins as the successor of `drops[n]` and then becomes `drops[n]` so that `drops[n-1]` // will branch to `drops[n]`. @@ -1492,7 +1492,7 @@ fn build_scope_drops<'tcx>( continue; } - // As in the `DropKind::Storage` case below: + // As in the `DropKind::Storage` case below: // normally lint-related drops are not emitted for unwind, // so we can just leave `unwind_to` unmodified, but in some // cases we emit things ALSO on the unwind path, so we need to adjust From f757f962962ce4ce2a004a6b32c7ec33996b5121 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Thu, 19 Dec 2024 18:49:52 +0800 Subject: [PATCH 14/18] Bump compiler `cc` to 1.2.5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `cc` 1.2.4 contains a fix to address [rustc uses wrong build tools when compiling from MSVC #133794](https://github.com/rust-lang/rust/issues/133794). See . - `cc` 1.2.5 contains a fix to also check linking when testing if certain compiler flags are supported, which fixed an issue that was causing previous compiler `cc` bumps to fail. See . Co-authored-by: David Lönnhager (cherry picked from commit 3775d220af5a666b4239c9fdd1e0184d3df0c7a8) --- Cargo.lock | 4 ++-- compiler/rustc_codegen_ssa/Cargo.toml | 4 +++- compiler/rustc_llvm/Cargo.toml | 4 +++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e4dcf13a84b73..f34baa8077d87 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -411,9 +411,9 @@ version = "0.1.0" [[package]] name = "cc" -version = "1.1.34" +version = "1.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67b9470d453346108f93a59222a9a1a5724db32d0a4727b7ab7ace4b4d822dc9" +checksum = "c31a0499c1dc64f458ad13872de75c0eb7e3fdb0e67964610c914b034fc5956e" dependencies = [ "shlex", ] diff --git a/compiler/rustc_codegen_ssa/Cargo.toml b/compiler/rustc_codegen_ssa/Cargo.toml index b898cfec79669..eb9a8ebbd3bda 100644 --- a/compiler/rustc_codegen_ssa/Cargo.toml +++ b/compiler/rustc_codegen_ssa/Cargo.toml @@ -8,7 +8,9 @@ edition = "2021" ar_archive_writer = "0.4.2" arrayvec = { version = "0.7", default-features = false } bitflags = "2.4.1" -cc = "1.1.23" +# Pinned so `cargo update` bumps don't cause breakage. Please also update the +# `cc` in `rustc_llvm` if you update the `cc` here. +cc = "=1.2.5" either = "1.5.0" itertools = "0.12" jobserver = "0.1.28" diff --git a/compiler/rustc_llvm/Cargo.toml b/compiler/rustc_llvm/Cargo.toml index b29d6b79250f3..79a6454dbb94a 100644 --- a/compiler/rustc_llvm/Cargo.toml +++ b/compiler/rustc_llvm/Cargo.toml @@ -10,5 +10,7 @@ libc = "0.2.73" [build-dependencies] # tidy-alphabetical-start -cc = "1.1.23" +# Pinned so `cargo update` bumps don't cause breakage. Please also update the +# pinned `cc` in `rustc_codegen_ssa` if you update `cc` here. +cc = "=1.2.5" # tidy-alphabetical-end From 730b84074daddf85905b70f6df9ce438b02cdedc Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 20 Dec 2024 17:47:53 +0000 Subject: [PATCH 15/18] Handle DropKind::ForLint in coroutines correctly (cherry picked from commit 42d1a4c48bd2c914f30fc6e97f9a1beda0c97729) --- compiler/rustc_mir_build/src/build/scope.rs | 20 +++---- ...ail_expr_drop_order-on-coroutine-unwind.rs | 29 +++++++++++ ...expr_drop_order-on-coroutine-unwind.stderr | 52 +++++++++++++++++++ 3 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.rs create mode 100644 tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.stderr diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index e352db2585f67..c2af064925ce0 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -1484,14 +1484,6 @@ fn build_scope_drops<'tcx>( block = next; } DropKind::ForLint => { - // If the operand has been moved, and we are not on an unwind - // path, then don't generate the drop. (We only take this into - // account for non-unwind paths so as not to disturb the - // caching mechanism.) - if scope.moved_locals.iter().any(|&o| o == local) { - continue; - } - // As in the `DropKind::Storage` case below: // normally lint-related drops are not emitted for unwind, // so we can just leave `unwind_to` unmodified, but in some @@ -1503,6 +1495,14 @@ fn build_scope_drops<'tcx>( unwind_to = unwind_drops.drops[unwind_to].next; } + // If the operand has been moved, and we are not on an unwind + // path, then don't generate the drop. (We only take this into + // account for non-unwind paths so as not to disturb the + // caching mechanism.) + if scope.moved_locals.iter().any(|&o| o == local) { + continue; + } + cfg.push(block, Statement { source_info, kind: StatementKind::BackwardIncompatibleDropHint { @@ -1555,7 +1555,7 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { let mut unwind_indices = IndexVec::from_elem_n(unwind_target, 1); for (drop_idx, drop_node) in drops.drops.iter_enumerated().skip(1) { match drop_node.data.kind { - DropKind::Storage => { + DropKind::Storage | DropKind::ForLint => { if is_coroutine { let unwind_drop = self .scopes @@ -1566,7 +1566,7 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { unwind_indices.push(unwind_indices[drop_node.next]); } } - DropKind::Value | DropKind::ForLint => { + DropKind::Value => { let unwind_drop = self .scopes .unwind_drops diff --git a/tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.rs b/tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.rs new file mode 100644 index 0000000000000..8d85cee19fd07 --- /dev/null +++ b/tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.rs @@ -0,0 +1,29 @@ +//@ edition: 2021 +//@ build-fail + +// Make sure we don't ICE when emitting the "lint" drop statement +// used for tail_expr_drop_order. + +#![deny(tail_expr_drop_order)] + +struct Drop; +impl std::ops::Drop for Drop { + fn drop(&mut self) {} +} + +async fn func() -> Result<(), Drop> { + todo!() +} + +async fn retry_db() -> Result<(), Drop> { + loop { + match func().await { + //~^ ERROR relative drop order changing in Rust 2024 + //~| WARNING this changes meaning in Rust 2024 + Ok(()) => return Ok(()), + Err(e) => {} + } + } +} + +fn main() {} diff --git a/tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.stderr b/tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.stderr new file mode 100644 index 0000000000000..37220c40b8b19 --- /dev/null +++ b/tests/ui/drop/tail_expr_drop_order-on-coroutine-unwind.stderr @@ -0,0 +1,52 @@ +error: relative drop order changing in Rust 2024 + --> $DIR/tail_expr_drop_order-on-coroutine-unwind.rs:20:15 + | +LL | match func().await { + | ^^^^^^^----- + | | | + | | this value will be stored in a temporary; let us call it `#1` + | | `#1` will be dropped later as of Edition 2024 + | this value will be stored in a temporary; let us call it `#2` + | up until Edition 2021 `#2` is dropped last but will be dropped earlier in Edition 2024 +... +LL | Err(e) => {} + | - + | | + | `e` calls a custom destructor + | `e` will be dropped later as of Edition 2024 +LL | } +LL | } + | - now the temporary value is dropped here, before the local variables in the block or statement + | + = warning: this changes meaning in Rust 2024 + = note: for more information, see issue #123739 +note: `#2` invokes this custom destructor + --> $DIR/tail_expr_drop_order-on-coroutine-unwind.rs:10:1 + | +LL | / impl std::ops::Drop for Drop { +LL | | fn drop(&mut self) {} +LL | | } + | |_^ +note: `#1` invokes this custom destructor + --> $DIR/tail_expr_drop_order-on-coroutine-unwind.rs:10:1 + | +LL | / impl std::ops::Drop for Drop { +LL | | fn drop(&mut self) {} +LL | | } + | |_^ +note: `e` invokes this custom destructor + --> $DIR/tail_expr_drop_order-on-coroutine-unwind.rs:10:1 + | +LL | / impl std::ops::Drop for Drop { +LL | | fn drop(&mut self) {} +LL | | } + | |_^ + = note: most of the time, changing drop order is harmless; inspect the `impl Drop`s for side effects like releasing locks or sending messages +note: the lint level is defined here + --> $DIR/tail_expr_drop_order-on-coroutine-unwind.rs:7:9 + | +LL | #![deny(tail_expr_drop_order)] + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + From 6cc24819e50794c81974635145e9d924cc27cada Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 26 Dec 2024 08:58:17 -0700 Subject: [PATCH 16/18] docs: inline `std::ffi::c_str` types to `std::ffi` Rustdoc has no way to show that an item is stable, but only at a different path. `std::ffi::c_str::NulError` is not stable, but `std::ffi::NulError` is. To avoid marking these types as unstable when someone just wants to follow a link from `CString`, inline them into their stable paths. (cherry picked from commit 40b0026a2f1c50e88909f49e8ef8c7ae074f5a9b) --- library/std/src/ffi/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/ffi/mod.rs b/library/std/src/ffi/mod.rs index 469136be8838a..7d7cce09a3f09 100644 --- a/library/std/src/ffi/mod.rs +++ b/library/std/src/ffi/mod.rs @@ -179,19 +179,19 @@ pub use core::ffi::{ c_ulong, c_ulonglong, c_ushort, }; -#[doc(no_inline)] +#[doc(inline)] #[stable(feature = "cstr_from_bytes_until_nul", since = "1.69.0")] pub use self::c_str::FromBytesUntilNulError; -#[doc(no_inline)] +#[doc(inline)] #[stable(feature = "cstr_from_bytes", since = "1.10.0")] pub use self::c_str::FromBytesWithNulError; -#[doc(no_inline)] +#[doc(inline)] #[stable(feature = "cstring_from_vec_with_nul", since = "1.58.0")] pub use self::c_str::FromVecWithNulError; -#[doc(no_inline)] +#[doc(inline)] #[stable(feature = "cstring_into", since = "1.7.0")] pub use self::c_str::IntoStringError; -#[doc(no_inline)] +#[doc(inline)] #[stable(feature = "rust1", since = "1.0.0")] pub use self::c_str::NulError; #[doc(inline)] From 0605512eadf40839c69d4fadff3febbc2f3065d6 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 26 Dec 2024 15:51:45 -0700 Subject: [PATCH 17/18] docs: inline `core::ffi::c_str` types to `core::ffi` (cherry picked from commit fc8a541eaa4b6555d948c382b75677b0e17040fa) --- library/core/src/ffi/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/ffi/mod.rs b/library/core/src/ffi/mod.rs index dc107c5d22cdd..a963778bcc42c 100644 --- a/library/core/src/ffi/mod.rs +++ b/library/core/src/ffi/mod.rs @@ -12,10 +12,10 @@ #[doc(inline)] #[stable(feature = "core_c_str", since = "1.64.0")] pub use self::c_str::CStr; -#[doc(no_inline)] +#[doc(inline)] #[stable(feature = "cstr_from_bytes_until_nul", since = "1.69.0")] pub use self::c_str::FromBytesUntilNulError; -#[doc(no_inline)] +#[doc(inline)] #[stable(feature = "core_c_str", since = "1.64.0")] pub use self::c_str::FromBytesWithNulError; use crate::fmt; From f72c836093960d1f2027e4316ad099da049ceaa0 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sat, 28 Dec 2024 15:42:39 +0100 Subject: [PATCH 18/18] docs: inline `alloc::ffi::c_str` types to `alloc::ffi` (cherry picked from commit 11ad6ff3cb3cb3da0040541877c716ddc38ec388) --- library/alloc/src/ffi/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/ffi/mod.rs b/library/alloc/src/ffi/mod.rs index 4f9dc40a3cfc9..695d7ad07cf76 100644 --- a/library/alloc/src/ffi/mod.rs +++ b/library/alloc/src/ffi/mod.rs @@ -83,7 +83,7 @@ #[doc(inline)] #[stable(feature = "alloc_c_string", since = "1.64.0")] pub use self::c_str::CString; -#[doc(no_inline)] +#[doc(inline)] #[stable(feature = "alloc_c_string", since = "1.64.0")] pub use self::c_str::{FromVecWithNulError, IntoStringError, NulError};