From 6a679ffc8125419123e416fbeb14a4e118e9f366 Mon Sep 17 00:00:00 2001 From: The8472 Date: Sat, 13 Feb 2021 00:54:17 +0100 Subject: [PATCH 01/25] bypass auto_da_alloc for metadata files --- compiler/rustc_interface/src/passes.rs | 2 +- compiler/rustc_interface/src/util.rs | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 56aa3939b22d..c1ae450483e6 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -983,7 +983,7 @@ fn encode_and_write_metadata( .unwrap_or_else(|err| tcx.sess.fatal(&format!("couldn't create a temp dir: {}", err))); let metadata_tmpdir = MaybeTempDir::new(metadata_tmpdir, tcx.sess.opts.cg.save_temps); let metadata_filename = emit_metadata(tcx.sess, &metadata, &metadata_tmpdir); - if let Err(e) = fs::rename(&metadata_filename, &out_filename) { + if let Err(e) = util::non_durable_rename(&metadata_filename, &out_filename) { tcx.sess.fatal(&format!("failed to write {}: {}", out_filename.display(), e)); } if tcx.sess.opts.json_artifact_notifications { diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index b7dc539c6d60..dc2c90472c0e 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -670,6 +670,24 @@ pub fn build_output_filenames( } } +#[cfg(not(target_os = "linux"))] +pub fn non_durable_rename(src: &Path, dst: &Path) -> std::io::Result<()> { + std::fs::rename(src, dst) +} + +/// This function attempts to bypass the auto_da_alloc heuristic implemented by some filesystems +/// such as btrfs and ext4. When renaming over a file that already exists then they will "helpfully" +/// write back the source file before committing the rename in case a developer forgot some of +/// the fsyncs in the open/write/fsync(file)/rename/fsync(dir) dance for atomic file updates. +/// +/// To avoid triggering this heuristic we delete the destination first, if it exists. +/// The cost of an extra syscall is much lower than getting descheduled for the sync IO. +#[cfg(target_os = "linux")] +pub fn non_durable_rename(src: &Path, dst: &Path) -> std::io::Result<()> { + let _ = std::fs::remove_file(dst); + std::fs::rename(src, dst) +} + // Note: Also used by librustdoc, see PR #43348. Consider moving this struct elsewhere. // // FIXME: Currently the `everybody_loops` transformation is not applied to: From 9d25ccf0e8dd69f4086ad8b59bb8785318926b05 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 11 Oct 2020 18:01:32 -0400 Subject: [PATCH 02/25] Change built-in kernel targets to be os = none throughout Whether for Rust's own `target_os`, LLVM's triples, or GNU config's, the OS-related have fields have been for code running *on* that OS, not code that is *part* of the OS. The difference is huge, as syscall interfaces are nothing like freestanding interfaces. Kernels are (hypervisors and other more exotic situations aside) freestanding programs that use the interfaces provided by the hardware. It's *those* interfaces, the ones external to the program being built and its software dependencies, that are the content of the target. For the Linux Kernel in particular, `target_env: "gnu"` is removed for the same reason: that `-gnu` refers to glibc or GNU/linux, neither of which applies to the kernel itself. Relates to #74247 Thanks @ojeda for catching some things. --- compiler/rustc_target/src/spec/mod.rs | 5 +++-- ...ermit_kernel.rs => x86_64_unknown_none_hermitkernel.rs} | 2 +- ..._linux_kernel.rs => x86_64_unknown_none_linuxkernel.rs} | 7 +++++-- library/unwind/src/lib.rs | 2 +- src/doc/rustc/src/platform-support.md | 4 ++-- 5 files changed, 12 insertions(+), 8 deletions(-) rename compiler/rustc_target/src/spec/{x86_64_unknown_hermit_kernel.rs => x86_64_unknown_none_hermitkernel.rs} (91%) rename compiler/rustc_target/src/spec/{x86_64_linux_kernel.rs => x86_64_unknown_none_linuxkernel.rs} (76%) diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 039e9a8b2745..eb1916637529 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -679,7 +679,7 @@ supported_targets! { ("thumbv7neon-linux-androideabi", thumbv7neon_linux_androideabi), ("aarch64-linux-android", aarch64_linux_android), - ("x86_64-linux-kernel", x86_64_linux_kernel), + ("x86_64-unknown-none-linuxkernel", x86_64_unknown_none_linuxkernel), ("aarch64-unknown-freebsd", aarch64_unknown_freebsd), ("armv6-unknown-freebsd", armv6_unknown_freebsd), @@ -777,7 +777,8 @@ supported_targets! { ("aarch64-unknown-hermit", aarch64_unknown_hermit), ("x86_64-unknown-hermit", x86_64_unknown_hermit), - ("x86_64-unknown-hermit-kernel", x86_64_unknown_hermit_kernel), + + ("x86_64-unknown-none-hermitkernel", x86_64_unknown_none_hermitkernel), ("riscv32i-unknown-none-elf", riscv32i_unknown_none_elf), ("riscv32imc-unknown-none-elf", riscv32imc_unknown_none_elf), diff --git a/compiler/rustc_target/src/spec/x86_64_unknown_hermit_kernel.rs b/compiler/rustc_target/src/spec/x86_64_unknown_none_hermitkernel.rs similarity index 91% rename from compiler/rustc_target/src/spec/x86_64_unknown_hermit_kernel.rs rename to compiler/rustc_target/src/spec/x86_64_unknown_none_hermitkernel.rs index 9fcfe4e6c14b..a357def190b8 100644 --- a/compiler/rustc_target/src/spec/x86_64_unknown_hermit_kernel.rs +++ b/compiler/rustc_target/src/spec/x86_64_unknown_none_hermitkernel.rs @@ -10,7 +10,7 @@ pub fn target() -> Target { base.stack_probes = StackProbeType::InlineOrCall { min_llvm_version_for_inline: (11, 0, 1) }; Target { - llvm_target: "x86_64-unknown-hermit".to_string(), + llvm_target: "x86_64-unknown-none-elf".to_string(), pointer_width: 64, data_layout: "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" .to_string(), diff --git a/compiler/rustc_target/src/spec/x86_64_linux_kernel.rs b/compiler/rustc_target/src/spec/x86_64_unknown_none_linuxkernel.rs similarity index 76% rename from compiler/rustc_target/src/spec/x86_64_linux_kernel.rs rename to compiler/rustc_target/src/spec/x86_64_unknown_none_linuxkernel.rs index 43e683ddbcc8..68d80205e147 100644 --- a/compiler/rustc_target/src/spec/x86_64_linux_kernel.rs +++ b/compiler/rustc_target/src/spec/x86_64_unknown_none_linuxkernel.rs @@ -14,8 +14,11 @@ pub fn target() -> Target { base.pre_link_args.get_mut(&LinkerFlavor::Gcc).unwrap().push("-m64".to_string()); Target { - // FIXME: Some dispute, the linux-on-clang folks think this should use "Linux" - llvm_target: "x86_64-elf".to_string(), + // FIXME: Some dispute, the linux-on-clang folks think this should use + // "Linux". We disagree because running *on* Linux is nothing like + // running *as" linux, and historically the "os" component as has always + // been used to mean the "on" part. + llvm_target: "x86_64-unknown-none-elf".to_string(), pointer_width: 64, data_layout: "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" .to_string(), diff --git a/library/unwind/src/lib.rs b/library/unwind/src/lib.rs index d145c6767ec2..be5e56c71e36 100644 --- a/library/unwind/src/lib.rs +++ b/library/unwind/src/lib.rs @@ -15,7 +15,7 @@ cfg_if::cfg_if! { target_os = "none", ))] { // These "unix" family members do not have unwinder. - // Note this also matches x86_64-linux-kernel. + // Note this also matches x86_64-unknown-none-linuxkernel. } else if #[cfg(any( unix, windows, diff --git a/src/doc/rustc/src/platform-support.md b/src/doc/rustc/src/platform-support.md index d3b88c019f01..611117788da8 100644 --- a/src/doc/rustc/src/platform-support.md +++ b/src/doc/rustc/src/platform-support.md @@ -217,13 +217,13 @@ target | std | host | notes `thumbv4t-none-eabi` | * | | ARMv4T T32 `x86_64-apple-ios-macabi` | ✓ | | Apple Catalyst on x86_64 `x86_64-apple-tvos` | * | | x86 64-bit tvOS -`x86_64-linux-kernel` | * | | Linux kernel modules +`x86_64-unknown-none-linuxkernel` | * | | Linux kernel modules `x86_64-sun-solaris` | ? | | Deprecated target for 64-bit Solaris 10/11, illumos `x86_64-pc-windows-msvc` | ✓ | | 64-bit Windows XP support `x86_64-unknown-dragonfly` | ✓ | ✓ | 64-bit DragonFlyBSD `x86_64-unknown-haiku` | ✓ | ✓ | 64-bit Haiku `x86_64-unknown-hermit` | ? | | -`x86_64-unknown-hermit-kernel` | ? | | HermitCore kernel +`x86_64-unknown-none-hermitkernel` | ? | | HermitCore kernel `x86_64-unknown-l4re-uclibc` | ? | | `x86_64-unknown-openbsd` | ✓ | ✓ | 64-bit OpenBSD `x86_64-unknown-uefi` | ? | | From 44c2794976986677c4f5524f63562d02e0e53d57 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 28 Feb 2021 22:41:05 -0500 Subject: [PATCH 03/25] Clean up error reporting for deprecated passes - Use spans for deprecated attributes - Use a proper diagnostic for unknown passes, instead of error logging - Add tests for unknown passes - Improve some wording in diagnostics --- src/librustdoc/config.rs | 5 +- src/librustdoc/core.rs | 57 ++++++++++++--------- src/test/rustdoc-ui/deprecated-attrs.rs | 16 ++++-- src/test/rustdoc-ui/deprecated-attrs.stderr | 32 ++++++++++-- 4 files changed, 74 insertions(+), 36 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index d9f5b5bfa3ae..6a62cacd34d1 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -654,9 +654,8 @@ fn check_deprecated_options(matches: &getopts::Matches, diag: &rustc_errors::Han { continue; } - let mut err = - diag.struct_warn(&format!("the '{}' flag is considered deprecated", flag)); - err.warn( + let mut err = diag.struct_warn(&format!("the `{}` flag is deprecated", flag)); + err.note( "see issue #44136 \ for more information", ); diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 6218aaf8276d..a935bd6b456d 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -22,7 +22,7 @@ use rustc_session::DiagnosticOutput; use rustc_session::Session; use rustc_span::source_map; use rustc_span::symbol::sym; -use rustc_span::DUMMY_SP; +use rustc_span::{Span, DUMMY_SP}; use std::mem; use std::rc::Rc; @@ -461,7 +461,7 @@ crate fn run_global_ctxt( tcx: TyCtxt<'_>, resolver: Rc>, mut default_passes: passes::DefaultPassOption, - mut manual_passes: Vec, + manual_passes: Vec, render_options: RenderOptions, output_format: OutputFormat, ) -> (clean::Crate, RenderOptions, Cache) { @@ -562,10 +562,10 @@ crate fn run_global_ctxt( } } - fn report_deprecated_attr(name: &str, diag: &rustc_errors::Handler) { - let mut msg = diag - .struct_warn(&format!("the `#![doc({})]` attribute is considered deprecated", name)); - msg.warn( + fn report_deprecated_attr(name: &str, diag: &rustc_errors::Handler, sp: Span) { + let mut msg = + diag.struct_span_warn(sp, &format!("the `#![doc({})]` attribute is deprecated", name)); + msg.note( "see issue #44136 \ for more information", ); @@ -577,6 +577,27 @@ crate fn run_global_ctxt( msg.emit(); } + let parse_pass = |name: &str, sp: Option| { + if let Some(pass) = passes::find_pass(name) { + Some(ConditionalPass::always(pass)) + } else { + let msg = &format!("ignoring unknown pass `{}`", name); + let mut warning = if let Some(sp) = sp { + tcx.sess.struct_span_warn(sp, msg) + } else { + tcx.sess.struct_warn(msg) + }; + if name == "collapse-docs" { + warning.note("the `collapse-docs` pass was removed in #80261 "); + } + warning.emit(); + None + } + }; + + let mut manual_passes: Vec<_> = + manual_passes.into_iter().flat_map(|name| parse_pass(&name, None)).collect(); + // Process all of the crate attributes, extracting plugin metadata along // with the passes which we are supposed to run. for attr in krate.module.as_ref().unwrap().attrs.lists(sym::doc) { @@ -585,19 +606,18 @@ crate fn run_global_ctxt( let name = attr.name_or_empty(); if attr.is_word() { if name == sym::no_default_passes { - report_deprecated_attr("no_default_passes", diag); + report_deprecated_attr("no_default_passes", diag, attr.span()); if default_passes == passes::DefaultPassOption::Default { default_passes = passes::DefaultPassOption::None; } } } else if let Some(value) = attr.value_str() { - let sink = match name { + match name { sym::passes => { - report_deprecated_attr("passes = \"...\"", diag); - &mut manual_passes + report_deprecated_attr("passes = \"...\"", diag, attr.span()); } sym::plugins => { - report_deprecated_attr("plugins = \"...\"", diag); + report_deprecated_attr("plugins = \"...\"", diag, attr.span()); eprintln!( "WARNING: `#![doc(plugins = \"...\")]` \ no longer functions; see CVE-2018-1000622" @@ -607,7 +627,8 @@ crate fn run_global_ctxt( _ => continue, }; for name in value.as_str().split_whitespace() { - sink.push(name.to_string()); + let span = attr.name_value_literal_span().unwrap_or(attr.span()); + manual_passes.extend(parse_pass(name, Some(span))); } } @@ -616,17 +637,7 @@ crate fn run_global_ctxt( } } - let passes = passes::defaults(default_passes).iter().copied().chain( - manual_passes.into_iter().flat_map(|name| { - if let Some(pass) = passes::find_pass(&name) { - Some(ConditionalPass::always(pass)) - } else { - error!("unknown pass {}, skipping", name); - None - } - }), - ); - + let passes = passes::defaults(default_passes).iter().copied().chain(manual_passes); info!("Executing passes"); for p in passes { diff --git a/src/test/rustdoc-ui/deprecated-attrs.rs b/src/test/rustdoc-ui/deprecated-attrs.rs index ca626afbe535..a0439acc0240 100644 --- a/src/test/rustdoc-ui/deprecated-attrs.rs +++ b/src/test/rustdoc-ui/deprecated-attrs.rs @@ -1,7 +1,13 @@ // check-pass +// compile-flags: --passes unknown-pass +// error-pattern: ignoring unknown pass `unknown-pass` -#![doc(no_default_passes, passes = "unindent-comments")] - -struct SomeStruct; - -pub struct OtherStruct; +#![doc(no_default_passes)] +//~^ WARNING attribute is deprecated +//~| NOTE see issue #44136 +//~| HELP use `#![doc(document_private_items)]` +#![doc(passes = "collapse-docs unindent-comments")] +//~^ WARNING attribute is deprecated +//~| NOTE see issue #44136 +//~| WARNING ignoring unknown pass +//~| NOTE `collapse-docs` pass was removed diff --git a/src/test/rustdoc-ui/deprecated-attrs.stderr b/src/test/rustdoc-ui/deprecated-attrs.stderr index f68fb4674480..42a229abf58e 100644 --- a/src/test/rustdoc-ui/deprecated-attrs.stderr +++ b/src/test/rustdoc-ui/deprecated-attrs.stderr @@ -1,11 +1,33 @@ -warning: the `#![doc(no_default_passes)]` attribute is considered deprecated +warning: the `passes` flag is deprecated | - = warning: see issue #44136 for more information + = note: see issue #44136 for more information + +warning: ignoring unknown pass `unknown-pass` + +warning: the `#![doc(no_default_passes)]` attribute is deprecated + --> $DIR/deprecated-attrs.rs:5:8 + | +LL | #![doc(no_default_passes)] + | ^^^^^^^^^^^^^^^^^ + | + = note: see issue #44136 for more information = help: you may want to use `#![doc(document_private_items)]` -warning: the `#![doc(passes = "...")]` attribute is considered deprecated +warning: the `#![doc(passes = "...")]` attribute is deprecated + --> $DIR/deprecated-attrs.rs:9:8 + | +LL | #![doc(passes = "collapse-docs unindent-comments")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #44136 for more information + +warning: ignoring unknown pass `collapse-docs` + --> $DIR/deprecated-attrs.rs:9:17 + | +LL | #![doc(passes = "collapse-docs unindent-comments")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = warning: see issue #44136 for more information + = note: the `collapse-docs` pass was removed in #80261 -warning: 2 warnings emitted +warning: 4 warnings emitted From d5c300b1f2e9a6f7f05b2fa51ca2b4d011d289f4 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 28 Feb 2021 22:51:35 -0500 Subject: [PATCH 04/25] Report that `doc(plugins)` doesn't work using diagnostics instead of `println!` This also adds a test for the output and fixes `rustc_attr` to properly know that `plugins` is a valid attribute. --- compiler/rustc_passes/src/check_attr.rs | 3 ++- src/librustdoc/core.rs | 6 ++---- src/test/rustdoc-ui/deprecated-attrs.rs | 4 ++++ src/test/rustdoc-ui/deprecated-attrs.stderr | 11 ++++++++++- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index afd1642bbd57..d0ab0d657d9e 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -559,7 +559,8 @@ impl CheckAttrVisitor<'tcx> { sym::masked, sym::no_default_passes, // deprecated sym::no_inline, - sym::passes, // deprecated + sym::passes, // deprecated + sym::plugins, // removed, but rustdoc warns about it itself sym::primitive, sym::spotlight, sym::test, diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index a935bd6b456d..19821c3ccefd 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -572,6 +572,8 @@ crate fn run_global_ctxt( if name == "no_default_passes" { msg.help("you may want to use `#![doc(document_private_items)]`"); + } else if name.starts_with("plugins") { + msg.warn("`#![doc(plugins = \"...\")]` no longer functions; see CVE-2018-1000622 "); } msg.emit(); @@ -618,10 +620,6 @@ crate fn run_global_ctxt( } sym::plugins => { report_deprecated_attr("plugins = \"...\"", diag, attr.span()); - eprintln!( - "WARNING: `#![doc(plugins = \"...\")]` \ - no longer functions; see CVE-2018-1000622" - ); continue; } _ => continue, diff --git a/src/test/rustdoc-ui/deprecated-attrs.rs b/src/test/rustdoc-ui/deprecated-attrs.rs index a0439acc0240..5febc5eb9cd5 100644 --- a/src/test/rustdoc-ui/deprecated-attrs.rs +++ b/src/test/rustdoc-ui/deprecated-attrs.rs @@ -11,3 +11,7 @@ //~| NOTE see issue #44136 //~| WARNING ignoring unknown pass //~| NOTE `collapse-docs` pass was removed +#![doc(plugins = "xxx")] +//~^ WARNING attribute is deprecated +//~| NOTE see issue #44136 +//~| WARNING no longer functions; see CVE diff --git a/src/test/rustdoc-ui/deprecated-attrs.stderr b/src/test/rustdoc-ui/deprecated-attrs.stderr index 42a229abf58e..b855cedf5223 100644 --- a/src/test/rustdoc-ui/deprecated-attrs.stderr +++ b/src/test/rustdoc-ui/deprecated-attrs.stderr @@ -29,5 +29,14 @@ LL | #![doc(passes = "collapse-docs unindent-comments")] | = note: the `collapse-docs` pass was removed in #80261 -warning: 4 warnings emitted +warning: the `#![doc(plugins = "...")]` attribute is deprecated + --> $DIR/deprecated-attrs.rs:14:8 + | +LL | #![doc(plugins = "xxx")] + | ^^^^^^^^^^^^^^^ + | + = note: see issue #44136 for more information + = warning: `#![doc(plugins = "...")]` no longer functions; see CVE-2018-1000622 + +warning: 5 warnings emitted From d9d69212c3d1dca7e8975d5163f09289db4a7420 Mon Sep 17 00:00:00 2001 From: Henry Boisdequin <65845077+henryboisdequin@users.noreply.github.com> Date: Wed, 3 Mar 2021 16:50:07 +0530 Subject: [PATCH 05/25] Fix diagnostic suggests adding type `[type error]` Fixes #79040 Unresolved questions: Why does this change output the diagnostic twice? Why does the CI fail when the errors are pointed out (UI tests)? --- compiler/rustc_typeck/src/collect/type_of.rs | 2 +- src/test/ui/79040.rs | 5 +++++ src/test/ui/79040.stderr | 19 +++++++++++++++++++ ...em-free-const-no-body-semantic-fail.stderr | 8 +------- ...m-free-static-no-body-semantic-fail.stderr | 14 +------------- 5 files changed, 27 insertions(+), 21 deletions(-) create mode 100644 src/test/ui/79040.rs create mode 100644 src/test/ui/79040.stderr diff --git a/compiler/rustc_typeck/src/collect/type_of.rs b/compiler/rustc_typeck/src/collect/type_of.rs index e4eabca9c3b7..92e2b5232027 100644 --- a/compiler/rustc_typeck/src/collect/type_of.rs +++ b/compiler/rustc_typeck/src/collect/type_of.rs @@ -659,7 +659,7 @@ fn infer_placeholder_type( format!("{}: {}", item_ident, ty), Applicability::MachineApplicable, ) - .emit(); + .emit_unless(matches!(ty.kind(), ty::Error(_))); } None => { let mut diag = bad_placeholder_type(tcx, vec![span]); diff --git a/src/test/ui/79040.rs b/src/test/ui/79040.rs new file mode 100644 index 000000000000..adc9dc8b4265 --- /dev/null +++ b/src/test/ui/79040.rs @@ -0,0 +1,5 @@ +fn main() { + const FOO = "hello" + 1; + //^~ ERROR cannot add `{integer}` to `&str` + println!("{}", FOO); +} \ No newline at end of file diff --git a/src/test/ui/79040.stderr b/src/test/ui/79040.stderr new file mode 100644 index 000000000000..be7d2363c2d5 --- /dev/null +++ b/src/test/ui/79040.stderr @@ -0,0 +1,19 @@ +error[E0369]: cannot add `{integer}` to `&str` + --> $DIR/79040.rs:2:25 + | +LL | const FOO = "hello" + 1; + | ------- ^ - {integer} + | | + | &str + +error[E0369]: cannot add `{integer}` to `&str` + --> $DIR/79040.rs:2:25 + | +LL | const FOO = "hello" + 1; + | ------- ^ - {integer} + | | + | &str + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0369`. diff --git a/src/test/ui/parser/item-free-const-no-body-semantic-fail.stderr b/src/test/ui/parser/item-free-const-no-body-semantic-fail.stderr index 4e97229fa1a4..aa75e5cee01d 100644 --- a/src/test/ui/parser/item-free-const-no-body-semantic-fail.stderr +++ b/src/test/ui/parser/item-free-const-no-body-semantic-fail.stderr @@ -14,11 +14,5 @@ LL | const B; | | | help: provide a definition for the constant: `= ;` -error: missing type for `const` item - --> $DIR/item-free-const-no-body-semantic-fail.rs:6:7 - | -LL | const B; - | ^ help: provide a type for the item: `B: [type error]` - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors diff --git a/src/test/ui/parser/item-free-static-no-body-semantic-fail.stderr b/src/test/ui/parser/item-free-static-no-body-semantic-fail.stderr index 60b7bb34c698..2a270c8290fd 100644 --- a/src/test/ui/parser/item-free-static-no-body-semantic-fail.stderr +++ b/src/test/ui/parser/item-free-static-no-body-semantic-fail.stderr @@ -30,17 +30,5 @@ LL | static mut D; | | | help: provide a definition for the static: `= ;` -error: missing type for `static` item - --> $DIR/item-free-static-no-body-semantic-fail.rs:6:8 - | -LL | static B; - | ^ help: provide a type for the item: `B: [type error]` - -error: missing type for `static mut` item - --> $DIR/item-free-static-no-body-semantic-fail.rs:10:12 - | -LL | static mut D; - | ^ help: provide a type for the item: `D: [type error]` - -error: aborting due to 6 previous errors +error: aborting due to 4 previous errors From 5461ee0b2632ccd912d316a68dd63a5b45125f57 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Thu, 27 Aug 2020 11:49:18 -0400 Subject: [PATCH 06/25] rustc_target: add "unwind" payloads to `Abi` ### Overview This commit begins the implementation work for RFC 2945. For more information, see the rendered RFC [1] and tracking issue [2]. A boolean `unwind` payload is added to the `C`, `System`, `Stdcall`, and `Thiscall` variants, marking whether unwinding across FFI boundaries is acceptable. The cases where each of these variants' `unwind` member is true correspond with the `C-unwind`, `system-unwind`, `stdcall-unwind`, and `thiscall-unwind` ABI strings introduced in RFC 2945 [3]. ### Feature Gate and Unstable Book This commit adds a `c_unwind` feature gate for the new ABI strings. Tests for this feature gate are included in `src/test/ui/c-unwind/`, which ensure that this feature gate works correctly for each of the new ABIs. A new language features entry in the unstable book is added as well. ### Further Work To Be Done This commit does not proceed to implement the new unwinding ABIs, and is intentionally scoped specifically to *defining* the ABIs and their feature flag. ### One Note on Test Churn This will lead to some test churn, in re-blessing hash tests, as the deleted comment in `src/librustc_target/spec/abi.rs` mentioned, because we can no longer guarantee the ordering of the `Abi` variants. While this is a downside, this decision was made bearing in mind that RFC 2945 states the following, in the "Other `unwind` Strings" section [3]: > More unwind variants of existing ABI strings may be introduced, > with the same semantics, without an additional RFC. Adding a new variant for each of these cases, rather than specifying a payload for a given ABI, would quickly become untenable, and make working with the `Abi` enum prone to mistakes. This approach encodes the unwinding information *into* a given ABI, to account for the future possibility of other `-unwind` ABI strings. ### Ignore Directives `ignore-*` directives are used in two of our `*-unwind` ABI test cases. Specifically, the `stdcall-unwind` and `thiscall-unwind` test cases ignore architectures that do not support `stdcall` and `thiscall`, respectively. These directives are cribbed from `src/test/ui/c-variadic/variadic-ffi-1.rs` for `stdcall`, and `src/test/ui/extern/extern-thiscall.rs` for `thiscall`. This would otherwise fail on some targets, see: https://github.com/rust-lang-ci/rust/commit/fcf697f90206e9c87b39d494f94ab35d976bfc60 ### Footnotes [1]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md [2]: https://github.com/rust-lang/rust/issues/74990 [3]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md#other-unwind-abi-strings --- compiler/rustc_ast_lowering/src/item.rs | 8 +-- compiler/rustc_ast_passes/src/feature_gate.rs | 32 +++++++++ .../rustc_codegen_cranelift/src/abi/mod.rs | 2 +- compiler/rustc_feature/src/active.rs | 3 + compiler/rustc_middle/src/ty/layout.rs | 8 +-- .../rustc_mir/src/interpret/terminator.rs | 6 +- compiler/rustc_span/src/symbol.rs | 1 + compiler/rustc_symbol_mangling/src/v0.rs | 2 +- compiler/rustc_target/src/spec/abi.rs | 65 +++++++++++++++---- compiler/rustc_target/src/spec/arm_base.rs | 11 +++- .../src/spec/mipsel_unknown_none.rs | 6 +- compiler/rustc_target/src/spec/mod.rs | 19 ++++-- .../src/spec/nvptx64_nvidia_cuda.rs | 6 +- compiler/rustc_target/src/spec/riscv_base.rs | 6 +- compiler/rustc_typeck/src/collect.rs | 2 +- compiler/rustc_typeck/src/lib.rs | 21 +++--- .../src/language-features/c-unwind.md | 14 ++++ src/test/ui/codemap_tests/unicode.stderr | 2 +- src/test/ui/parser/issue-8537.stderr | 2 +- src/test/ui/symbol-names/impl1.rs | 6 +- .../feature-gate-c-unwind-enabled.rs | 12 ++++ .../ui/unwind-abis/feature-gate-c-unwind.rs | 9 +++ .../unwind-abis/feature-gate-c-unwind.stderr | 12 ++++ .../feature-gate-stdcall-unwind.rs | 13 ++++ .../feature-gate-stdcall-unwind.stderr | 12 ++++ .../unwind-abis/feature-gate-system-unwind.rs | 9 +++ .../feature-gate-system-unwind.stderr | 12 ++++ .../feature-gate-thiscall-unwind.rs | 13 ++++ .../feature-gate-thiscall-unwind.stderr | 12 ++++ 29 files changed, 274 insertions(+), 52 deletions(-) create mode 100644 src/doc/unstable-book/src/language-features/c-unwind.md create mode 100644 src/test/ui/unwind-abis/feature-gate-c-unwind-enabled.rs create mode 100644 src/test/ui/unwind-abis/feature-gate-c-unwind.rs create mode 100644 src/test/ui/unwind-abis/feature-gate-c-unwind.stderr create mode 100644 src/test/ui/unwind-abis/feature-gate-stdcall-unwind.rs create mode 100644 src/test/ui/unwind-abis/feature-gate-stdcall-unwind.stderr create mode 100644 src/test/ui/unwind-abis/feature-gate-system-unwind.rs create mode 100644 src/test/ui/unwind-abis/feature-gate-system-unwind.stderr create mode 100644 src/test/ui/unwind-abis/feature-gate-thiscall-unwind.rs create mode 100644 src/test/ui/unwind-abis/feature-gate-thiscall-unwind.stderr diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 8b740b777408..2267f09ec1f1 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -322,10 +322,10 @@ impl<'hir> LoweringContext<'_, 'hir> { }, ItemKind::ForeignMod(ref fm) => { if fm.abi.is_none() { - self.maybe_lint_missing_abi(span, id, abi::Abi::C); + self.maybe_lint_missing_abi(span, id, abi::Abi::C { unwind: false }); } hir::ItemKind::ForeignMod { - abi: fm.abi.map_or(abi::Abi::C, |abi| self.lower_abi(abi)), + abi: fm.abi.map_or(abi::Abi::C { unwind: false }, |abi| self.lower_abi(abi)), items: self .arena .alloc_from_iter(fm.items.iter().map(|x| self.lower_foreign_item_ref(x))), @@ -1322,8 +1322,8 @@ impl<'hir> LoweringContext<'_, 'hir> { match ext { Extern::None => abi::Abi::Rust, Extern::Implicit => { - self.maybe_lint_missing_abi(span, id, abi::Abi::C); - abi::Abi::C + self.maybe_lint_missing_abi(span, id, abi::Abi::C { unwind: false }); + abi::Abi::C { unwind: false } } Extern::Explicit(abi) => self.lower_abi(abi), } diff --git a/compiler/rustc_ast_passes/src/feature_gate.rs b/compiler/rustc_ast_passes/src/feature_gate.rs index 474ec2b589b7..acfbec6fa21b 100644 --- a/compiler/rustc_ast_passes/src/feature_gate.rs +++ b/compiler/rustc_ast_passes/src/feature_gate.rs @@ -164,6 +164,38 @@ impl<'a> PostExpansionVisitor<'a> { "C-cmse-nonsecure-call ABI is experimental and subject to change" ); } + "C-unwind" => { + gate_feature_post!( + &self, + c_unwind, + span, + "C-unwind ABI is experimental and subject to change" + ); + } + "stdcall-unwind" => { + gate_feature_post!( + &self, + c_unwind, + span, + "stdcall-unwind ABI is experimental and subject to change" + ); + } + "system-unwind" => { + gate_feature_post!( + &self, + c_unwind, + span, + "system-unwind ABI is experimental and subject to change" + ); + } + "thiscall-unwind" => { + gate_feature_post!( + &self, + c_unwind, + span, + "thiscall-unwind ABI is experimental and subject to change" + ); + } abi => self .sess .parse_sess diff --git a/compiler/rustc_codegen_cranelift/src/abi/mod.rs b/compiler/rustc_codegen_cranelift/src/abi/mod.rs index b2647e6c8d38..83bb1d0172fb 100644 --- a/compiler/rustc_codegen_cranelift/src/abi/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/abi/mod.rs @@ -542,7 +542,7 @@ pub(crate) fn codegen_terminator_call<'tcx>( // FIXME find a cleaner way to support varargs if fn_sig.c_variadic { - if fn_sig.abi != Abi::C { + if !matches!(fn_sig.abi, Abi::C { .. }) { fx.tcx.sess.span_fatal( span, &format!("Variadic call for non-C abi {:?}", fn_sig.abi), diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 8ee995a59d80..95ae6a1ac9a2 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -644,6 +644,9 @@ declare_features! ( /// Allows associated types in inherent impls. (active, inherent_associated_types, "1.52.0", Some(8995), None), + /// Allows `extern "C-unwind" fn` to enable unwinding across ABI boundaries. + (active, c_unwind, "1.52.0", Some(74990), None), + // ------------------------------------------------------------------------- // feature-group-end: actual feature gates // ------------------------------------------------------------------------- diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 6d2ab0e5f5a8..2f5380861c49 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2654,14 +2654,14 @@ where RustIntrinsic | PlatformIntrinsic | Rust | RustCall => Conv::Rust, // It's the ABI's job to select this, not ours. - System => bug!("system abi should be selected elsewhere"), + System { .. } => bug!("system abi should be selected elsewhere"), EfiApi => bug!("eficall abi should be selected elsewhere"), - Stdcall => Conv::X86Stdcall, + Stdcall { .. } => Conv::X86Stdcall, Fastcall => Conv::X86Fastcall, Vectorcall => Conv::X86VectorCall, - Thiscall => Conv::X86ThisCall, - C => Conv::C, + Thiscall { .. } => Conv::X86ThisCall, + C { .. } => Conv::C, Unadjusted => Conv::C, Win64 => Conv::X86_64Win64, SysV64 => Conv::X86_64SysV, diff --git a/compiler/rustc_mir/src/interpret/terminator.rs b/compiler/rustc_mir/src/interpret/terminator.rs index 0807949a2d91..4aa1360d5353 100644 --- a/compiler/rustc_mir/src/interpret/terminator.rs +++ b/compiler/rustc_mir/src/interpret/terminator.rs @@ -248,9 +248,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; if normalize_abi(caller_abi) != normalize_abi(callee_abi) { throw_ub_format!( - "calling a function with ABI {:?} using caller ABI {:?}", - callee_abi, - caller_abi + "calling a function with ABI {} using caller ABI {}", + callee_abi.name(), + caller_abi.name() ) } } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 27bb45bcc851..6eb5a40fba92 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -328,6 +328,7 @@ symbols! { bridge, bswap, c_str, + c_unwind, c_variadic, call, call_mut, diff --git a/compiler/rustc_symbol_mangling/src/v0.rs b/compiler/rustc_symbol_mangling/src/v0.rs index bbf7ecc39cfd..12c0a147990f 100644 --- a/compiler/rustc_symbol_mangling/src/v0.rs +++ b/compiler/rustc_symbol_mangling/src/v0.rs @@ -440,7 +440,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> { } match sig.abi { Abi::Rust => {} - Abi::C => cx.push("KC"), + Abi::C { unwind: false } => cx.push("KC"), abi => { cx.push("K"); let name = abi.name(); diff --git a/compiler/rustc_target/src/spec/abi.rs b/compiler/rustc_target/src/spec/abi.rs index 65e8a4e8db2a..f7f9c30d3b7a 100644 --- a/compiler/rustc_target/src/spec/abi.rs +++ b/compiler/rustc_target/src/spec/abi.rs @@ -8,24 +8,21 @@ mod tests; #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, Debug)] #[derive(HashStable_Generic, Encodable, Decodable)] pub enum Abi { - // N.B., this ordering MUST match the AbiDatas array below. - // (This is ensured by the test indices_are_correct().) - // Multiplatform / generic ABIs // // These ABIs come first because every time we add a new ABI, we // have to re-bless all the hashing tests. These are used in many // places, so giving them stable values reduces test churn. The // specific values are meaningless. - Rust = 0, - C = 1, + Rust, + C { unwind: bool }, // Single platform ABIs Cdecl, - Stdcall, + Stdcall { unwind: bool }, Fastcall, Vectorcall, - Thiscall, + Thiscall { unwind: bool }, Aapcs, Win64, SysV64, @@ -39,7 +36,7 @@ pub enum Abi { CCmseNonSecureCall, // Multiplatform / generic ABIs - System, + System { unwind: bool }, RustIntrinsic, RustCall, PlatformIntrinsic, @@ -61,13 +58,16 @@ pub struct AbiData { const AbiDatas: &[AbiData] = &[ // Cross-platform ABIs AbiData { abi: Abi::Rust, name: "Rust", generic: true }, - AbiData { abi: Abi::C, name: "C", generic: true }, + AbiData { abi: Abi::C { unwind: false }, name: "C", generic: true }, + AbiData { abi: Abi::C { unwind: true }, name: "C-unwind", generic: true }, // Platform-specific ABIs AbiData { abi: Abi::Cdecl, name: "cdecl", generic: false }, - AbiData { abi: Abi::Stdcall, name: "stdcall", generic: false }, + AbiData { abi: Abi::Stdcall { unwind: false }, name: "stdcall", generic: false }, + AbiData { abi: Abi::Stdcall { unwind: true }, name: "stdcall-unwind", generic: false }, AbiData { abi: Abi::Fastcall, name: "fastcall", generic: false }, AbiData { abi: Abi::Vectorcall, name: "vectorcall", generic: false }, - AbiData { abi: Abi::Thiscall, name: "thiscall", generic: false }, + AbiData { abi: Abi::Thiscall { unwind: false }, name: "thiscall", generic: false }, + AbiData { abi: Abi::Thiscall { unwind: true }, name: "thiscall-unwind", generic: false }, AbiData { abi: Abi::Aapcs, name: "aapcs", generic: false }, AbiData { abi: Abi::Win64, name: "win64", generic: false }, AbiData { abi: Abi::SysV64, name: "sysv64", generic: false }, @@ -84,7 +84,8 @@ const AbiDatas: &[AbiData] = &[ }, AbiData { abi: Abi::CCmseNonSecureCall, name: "C-cmse-nonsecure-call", generic: false }, // Cross-platform ABIs - AbiData { abi: Abi::System, name: "system", generic: true }, + AbiData { abi: Abi::System { unwind: false }, name: "system", generic: true }, + AbiData { abi: Abi::System { unwind: true }, name: "system-unwind", generic: true }, AbiData { abi: Abi::RustIntrinsic, name: "rust-intrinsic", generic: true }, AbiData { abi: Abi::RustCall, name: "rust-call", generic: true }, AbiData { abi: Abi::PlatformIntrinsic, name: "platform-intrinsic", generic: true }, @@ -103,7 +104,41 @@ pub fn all_names() -> Vec<&'static str> { impl Abi { #[inline] pub fn index(self) -> usize { - self as usize + // N.B., this ordering MUST match the AbiDatas array above. + // (This is ensured by the test indices_are_correct().) + use Abi::*; + match self { + // Cross-platform ABIs + Rust => 0, + C { unwind: false } => 1, + C { unwind: true } => 2, + // Platform-specific ABIs + Cdecl => 3, + Stdcall { unwind: false } => 4, + Stdcall { unwind: true } => 5, + Fastcall => 6, + Vectorcall => 7, + Thiscall { unwind: false } => 8, + Thiscall { unwind: true } => 9, + Aapcs => 10, + Win64 => 11, + SysV64 => 12, + PtxKernel => 13, + Msp430Interrupt => 14, + X86Interrupt => 15, + AmdGpuKernel => 16, + EfiApi => 17, + AvrInterrupt => 18, + AvrNonBlockingInterrupt => 19, + CCmseNonSecureCall => 20, + // Cross-platform ABIs + System { unwind: false } => 21, + System { unwind: true } => 22, + RustIntrinsic => 23, + RustCall => 24, + PlatformIntrinsic => 25, + Unadjusted => 26, + } } #[inline] @@ -122,6 +157,8 @@ impl Abi { impl fmt::Display for Abi { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "\"{}\"", self.name()) + match self { + abi => write!(f, "\"{}\"", abi.name()), + } } } diff --git a/compiler/rustc_target/src/spec/arm_base.rs b/compiler/rustc_target/src/spec/arm_base.rs index b74d80dc6bb2..01f573313c97 100644 --- a/compiler/rustc_target/src/spec/arm_base.rs +++ b/compiler/rustc_target/src/spec/arm_base.rs @@ -2,5 +2,14 @@ use crate::spec::abi::Abi; // All the calling conventions trigger an assertion(Unsupported calling convention) in llvm on arm pub fn unsupported_abis() -> Vec { - vec![Abi::Stdcall, Abi::Fastcall, Abi::Vectorcall, Abi::Thiscall, Abi::Win64, Abi::SysV64] + vec![ + Abi::Stdcall { unwind: false }, + Abi::Stdcall { unwind: true }, + Abi::Fastcall, + Abi::Vectorcall, + Abi::Thiscall { unwind: false }, + Abi::Thiscall { unwind: true }, + Abi::Win64, + Abi::SysV64, + ] } diff --git a/compiler/rustc_target/src/spec/mipsel_unknown_none.rs b/compiler/rustc_target/src/spec/mipsel_unknown_none.rs index 0f9d3c3de154..110c8dd80ea7 100644 --- a/compiler/rustc_target/src/spec/mipsel_unknown_none.rs +++ b/compiler/rustc_target/src/spec/mipsel_unknown_none.rs @@ -23,10 +23,12 @@ pub fn target() -> Target { panic_strategy: PanicStrategy::Abort, relocation_model: RelocModel::Static, unsupported_abis: vec![ - Abi::Stdcall, + Abi::Stdcall { unwind: false }, + Abi::Stdcall { unwind: true }, Abi::Fastcall, Abi::Vectorcall, - Abi::Thiscall, + Abi::Thiscall { unwind: false }, + Abi::Thiscall { unwind: true }, Abi::Win64, Abi::SysV64, ], diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 039e9a8b2745..4e8c0c03696d 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -1282,24 +1282,31 @@ impl Target { /// Given a function ABI, turn it into the correct ABI for this target. pub fn adjust_abi(&self, abi: Abi) -> Abi { match abi { - Abi::System => { + Abi::System { unwind } => { if self.is_like_windows && self.arch == "x86" { - Abi::Stdcall + Abi::Stdcall { unwind } } else { - Abi::C + Abi::C { unwind } } } // These ABI kinds are ignored on non-x86 Windows targets. // See https://docs.microsoft.com/en-us/cpp/cpp/argument-passing-and-naming-conventions // and the individual pages for __stdcall et al. - Abi::Stdcall | Abi::Fastcall | Abi::Vectorcall | Abi::Thiscall => { - if self.is_like_windows && self.arch != "x86" { Abi::C } else { abi } + Abi::Stdcall { unwind } | Abi::Thiscall { unwind } => { + if self.is_like_windows && self.arch != "x86" { Abi::C { unwind } } else { abi } + } + Abi::Fastcall | Abi::Vectorcall => { + if self.is_like_windows && self.arch != "x86" { + Abi::C { unwind: false } + } else { + abi + } } Abi::EfiApi => { if self.arch == "x86_64" { Abi::Win64 } else { - Abi::C + Abi::C { unwind: false } } } abi => abi, diff --git a/compiler/rustc_target/src/spec/nvptx64_nvidia_cuda.rs b/compiler/rustc_target/src/spec/nvptx64_nvidia_cuda.rs index 3c9c7d578fbd..15d8e4843f97 100644 --- a/compiler/rustc_target/src/spec/nvptx64_nvidia_cuda.rs +++ b/compiler/rustc_target/src/spec/nvptx64_nvidia_cuda.rs @@ -49,10 +49,12 @@ pub fn target() -> Target { // create the tests for this. unsupported_abis: vec![ Abi::Cdecl, - Abi::Stdcall, + Abi::Stdcall { unwind: false }, + Abi::Stdcall { unwind: true }, Abi::Fastcall, Abi::Vectorcall, - Abi::Thiscall, + Abi::Thiscall { unwind: false }, + Abi::Thiscall { unwind: true }, Abi::Aapcs, Abi::Win64, Abi::SysV64, diff --git a/compiler/rustc_target/src/spec/riscv_base.rs b/compiler/rustc_target/src/spec/riscv_base.rs index 64cf890037e5..5bcbb2e621bd 100644 --- a/compiler/rustc_target/src/spec/riscv_base.rs +++ b/compiler/rustc_target/src/spec/riscv_base.rs @@ -5,10 +5,12 @@ use crate::spec::abi::Abi; pub fn unsupported_abis() -> Vec { vec![ Abi::Cdecl, - Abi::Stdcall, + Abi::Stdcall { unwind: false }, + Abi::Stdcall { unwind: true }, Abi::Fastcall, Abi::Vectorcall, - Abi::Thiscall, + Abi::Thiscall { unwind: false }, + Abi::Thiscall { unwind: true }, Abi::Aapcs, Abi::Win64, Abi::SysV64, diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 3881d55ef916..a175da327063 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2666,7 +2666,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { } else if tcx.sess.check_name(attr, sym::used) { codegen_fn_attrs.flags |= CodegenFnAttrFlags::USED; } else if tcx.sess.check_name(attr, sym::cmse_nonsecure_entry) { - if tcx.fn_sig(id).abi() != abi::Abi::C { + if !matches!(tcx.fn_sig(id).abi(), abi::Abi::C { .. }) { struct_span_err!( tcx.sess, attr.span, diff --git a/compiler/rustc_typeck/src/lib.rs b/compiler/rustc_typeck/src/lib.rs index 6ddc26efeae3..fb864c5b4a7d 100644 --- a/compiler/rustc_typeck/src/lib.rs +++ b/compiler/rustc_typeck/src/lib.rs @@ -118,14 +118,19 @@ use astconv::AstConv; use bounds::Bounds; fn require_c_abi_if_c_variadic(tcx: TyCtxt<'_>, decl: &hir::FnDecl<'_>, abi: Abi, span: Span) { - if decl.c_variadic && !(abi == Abi::C || abi == Abi::Cdecl) { - let mut err = struct_span_err!( - tcx.sess, - span, - E0045, - "C-variadic function must have C or cdecl calling convention" - ); - err.span_label(span, "C-variadics require C or cdecl calling convention").emit(); + match (decl.c_variadic, abi) { + // The function has the correct calling convention, or isn't a "C-variadic" function. + (false, _) | (true, Abi::C { .. }) | (true, Abi::Cdecl) => {} + // The function is a "C-variadic" function with an incorrect calling convention. + (true, _) => { + let mut err = struct_span_err!( + tcx.sess, + span, + E0045, + "C-variadic function must have C or cdecl calling convention" + ); + err.span_label(span, "C-variadics require C or cdecl calling convention").emit(); + } } } diff --git a/src/doc/unstable-book/src/language-features/c-unwind.md b/src/doc/unstable-book/src/language-features/c-unwind.md new file mode 100644 index 000000000000..c1705d59acc2 --- /dev/null +++ b/src/doc/unstable-book/src/language-features/c-unwind.md @@ -0,0 +1,14 @@ +# `c_unwind` + +The tracking issue for this feature is: [#74990] + +[#74990]: https://github.com/rust-lang/rust/issues/74990 + +------------------------ + +Introduces a new ABI string, "C-unwind", to enable unwinding from other +languages (such as C++) into Rust frames and from Rust into other languages. + +See [RFC 2945] for more information. + +[RFC 2945]: https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md diff --git a/src/test/ui/codemap_tests/unicode.stderr b/src/test/ui/codemap_tests/unicode.stderr index 61c3f4f1c984..b7ba4fa46d92 100644 --- a/src/test/ui/codemap_tests/unicode.stderr +++ b/src/test/ui/codemap_tests/unicode.stderr @@ -4,7 +4,7 @@ error[E0703]: invalid ABI: found `路濫狼á́́` LL | extern "路濫狼á́́" fn foo() {} | ^^^^^^^^^ invalid ABI | - = help: valid ABIs: Rust, C, cdecl, stdcall, fastcall, vectorcall, thiscall, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, efiapi, avr-interrupt, avr-non-blocking-interrupt, C-cmse-nonsecure-call, system, rust-intrinsic, rust-call, platform-intrinsic, unadjusted + = help: valid ABIs: Rust, C, C-unwind, cdecl, stdcall, stdcall-unwind, fastcall, vectorcall, thiscall, thiscall-unwind, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, efiapi, avr-interrupt, avr-non-blocking-interrupt, C-cmse-nonsecure-call, system, system-unwind, rust-intrinsic, rust-call, platform-intrinsic, unadjusted error: aborting due to previous error diff --git a/src/test/ui/parser/issue-8537.stderr b/src/test/ui/parser/issue-8537.stderr index 3f63c0802109..85e2b77d867b 100644 --- a/src/test/ui/parser/issue-8537.stderr +++ b/src/test/ui/parser/issue-8537.stderr @@ -4,7 +4,7 @@ error[E0703]: invalid ABI: found `invalid-ab_isize` LL | "invalid-ab_isize" | ^^^^^^^^^^^^^^^^^^ invalid ABI | - = help: valid ABIs: Rust, C, cdecl, stdcall, fastcall, vectorcall, thiscall, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, efiapi, avr-interrupt, avr-non-blocking-interrupt, C-cmse-nonsecure-call, system, rust-intrinsic, rust-call, platform-intrinsic, unadjusted + = help: valid ABIs: Rust, C, C-unwind, cdecl, stdcall, stdcall-unwind, fastcall, vectorcall, thiscall, thiscall-unwind, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, efiapi, avr-interrupt, avr-non-blocking-interrupt, C-cmse-nonsecure-call, system, system-unwind, rust-intrinsic, rust-call, platform-intrinsic, unadjusted error: aborting due to previous error diff --git a/src/test/ui/symbol-names/impl1.rs b/src/test/ui/symbol-names/impl1.rs index 24bdf6d669e8..7dd74bd229d3 100644 --- a/src/test/ui/symbol-names/impl1.rs +++ b/src/test/ui/symbol-names/impl1.rs @@ -4,7 +4,7 @@ //[legacy]compile-flags: -Z symbol-mangling-version=legacy //[v0]compile-flags: -Z symbol-mangling-version=v0 //[legacy]normalize-stderr-32bit: "hee444285569b39c2" -> "SYMBOL_HASH" -//[legacy]normalize-stderr-64bit: "h310ea0259fc3d32d" -> "SYMBOL_HASH" +//[legacy]normalize-stderr-64bit: "hd949d7797008991f" -> "SYMBOL_HASH" #![feature(auto_traits, rustc_attrs)] #![allow(dead_code)] @@ -75,3 +75,7 @@ fn main() { } }; } + +// FIXME(katie): The 32-bit symbol hash probably needs updating as well, but I'm slightly unsure +// about how to do that. This comment is here so that we don't break the test due to error messages +// including incorrect line numbers. diff --git a/src/test/ui/unwind-abis/feature-gate-c-unwind-enabled.rs b/src/test/ui/unwind-abis/feature-gate-c-unwind-enabled.rs new file mode 100644 index 000000000000..6ff5dbda2d56 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-c-unwind-enabled.rs @@ -0,0 +1,12 @@ +// Test that the "C-unwind" ABI is feature-gated, and *can* be used when the +// `c_unwind` feature gate is enabled. + +// check-pass + +#![feature(c_unwind)] + +extern "C-unwind" fn f() {} + +fn main() { + f(); +} diff --git a/src/test/ui/unwind-abis/feature-gate-c-unwind.rs b/src/test/ui/unwind-abis/feature-gate-c-unwind.rs new file mode 100644 index 000000000000..f02a368d4e09 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-c-unwind.rs @@ -0,0 +1,9 @@ +// Test that the "C-unwind" ABI is feature-gated, and cannot be used when the +// `c_unwind` feature gate is not used. + +extern "C-unwind" fn f() {} +//~^ ERROR C-unwind ABI is experimental and subject to change [E0658] + +fn main() { + f(); +} diff --git a/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr b/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr new file mode 100644 index 000000000000..f4c785a235f6 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-c-unwind.stderr @@ -0,0 +1,12 @@ +error[E0658]: C-unwind ABI is experimental and subject to change + --> $DIR/feature-gate-c-unwind.rs:4:8 + | +LL | extern "C-unwind" fn f() {} + | ^^^^^^^^^^ + | + = note: see issue #74990 for more information + = help: add `#![feature(c_unwind)]` to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.rs b/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.rs new file mode 100644 index 000000000000..7d4dc8c9343f --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.rs @@ -0,0 +1,13 @@ +// ignore-arm stdcall isn't supported +// ignore-aarch64 stdcall isn't supported +// ignore-riscv64 stdcall isn't supported + +// Test that the "stdcall-unwind" ABI is feature-gated, and cannot be used when +// the `c_unwind` feature gate is not used. + +extern "stdcall-unwind" fn f() {} +//~^ ERROR stdcall-unwind ABI is experimental and subject to change [E0658] + +fn main() { + f(); +} diff --git a/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.stderr b/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.stderr new file mode 100644 index 000000000000..e3d569f464f8 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-stdcall-unwind.stderr @@ -0,0 +1,12 @@ +error[E0658]: stdcall-unwind ABI is experimental and subject to change + --> $DIR/feature-gate-stdcall-unwind.rs:8:8 + | +LL | extern "stdcall-unwind" fn f() {} + | ^^^^^^^^^^^^^^^^ + | + = note: see issue #74990 for more information + = help: add `#![feature(c_unwind)]` to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/unwind-abis/feature-gate-system-unwind.rs b/src/test/ui/unwind-abis/feature-gate-system-unwind.rs new file mode 100644 index 000000000000..26c2de4e8176 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-system-unwind.rs @@ -0,0 +1,9 @@ +// Test that the "system-unwind" ABI is feature-gated, and cannot be used when +// the `c_unwind` feature gate is not used. + +extern "system-unwind" fn f() {} +//~^ ERROR system-unwind ABI is experimental and subject to change [E0658] + +fn main() { + f(); +} diff --git a/src/test/ui/unwind-abis/feature-gate-system-unwind.stderr b/src/test/ui/unwind-abis/feature-gate-system-unwind.stderr new file mode 100644 index 000000000000..87877336475b --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-system-unwind.stderr @@ -0,0 +1,12 @@ +error[E0658]: system-unwind ABI is experimental and subject to change + --> $DIR/feature-gate-system-unwind.rs:4:8 + | +LL | extern "system-unwind" fn f() {} + | ^^^^^^^^^^^^^^^ + | + = note: see issue #74990 for more information + = help: add `#![feature(c_unwind)]` to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.rs b/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.rs new file mode 100644 index 000000000000..2f4cefccc196 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.rs @@ -0,0 +1,13 @@ +// ignore-arm thiscall isn't supported +// ignore-aarch64 thiscall isn't supported +// ignore-riscv64 thiscall isn't supported + +// Test that the "thiscall-unwind" ABI is feature-gated, and cannot be used when +// the `c_unwind` feature gate is not used. + +extern "thiscall-unwind" fn f() {} +//~^ ERROR thiscall-unwind ABI is experimental and subject to change [E0658] + +fn main() { + f(); +} diff --git a/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.stderr b/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.stderr new file mode 100644 index 000000000000..b103bb8d5658 --- /dev/null +++ b/src/test/ui/unwind-abis/feature-gate-thiscall-unwind.stderr @@ -0,0 +1,12 @@ +error[E0658]: thiscall-unwind ABI is experimental and subject to change + --> $DIR/feature-gate-thiscall-unwind.rs:8:8 + | +LL | extern "thiscall-unwind" fn f() {} + | ^^^^^^^^^^^^^^^^^ + | + = note: see issue #74990 for more information + = help: add `#![feature(c_unwind)]` to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. From 9a007cfdc0857a4ad0601e2e130ef53d158493ee Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Thu, 10 Sep 2020 13:38:39 -0400 Subject: [PATCH 07/25] implement unwinding abi's (RFC 2945) ### Changes This commit implements unwind ABI's, specified in RFC 2945. We adjust the `rustc_middle::ty::layout::fn_can_unwind` function, used to compute whether or not a `FnAbi` object represents a function that should be able to unwind when `panic=unwind` is in use. Changes are also made to `rustc_mir_build::build::should_abort_on_panic` so that the function ABI is used to determind whether it should abort, assuming that the `panic=unwind` strategy is being used, and no explicit unwind attribute was provided. ### Tests Unit tests, checking that the behavior is correct for `C-unwind`, `stdcall-unwind`, `system-unwind`, and `thiscall-unwind`, are included. These alternative `unwind` ABI strings are specified in RFC 2945, in the "_Other `unwind` ABI strings_" section. Additionally, a test case is included to assert that the LLVM IR generated for an external function defined with the `C-unwind` ABI will be appropriately labeled with the `nounwind` LLVM attribute when the `panic=abort` compilation flag is used. ### Ignore Directives This commit uses `ignore-*` directives in two of our `*-unwind` ABI test cases. Specifically, the `stdcall-unwind` and `thiscall-unwind` test cases ignore architectures that do not support `stdcall` and `thiscall`, respectively. These directives are cribbed from `src/test/ui/c-variadic/variadic-ffi-1.rs` for `stdcall`, and `src/test/ui/extern/extern-thiscall.rs` for `thiscall`. --- compiler/rustc_middle/src/ty/layout.rs | 29 +++++++++------- compiler/rustc_mir_build/src/build/mod.rs | 22 ++++++++++--- .../unwind-abis/c-unwind-abi-panic-abort.rs | 18 ++++++++++ src/test/codegen/unwind-abis/c-unwind-abi.rs | 29 ++++++++++++++++ .../codegen/unwind-abis/stdcall-unwind-abi.rs | 32 ++++++++++++++++++ .../codegen/unwind-abis/system-unwind-abi.rs | 29 ++++++++++++++++ .../unwind-abis/thiscall-unwind-abi.rs | 33 +++++++++++++++++++ 7 files changed, 176 insertions(+), 16 deletions(-) create mode 100644 src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs create mode 100644 src/test/codegen/unwind-abis/c-unwind-abi.rs create mode 100644 src/test/codegen/unwind-abis/stdcall-unwind-abi.rs create mode 100644 src/test/codegen/unwind-abis/system-unwind-abi.rs create mode 100644 src/test/codegen/unwind-abis/thiscall-unwind-abi.rs diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 2f5380861c49..ee2dffd8baeb 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2562,6 +2562,7 @@ fn fn_can_unwind( panic_strategy: PanicStrategy, codegen_fn_attr_flags: CodegenFnAttrFlags, call_conv: Conv, + abi: SpecAbi, ) -> bool { if panic_strategy != PanicStrategy::Unwind { // In panic=abort mode we assume nothing can unwind anywhere, so @@ -2586,17 +2587,16 @@ fn fn_can_unwind( // // 2. A Rust item using a non-Rust ABI (like `extern "C" fn foo() { ... }`). // - // Foreign items (case 1) are assumed to not unwind; it is - // UB otherwise. (At least for now; see also - // rust-lang/rust#63909 and Rust RFC 2753.) - // - // Items defined in Rust with non-Rust ABIs (case 2) are also - // not supposed to unwind. Whether this should be enforced - // (versus stating it is UB) and *how* it would be enforced - // is currently under discussion; see rust-lang/rust#58794. - // - // In either case, we mark item as explicitly nounwind. - false + // In both of these cases, we should refer to the ABI to determine whether or not we + // should unwind. See Rust RFC 2945 for more information on this behavior, here: + // https://github.com/rust-lang/rfcs/blob/master/text/2945-c-unwind-abi.md + use SpecAbi::*; + match abi { + C { unwind } | Stdcall { unwind } | System { unwind } | Thiscall { unwind } => { + unwind + } + _ => false, + } } } } @@ -2823,7 +2823,12 @@ where c_variadic: sig.c_variadic, fixed_count: inputs.len(), conv, - can_unwind: fn_can_unwind(cx.tcx().sess.panic_strategy(), codegen_fn_attr_flags, conv), + can_unwind: fn_can_unwind( + cx.tcx().sess.panic_strategy(), + codegen_fn_attr_flags, + conv, + sig.abi, + ), }; fn_abi.adjust_for_abi(cx, sig.abi); debug!("FnAbi::new_internal = {:?}", fn_abi); diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 5f6c8d26402e..dec26278290d 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -546,7 +546,7 @@ macro_rules! unpack { }}; } -fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, _abi: Abi) -> bool { +fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, abi: Abi) -> bool { // Validate `#[unwind]` syntax regardless of platform-specific panic strategy. let attrs = &tcx.get_attrs(fn_def_id.to_def_id()); let unwind_attr = attr::find_unwind_attr(&tcx.sess, attrs); @@ -556,12 +556,26 @@ fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, _abi: Abi) -> b return false; } - // This is a special case: some functions have a C abi but are meant to - // unwind anyway. Don't stop them. match unwind_attr { - None => false, // FIXME(#58794); should be `!(abi == Abi::Rust || abi == Abi::RustCall)` + // If an `#[unwind]` attribute was found, we should adhere to it. Some(UnwindAttr::Allowed) => false, Some(UnwindAttr::Aborts) => true, + // If no attribute was found and the panic strategy is `unwind`, then we should examine + // the function's ABI string to determine whether it should abort upon panic. + None => { + use Abi::*; + match abi { + // In the case of ABI's that have an `-unwind` equivalent, check whether the ABI + // permits unwinding. If so, we should not abort. Otherwise, we should. + C { unwind } | Stdcall { unwind } | System { unwind } | Thiscall { unwind } => { + !unwind + } + // Rust and `rust-call` functions are allowed to unwind, and should not abort. + Rust | RustCall => false, + // Other ABI's should abort. + _ => true, + } + } } } diff --git a/src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs b/src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs new file mode 100644 index 000000000000..afd65ff6741a --- /dev/null +++ b/src/test/codegen/unwind-abis/c-unwind-abi-panic-abort.rs @@ -0,0 +1,18 @@ +// compile-flags: -C panic=abort -C opt-level=0 + +// Test that `nounwind` atributes are applied to `C-unwind` extern functions when the +// code is compiled with `panic=abort`. We disable optimizations above to prevent LLVM from +// inferring the attribute. + +#![crate_type = "lib"] +#![feature(c_unwind)] + +// CHECK: @rust_item_that_can_unwind() unnamed_addr #0 { +#[no_mangle] +pub extern "C-unwind" fn rust_item_that_can_unwind() { +} + +// Now, make sure that the LLVM attributes for this functions are correct. First, make +// sure that the first item is correctly marked with the `nounwind` attribute: +// +// CHECK: attributes #0 = { {{.*}}nounwind{{.*}} } diff --git a/src/test/codegen/unwind-abis/c-unwind-abi.rs b/src/test/codegen/unwind-abis/c-unwind-abi.rs new file mode 100644 index 000000000000..f15765367532 --- /dev/null +++ b/src/test/codegen/unwind-abis/c-unwind-abi.rs @@ -0,0 +1,29 @@ +// compile-flags: -C opt-level=0 + +// Test that `nounwind` atributes are correctly applied to exported `C` and `C-unwind` extern +// functions. `C-unwind` functions MUST NOT have this attribute. We disable optimizations above +// to prevent LLVM from inferring the attribute. + +#![crate_type = "lib"] +#![feature(c_unwind)] + +// CHECK: @rust_item_that_cannot_unwind() unnamed_addr #0 { +#[no_mangle] +pub extern "C" fn rust_item_that_cannot_unwind() { +} + +// CHECK: @rust_item_that_can_unwind() unnamed_addr #1 { +#[no_mangle] +pub extern "C-unwind" fn rust_item_that_can_unwind() { +} + +// Now, make some assertions that the LLVM attributes for these functions are correct. First, make +// sure that the first item is correctly marked with the `nounwind` attribute: +// +// CHECK: attributes #0 = { {{.*}}nounwind{{.*}} } +// +// Next, let's assert that the second item, which CAN unwind, does not have this attribute. +// +// CHECK: attributes #1 = { +// CHECK-NOT: nounwind +// CHECK: } diff --git a/src/test/codegen/unwind-abis/stdcall-unwind-abi.rs b/src/test/codegen/unwind-abis/stdcall-unwind-abi.rs new file mode 100644 index 000000000000..ed804ca278d2 --- /dev/null +++ b/src/test/codegen/unwind-abis/stdcall-unwind-abi.rs @@ -0,0 +1,32 @@ +// compile-flags: -C opt-level=0 +// ignore-arm stdcall isn't supported +// ignore-aarch64 stdcall isn't supported +// ignore-riscv64 stdcall isn't supported + +// Test that `nounwind` atributes are correctly applied to exported `stdcall` and `stdcall-unwind` +// extern functions. `stdcall-unwind` functions MUST NOT have this attribute. We disable +// optimizations above to prevent LLVM from inferring the attribute. + +#![crate_type = "lib"] +#![feature(c_unwind)] + +// CHECK: @rust_item_that_cannot_unwind() unnamed_addr #0 { +#[no_mangle] +pub extern "stdcall" fn rust_item_that_cannot_unwind() { +} + +// CHECK: @rust_item_that_can_unwind() unnamed_addr #1 { +#[no_mangle] +pub extern "stdcall-unwind" fn rust_item_that_can_unwind() { +} + +// Now, make some assertions that the LLVM attributes for these functions are correct. First, make +// sure that the first item is correctly marked with the `nounwind` attribute: +// +// CHECK: attributes #0 = { {{.*}}nounwind{{.*}} } +// +// Next, let's assert that the second item, which CAN unwind, does not have this attribute. +// +// CHECK: attributes #1 = { +// CHECK-NOT: nounwind +// CHECK: } diff --git a/src/test/codegen/unwind-abis/system-unwind-abi.rs b/src/test/codegen/unwind-abis/system-unwind-abi.rs new file mode 100644 index 000000000000..c4d51328352c --- /dev/null +++ b/src/test/codegen/unwind-abis/system-unwind-abi.rs @@ -0,0 +1,29 @@ +// compile-flags: -C opt-level=0 + +// Test that `nounwind` atributes are correctly applied to exported `system` and `system-unwind` +// extern functions. `system-unwind` functions MUST NOT have this attribute. We disable +// optimizations above to prevent LLVM from inferring the attribute. + +#![crate_type = "lib"] +#![feature(c_unwind)] + +// CHECK: @rust_item_that_cannot_unwind() unnamed_addr #0 { +#[no_mangle] +pub extern "system" fn rust_item_that_cannot_unwind() { +} + +// CHECK: @rust_item_that_can_unwind() unnamed_addr #1 { +#[no_mangle] +pub extern "system-unwind" fn rust_item_that_can_unwind() { +} + +// Now, make some assertions that the LLVM attributes for these functions are correct. First, make +// sure that the first item is correctly marked with the `nounwind` attribute: +// +// CHECK: attributes #0 = { {{.*}}nounwind{{.*}} } +// +// Next, let's assert that the second item, which CAN unwind, does not have this attribute. +// +// CHECK: attributes #1 = { +// CHECK-NOT: nounwind +// CHECK: } diff --git a/src/test/codegen/unwind-abis/thiscall-unwind-abi.rs b/src/test/codegen/unwind-abis/thiscall-unwind-abi.rs new file mode 100644 index 000000000000..aaa63ae55c3a --- /dev/null +++ b/src/test/codegen/unwind-abis/thiscall-unwind-abi.rs @@ -0,0 +1,33 @@ +// compile-flags: -C opt-level=0 +// ignore-arm thiscall isn't supported +// ignore-aarch64 thiscall isn't supported +// ignore-riscv64 thiscall isn't supported + +// Test that `nounwind` atributes are correctly applied to exported `thiscall` and +// `thiscall-unwind` extern functions. `thiscall-unwind` functions MUST NOT have this attribute. We +// disable optimizations above to prevent LLVM from inferring the attribute. + +#![crate_type = "lib"] +#![feature(abi_thiscall)] +#![feature(c_unwind)] + +// CHECK: @rust_item_that_cannot_unwind() unnamed_addr #0 { +#[no_mangle] +pub extern "thiscall" fn rust_item_that_cannot_unwind() { +} + +// CHECK: @rust_item_that_can_unwind() unnamed_addr #1 { +#[no_mangle] +pub extern "thiscall-unwind" fn rust_item_that_can_unwind() { +} + +// Now, make some assertions that the LLVM attributes for these functions are correct. First, make +// sure that the first item is correctly marked with the `nounwind` attribute: +// +// CHECK: attributes #0 = { {{.*}}nounwind{{.*}} } +// +// Next, let's assert that the second item, which CAN unwind, does not have this attribute. +// +// CHECK: attributes #1 = { +// CHECK-NOT: nounwind +// CHECK: } From 0fd2fd92dcc0c255bb5945331b82022171ae4fa5 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Fri, 11 Dec 2020 20:54:47 -0500 Subject: [PATCH 08/25] add integration tests, unwind across FFI boundary ### Integration Tests This commit introduces some new fixtures to the `run-make-fulldeps` test suite. * c-unwind-abi-catch-panic: Exercise unwinding a panic. This catches a panic across an FFI boundary and downcasts it into an integer. * c-unwind-abi-catch-lib-panic: This is similar to the previous `*catch-panic` test, however in this case the Rust code that panics resides in a separate crate. ### Add `rust_eh_personality` to `#[no_std]` alloc tests This commit addresses some test failures that now occur in the following two tests: * no_std-alloc-error-handler-custom.rs * no_std-alloc-error-handler-default.rs Each test now defines a `rust_eh_personality` extern function, in the same manner as shown in the "Writing an executable without stdlib" section of the `lang_items` documentation here: https://doc.rust-lang.org/unstable-book/language-features/lang-items.html#writing-an-executable-without-stdlib Without this change, these tests would fail to compile due to a linking error explaining that there was an "undefined reference to `rust_eh_personality'." ### Updated hash * update 32-bit hash in `impl1` test ### Panics This commit uses `panic!` macro invocations that return a string, rather than using an integer as a panic payload. Doing so avoids the following warnings that were observed during rollup for the `*-msvc-1` targets: ``` warning: panic message is not a string literal --> panic.rs:10:16 | 10 | panic!(x); // That is too big! | ^ | = note: `#[warn(non_fmt_panic)]` on by default = note: this is no longer accepted in Rust 2021 help: add a "{}" format string to Display the message | 10 | panic!("{}", x); // That is too big! | ^^^^^ help: or use std::panic::panic_any instead | 10 | std::panic::panic_any(x); // That is too big! | ^^^^^^^^^^^^^^^^^^^^^ warning: 1 warning emitted ``` See: https://github.com/rust-lang-ci/rust/runs/1992118428 As these errors imply, panicking without a format string will be disallowed in Rust 2021, per #78500. --- .../c-unwind-abi-catch-lib-panic/Makefile | 30 +++++++++++++ .../c-unwind-abi-catch-lib-panic/add.c | 12 +++++ .../c-unwind-abi-catch-lib-panic/main.rs | 35 +++++++++++++++ .../c-unwind-abi-catch-lib-panic/panic.rs | 12 +++++ .../c-unwind-abi-catch-panic/Makefile | 5 +++ .../c-unwind-abi-catch-panic/add.c | 12 +++++ .../c-unwind-abi-catch-panic/main.rs | 44 +++++++++++++++++++ .../no_std-alloc-error-handler-custom.rs | 9 +++- .../no_std-alloc-error-handler-default.rs | 9 +++- src/test/ui/symbol-names/impl1.rs | 6 +-- 10 files changed, 167 insertions(+), 7 deletions(-) create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/Makefile create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/add.c create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/main.rs create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/panic.rs create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-panic/Makefile create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-panic/add.c create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-catch-panic/main.rs diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/Makefile b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/Makefile new file mode 100644 index 000000000000..a8515c533af5 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/Makefile @@ -0,0 +1,30 @@ +-include ../tools.mk + +all: archive + # Compile `main.rs`, which will link into our library, and run it. + $(RUSTC) main.rs + $(call RUN,main) + +ifdef IS_MSVC +archive: add.o panic.o + # Now, create an archive using these two objects. + $(AR) crus $(TMPDIR)/add.lib $(TMPDIR)/add.o $(TMPDIR)/panic.o +else +archive: add.o panic.o + # Now, create an archive using these two objects. + $(AR) crus $(TMPDIR)/libadd.a $(TMPDIR)/add.o $(TMPDIR)/panic.o +endif + +# Compile `panic.rs` into an object file. +# +# Note that we invoke `rustc` directly, so we may emit an object rather +# than an archive. We'll do that later. +panic.o: + $(BARE_RUSTC) $(RUSTFLAGS) \ + --out-dir $(TMPDIR) \ + --emit=obj panic.rs + +# Compile `add.c` into an object file. +add.o: + $(call COMPILE_OBJ,$(TMPDIR)/add.o,add.c) + diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/add.c b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/add.c new file mode 100644 index 000000000000..444359451f6e --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/add.c @@ -0,0 +1,12 @@ +#ifdef _WIN32 +__declspec(dllexport) +#endif + +// An external function, defined in Rust. +extern void panic_if_greater_than_10(unsigned x); + +unsigned add_small_numbers(unsigned a, unsigned b) { + unsigned c = a + b; + panic_if_greater_than_10(c); + return c; +} diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/main.rs b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/main.rs new file mode 100644 index 000000000000..78a71219c781 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/main.rs @@ -0,0 +1,35 @@ +//! A test for calling `C-unwind` functions across foreign function boundaries. +//! +//! This test triggers a panic in a Rust library that our foreign function invokes. This shows +//! that we can unwind through the C code in that library, and catch the underlying panic. +#![feature(c_unwind)] + +use std::panic::{catch_unwind, AssertUnwindSafe}; + +fn main() { + // Call `add_small_numbers`, passing arguments that will NOT trigger a panic. + let (a, b) = (9, 1); + let c = unsafe { add_small_numbers(a, b) }; + assert_eq!(c, 10); + + // Call `add_small_numbers`, passing arguments that will trigger a panic, and catch it. + let caught_unwind = catch_unwind(AssertUnwindSafe(|| { + let (a, b) = (10, 1); + let _c = unsafe { add_small_numbers(a, b) }; + unreachable!("should have unwound instead of returned"); + })); + + // Assert that we did indeed panic, then unwrap and downcast the panic into the sum. + assert!(caught_unwind.is_err()); + let panic_obj = caught_unwind.unwrap_err(); + let msg = panic_obj.downcast_ref::().unwrap(); + assert_eq!(msg, "11"); +} + +#[link(name = "add", kind = "static")] +extern "C-unwind" { + /// An external function, defined in C. + /// + /// Returns the sum of two numbers, or panics if the sum is greater than 10. + fn add_small_numbers(a: u32, b: u32) -> u32; +} diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/panic.rs b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/panic.rs new file mode 100644 index 000000000000..a99a04d5c6f4 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/panic.rs @@ -0,0 +1,12 @@ +#![crate_type = "staticlib"] +#![feature(c_unwind)] + +/// This function will panic if `x` is greater than 10. +/// +/// This function is called by `add_small_numbers`. +#[no_mangle] +pub extern "C-unwind" fn panic_if_greater_than_10(x: u32) { + if x > 10 { + panic!("{}", x); // That is too big! + } +} diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/Makefile b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/Makefile new file mode 100644 index 000000000000..9553b7aeeb98 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/Makefile @@ -0,0 +1,5 @@ +-include ../tools.mk + +all: $(call NATIVE_STATICLIB,add) + $(RUSTC) main.rs + $(call RUN,main) || exit 1 diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/add.c b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/add.c new file mode 100644 index 000000000000..444359451f6e --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/add.c @@ -0,0 +1,12 @@ +#ifdef _WIN32 +__declspec(dllexport) +#endif + +// An external function, defined in Rust. +extern void panic_if_greater_than_10(unsigned x); + +unsigned add_small_numbers(unsigned a, unsigned b) { + unsigned c = a + b; + panic_if_greater_than_10(c); + return c; +} diff --git a/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/main.rs b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/main.rs new file mode 100644 index 000000000000..15d38d721605 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/main.rs @@ -0,0 +1,44 @@ +//! A test for calling `C-unwind` functions across foreign function boundaries. +//! +//! This test triggers a panic when calling a foreign function that calls *back* into Rust. +#![feature(c_unwind)] + +use std::panic::{catch_unwind, AssertUnwindSafe}; + +fn main() { + // Call `add_small_numbers`, passing arguments that will NOT trigger a panic. + let (a, b) = (9, 1); + let c = unsafe { add_small_numbers(a, b) }; + assert_eq!(c, 10); + + // Call `add_small_numbers`, passing arguments that will trigger a panic, and catch it. + let caught_unwind = catch_unwind(AssertUnwindSafe(|| { + let (a, b) = (10, 1); + let _c = unsafe { add_small_numbers(a, b) }; + unreachable!("should have unwound instead of returned"); + })); + + // Assert that we did indeed panic, then unwrap and downcast the panic into the sum. + assert!(caught_unwind.is_err()); + let panic_obj = caught_unwind.unwrap_err(); + let msg = panic_obj.downcast_ref::().unwrap(); + assert_eq!(msg, "11"); +} + +#[link(name = "add", kind = "static")] +extern "C-unwind" { + /// An external function, defined in C. + /// + /// Returns the sum of two numbers, or panics if the sum is greater than 10. + fn add_small_numbers(a: u32, b: u32) -> u32; +} + +/// This function will panic if `x` is greater than 10. +/// +/// This function is called by `add_small_numbers`. +#[no_mangle] +pub extern "C-unwind" fn panic_if_greater_than_10(x: u32) { + if x > 10 { + panic!("{}", x); // That is too big! + } +} diff --git a/src/test/ui/allocator/no_std-alloc-error-handler-custom.rs b/src/test/ui/allocator/no_std-alloc-error-handler-custom.rs index 4d40c7d0d223..c9b4abbfd3fd 100644 --- a/src/test/ui/allocator/no_std-alloc-error-handler-custom.rs +++ b/src/test/ui/allocator/no_std-alloc-error-handler-custom.rs @@ -7,7 +7,7 @@ // compile-flags:-C panic=abort // aux-build:helper.rs -#![feature(start, rustc_private, new_uninit, panic_info_message)] +#![feature(start, rustc_private, new_uninit, panic_info_message, lang_items)] #![feature(alloc_error_handler)] #![no_std] @@ -84,6 +84,13 @@ fn panic(panic_info: &core::panic::PanicInfo) -> ! { } } +// Because we are compiling this code with `-C panic=abort`, this wouldn't normally be needed. +// However, `core` and `alloc` are both compiled with `-C panic=unwind`, which means that functions +// in these libaries will refer to `rust_eh_personality` if LLVM can not *prove* the contents won't +// unwind. So, for this test case we will define the symbol. +#[lang = "eh_personality"] +extern fn rust_eh_personality() {} + #[derive(Debug)] struct Page([[u64; 32]; 16]); diff --git a/src/test/ui/allocator/no_std-alloc-error-handler-default.rs b/src/test/ui/allocator/no_std-alloc-error-handler-default.rs index 4f8c44f1763c..d6cd4a6af855 100644 --- a/src/test/ui/allocator/no_std-alloc-error-handler-default.rs +++ b/src/test/ui/allocator/no_std-alloc-error-handler-default.rs @@ -8,7 +8,7 @@ // aux-build:helper.rs // gate-test-default_alloc_error_handler -#![feature(start, rustc_private, new_uninit, panic_info_message)] +#![feature(start, rustc_private, new_uninit, panic_info_message, lang_items)] #![feature(default_alloc_error_handler)] #![no_std] @@ -71,6 +71,13 @@ fn panic(panic_info: &core::panic::PanicInfo) -> ! { } } +// Because we are compiling this code with `-C panic=abort`, this wouldn't normally be needed. +// However, `core` and `alloc` are both compiled with `-C panic=unwind`, which means that functions +// in these libaries will refer to `rust_eh_personality` if LLVM can not *prove* the contents won't +// unwind. So, for this test case we will define the symbol. +#[lang = "eh_personality"] +extern fn rust_eh_personality() {} + #[derive(Debug)] struct Page([[u64; 32]; 16]); diff --git a/src/test/ui/symbol-names/impl1.rs b/src/test/ui/symbol-names/impl1.rs index 7dd74bd229d3..a352d255c782 100644 --- a/src/test/ui/symbol-names/impl1.rs +++ b/src/test/ui/symbol-names/impl1.rs @@ -3,7 +3,7 @@ // revisions: legacy v0 //[legacy]compile-flags: -Z symbol-mangling-version=legacy //[v0]compile-flags: -Z symbol-mangling-version=v0 -//[legacy]normalize-stderr-32bit: "hee444285569b39c2" -> "SYMBOL_HASH" +//[legacy]normalize-stderr-32bit: "h13cfe136e83a9196" -> "SYMBOL_HASH" //[legacy]normalize-stderr-64bit: "hd949d7797008991f" -> "SYMBOL_HASH" #![feature(auto_traits, rustc_attrs)] @@ -75,7 +75,3 @@ fn main() { } }; } - -// FIXME(katie): The 32-bit symbol hash probably needs updating as well, but I'm slightly unsure -// about how to do that. This comment is here so that we don't break the test due to error messages -// including incorrect line numbers. From d8bfdc8ec4cc3b1aab0fed675bafa6d65aab1672 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Fri, 23 Oct 2020 18:49:34 -0400 Subject: [PATCH 09/25] address pr review comments ### Add debug assertion to check `AbiDatas` ordering This makes a small alteration to `Abi::index`, so that we include a debug assertion to check that the index we are returning corresponds with the same abi in our data array. This will help prevent ordering bugs in the future, which can manifest in rather strange errors. ### Using exhaustive ABI matches This slightly modifies the changes from our previous commits, favoring exhaustive matches in place of `_ => ...` fall-through arms. This should help with maintenance in the future, when additional ABI's are added, or when existing ABI's are modified. ### List all `-unwind` ABI's in unstable book This updates the `c-unwind` page in the unstable book to list _all_ of the other ABI strings that are introduced by this feature gate. Now, all of the ABI's specified by RFC 2945 are shown. Co-authored-by: Amanieu d'Antras Co-authored-by: Niko Matsakis --- compiler/rustc_middle/src/ty/layout.rs | 20 ++++++++++++++++++- compiler/rustc_mir_build/src/build/mod.rs | 18 ++++++++++++++++- compiler/rustc_target/src/spec/abi.rs | 15 ++++++++++++-- .../src/language-features/c-unwind.md | 3 ++- 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index ee2dffd8baeb..814581a6cf17 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2595,7 +2595,25 @@ fn fn_can_unwind( C { unwind } | Stdcall { unwind } | System { unwind } | Thiscall { unwind } => { unwind } - _ => false, + Cdecl + | Fastcall + | Vectorcall + | Aapcs + | Win64 + | SysV64 + | PtxKernel + | Msp430Interrupt + | X86Interrupt + | AmdGpuKernel + | EfiApi + | AvrInterrupt + | AvrNonBlockingInterrupt + | CCmseNonSecureCall + | RustIntrinsic + | PlatformIntrinsic + | Unadjusted => false, + // In the `if` above, we checked for functions with the Rust calling convention. + Rust | RustCall => unreachable!(), } } } diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index dec26278290d..08a23d207015 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -573,7 +573,23 @@ fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: LocalDefId, abi: Abi) -> bo // Rust and `rust-call` functions are allowed to unwind, and should not abort. Rust | RustCall => false, // Other ABI's should abort. - _ => true, + Cdecl + | Fastcall + | Vectorcall + | Aapcs + | Win64 + | SysV64 + | PtxKernel + | Msp430Interrupt + | X86Interrupt + | AmdGpuKernel + | EfiApi + | AvrInterrupt + | AvrNonBlockingInterrupt + | CCmseNonSecureCall + | RustIntrinsic + | PlatformIntrinsic + | Unadjusted => true, } } } diff --git a/compiler/rustc_target/src/spec/abi.rs b/compiler/rustc_target/src/spec/abi.rs index f7f9c30d3b7a..17eb33b8f2ea 100644 --- a/compiler/rustc_target/src/spec/abi.rs +++ b/compiler/rustc_target/src/spec/abi.rs @@ -107,7 +107,7 @@ impl Abi { // N.B., this ordering MUST match the AbiDatas array above. // (This is ensured by the test indices_are_correct().) use Abi::*; - match self { + let i = match self { // Cross-platform ABIs Rust => 0, C { unwind: false } => 1, @@ -138,7 +138,18 @@ impl Abi { RustCall => 24, PlatformIntrinsic => 25, Unadjusted => 26, - } + }; + debug_assert!( + AbiDatas + .iter() + .enumerate() + .find(|(_, AbiData { abi, .. })| *abi == self) + .map(|(index, _)| index) + .expect("abi variant has associated data") + == i, + "Abi index did not match `AbiDatas` ordering" + ); + i } #[inline] diff --git a/src/doc/unstable-book/src/language-features/c-unwind.md b/src/doc/unstable-book/src/language-features/c-unwind.md index c1705d59acc2..2801d9b5e777 100644 --- a/src/doc/unstable-book/src/language-features/c-unwind.md +++ b/src/doc/unstable-book/src/language-features/c-unwind.md @@ -6,7 +6,8 @@ The tracking issue for this feature is: [#74990] ------------------------ -Introduces a new ABI string, "C-unwind", to enable unwinding from other +Introduces four new ABI strings: "C-unwind", "stdcall-unwind", +"thiscall-unwind", and "system-unwind". These enable unwinding from other languages (such as C++) into Rust frames and from Rust into other languages. See [RFC 2945] for more information. From b3c4e25e1740da69686f940165dd3c9d563d54e5 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 22 Feb 2021 09:31:41 -0500 Subject: [PATCH 10/25] Don't require a `DocContext` for `report_diagnostic` This is needed for the next commit, which needs access to the `cx` from within the `decorate` closure. - Change `as_local_hir_id` to an associated function, since it only needs a `TyCtxt` - Change `source_span_for_markdown_range` to only take a `TyCtxt` --- src/librustdoc/core.rs | 6 +++--- .../passes/check_code_block_syntax.rs | 9 ++++++--- .../passes/collect_intra_doc_links.rs | 20 +++++++++---------- src/librustdoc/passes/doc_test_lints.rs | 2 +- src/librustdoc/passes/html_tags.rs | 8 ++++---- src/librustdoc/passes/mod.rs | 5 +++-- src/librustdoc/passes/non_autolinks.rs | 4 ++-- 7 files changed, 29 insertions(+), 25 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 9cb0649b314f..5ff50e253805 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -169,13 +169,13 @@ impl<'tcx> DocContext<'tcx> { /// Like `hir().local_def_id_to_hir_id()`, but skips calling it on fake DefIds. /// (This avoids a slice-index-out-of-bounds panic.) - crate fn as_local_hir_id(&self, def_id: DefId) -> Option { + crate fn as_local_hir_id(tcx: TyCtxt<'_>, def_id: DefId) -> Option { if MAX_DEF_IDX.with(|m| { m.borrow().get(&def_id.krate).map(|&idx| idx <= def_id.index).unwrap_or(false) }) { None } else { - def_id.as_local().map(|def_id| self.tcx.hir().local_def_id_to_hir_id(def_id)) + def_id.as_local().map(|def_id| tcx.hir().local_def_id_to_hir_id(def_id)) } } } @@ -479,7 +479,7 @@ crate fn run_global_ctxt( https://doc.rust-lang.org/nightly/rustdoc/how-to-write-documentation.html"; tcx.struct_lint_node( crate::lint::MISSING_CRATE_LEVEL_DOCS, - ctxt.as_local_hir_id(m.def_id).unwrap(), + DocContext::as_local_hir_id(tcx, m.def_id).unwrap(), |lint| { let mut diag = lint.build("no documentation found for this crate's top-level module"); diff --git a/src/librustdoc/passes/check_code_block_syntax.rs b/src/librustdoc/passes/check_code_block_syntax.rs index c85490864ec7..98886139f308 100644 --- a/src/librustdoc/passes/check_code_block_syntax.rs +++ b/src/librustdoc/passes/check_code_block_syntax.rs @@ -48,9 +48,12 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> { let buffer = buffer.borrow(); if buffer.has_errors || is_empty { - let mut diag = if let Some(sp) = - super::source_span_for_markdown_range(self.cx, &dox, &code_block.range, &item.attrs) - { + let mut diag = if let Some(sp) = super::source_span_for_markdown_range( + self.cx.tcx, + &dox, + &code_block.range, + &item.attrs, + ) { let (warning_message, suggest_using_text) = if buffer.has_errors { ("could not parse code block as Rust code", true) } else { diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 38efecb393b7..93aa06959369 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1146,7 +1146,7 @@ impl LinkCollector<'_, '_> { suggest_disambiguator(resolved, diag, path_str, dox, sp, &ori_link.range); }; report_diagnostic( - self.cx, + self.cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, &item, @@ -1220,7 +1220,7 @@ impl LinkCollector<'_, '_> { && !self.cx.tcx.features().intra_doc_pointers { let span = super::source_span_for_markdown_range( - self.cx, + self.cx.tcx, dox, &ori_link.range, &item.attrs, @@ -1674,7 +1674,7 @@ impl Suggestion { /// parameter of the callback will contain it, and the primary span of the diagnostic will be set /// to it. fn report_diagnostic( - cx: &DocContext<'_>, + tcx: TyCtxt<'_>, lint: &'static Lint, msg: &str, item: &Item, @@ -1682,7 +1682,7 @@ fn report_diagnostic( link_range: &Range, decorate: impl FnOnce(&mut DiagnosticBuilder<'_>, Option), ) { - let hir_id = match cx.as_local_hir_id(item.def_id) { + let hir_id = match DocContext::as_local_hir_id(tcx, item.def_id) { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. @@ -1694,10 +1694,10 @@ fn report_diagnostic( let attrs = &item.attrs; let sp = span_of_attrs(attrs).unwrap_or(item.source.span()); - cx.tcx.struct_span_lint_hir(lint, hir_id, sp, |lint| { + tcx.struct_span_lint_hir(lint, hir_id, sp, |lint| { let mut diag = lint.build(msg); - let span = super::source_span_for_markdown_range(cx, dox, link_range, attrs); + let span = super::source_span_for_markdown_range(tcx, dox, link_range, attrs); if let Some(sp) = span { diag.set_span(sp); @@ -1742,7 +1742,7 @@ fn resolution_failure( ) { let tcx = collector.cx.tcx; report_diagnostic( - collector.cx, + tcx, BROKEN_INTRA_DOC_LINKS, &format!("unresolved link to `{}`", path_str), item, @@ -1973,7 +1973,7 @@ fn anchor_failure( ), }; - report_diagnostic(cx, BROKEN_INTRA_DOC_LINKS, &msg, item, dox, &link_range, |diag, sp| { + report_diagnostic(cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, item, dox, &link_range, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "contains invalid anchor"); } @@ -2013,7 +2013,7 @@ fn ambiguity_error( } } - report_diagnostic(cx, BROKEN_INTRA_DOC_LINKS, &msg, item, dox, &link_range, |diag, sp| { + report_diagnostic(cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, item, dox, &link_range, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "ambiguous link"); } else { @@ -2066,7 +2066,7 @@ fn privacy_error(cx: &DocContext<'_>, item: &Item, path_str: &str, dox: &str, li let msg = format!("public documentation for `{}` links to private item `{}`", item_name, path_str); - report_diagnostic(cx, PRIVATE_INTRA_DOC_LINKS, &msg, item, dox, &link.range, |diag, sp| { + report_diagnostic(cx.tcx, PRIVATE_INTRA_DOC_LINKS, &msg, item, dox, &link.range, |diag, sp| { if let Some(sp) = sp { diag.span_label(sp, "this item is private"); } diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index 81104236314d..ed6d2d59845f 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -73,7 +73,7 @@ crate fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) -> boo } crate fn look_for_tests<'tcx>(cx: &DocContext<'tcx>, dox: &str, item: &Item) { - let hir_id = match cx.as_local_hir_id(item.def_id) { + let hir_id = match DocContext::as_local_hir_id(cx.tcx, item.def_id) { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index 27e669aa44fc..5fff7c8def31 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -167,7 +167,8 @@ fn extract_tags( impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { fn fold_item(&mut self, item: Item) -> Option { - let hir_id = match self.cx.as_local_hir_id(item.def_id) { + let tcx = self.cx.tcx; + let hir_id = match DocContext::as_local_hir_id(tcx, item.def_id) { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. @@ -176,13 +177,12 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { }; let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); if !dox.is_empty() { - let cx = &self.cx; let report_diag = |msg: &str, range: &Range| { - let sp = match super::source_span_for_markdown_range(cx, &dox, range, &item.attrs) { + let sp = match super::source_span_for_markdown_range(tcx, &dox, range, &item.attrs) { Some(sp) => sp, None => span_of_attrs(&item.attrs).unwrap_or(item.source.span()), }; - cx.tcx.struct_span_lint_hir(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| { + tcx.struct_span_lint_hir(crate::lint::INVALID_HTML_TAGS, hir_id, sp, |lint| { lint.build(msg).emit() }); }; diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 5813732facb6..4c639c8496db 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -1,6 +1,7 @@ //! Contains information about "passes", used to modify crate information during the documentation //! process. +use rustc_middle::ty::TyCtxt; use rustc_span::{InnerSpan, Span, DUMMY_SP}; use std::ops::Range; @@ -167,7 +168,7 @@ crate fn span_of_attrs(attrs: &clean::Attributes) -> Option { /// attributes are not all sugared doc comments. It's difficult to calculate the correct span in /// that case due to escaping and other source features. crate fn source_span_for_markdown_range( - cx: &DocContext<'_>, + tcx: TyCtxt<'_>, markdown: &str, md_range: &Range, attrs: &clean::Attributes, @@ -179,7 +180,7 @@ crate fn source_span_for_markdown_range( return None; } - let snippet = cx.sess().source_map().span_to_snippet(span_of_attrs(attrs)?).ok()?; + let snippet = tcx.sess.source_map().span_to_snippet(span_of_attrs(attrs)?).ok()?; let starting_line = markdown[..md_range.start].matches('\n').count(); let ending_line = starting_line + markdown[md_range.start..md_range.end].matches('\n').count(); diff --git a/src/librustdoc/passes/non_autolinks.rs b/src/librustdoc/passes/non_autolinks.rs index 09a1959fa113..16ad940c6271 100644 --- a/src/librustdoc/passes/non_autolinks.rs +++ b/src/librustdoc/passes/non_autolinks.rs @@ -60,7 +60,7 @@ crate fn check_non_autolinks(krate: Crate, cx: &mut DocContext<'_>) -> Crate { impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> { fn fold_item(&mut self, item: Item) -> Option { - let hir_id = match self.cx.as_local_hir_id(item.def_id) { + let hir_id = match DocContext::as_local_hir_id(self.cx.tcx, item.def_id) { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. @@ -70,7 +70,7 @@ impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> { let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); if !dox.is_empty() { let report_diag = |cx: &DocContext<'_>, msg: &str, url: &str, range: Range| { - let sp = super::source_span_for_markdown_range(cx, &dox, &range, &item.attrs) + let sp = super::source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs) .or_else(|| span_of_attrs(&item.attrs)) .unwrap_or(item.source.span()); cx.tcx.struct_span_lint_hir(crate::lint::NON_AUTOLINKS, hir_id, sp, |lint| { From 675edd0231fd799f130ae2d46e079b3753475449 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 22 Feb 2021 09:21:47 -0500 Subject: [PATCH 11/25] Remove RefCell around module_trait_cache --- src/librustdoc/core.rs | 4 +- .../passes/collect_intra_doc_links.rs | 47 ++++++++++--------- src/librustdoc/passes/html_tags.rs | 3 +- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 5ff50e253805..bcbc6e557d0a 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -76,7 +76,7 @@ crate struct DocContext<'tcx> { /// /// See `collect_intra_doc_links::traits_implemented_by` for more details. /// `map>` - crate module_trait_cache: RefCell>>, + crate module_trait_cache: FxHashMap>, /// This same cache is used throughout rustdoc, including in [`crate::html::render`]. crate cache: Cache, /// Used by [`clean::inline`] to tell if an item has already been inlined. @@ -450,7 +450,7 @@ crate fn run_global_ctxt( .cloned() .filter(|trait_def_id| tcx.trait_is_auto(*trait_def_id)) .collect(), - module_trait_cache: RefCell::new(FxHashMap::default()), + module_trait_cache: FxHashMap::default(), cache: Cache::new(access_levels, render_options.document_private), inlined: FxHashSet::default(), output_format, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 93aa06959369..ec5994b762b8 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -484,13 +484,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// Resolves a string as a path within a particular namespace. Returns an /// optional URL fragment in the case of variants and methods. fn resolve<'path>( - &self, + &mut self, path_str: &'path str, ns: Namespace, module_id: DefId, extra_fragment: &Option, ) -> Result<(Res, Option), ErrorKind<'path>> { - let cx = &self.cx; + let tcx = self.cx.tcx; if let Some(res) = self.resolve_path(path_str, ns, module_id) { match res { @@ -498,7 +498,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // item a separate function. Res::Def(DefKind::AssocFn | DefKind::AssocConst, _) => assert_eq!(ns, ValueNS), Res::Def(DefKind::AssocTy, _) => assert_eq!(ns, TypeNS), - Res::Def(DefKind::Variant, _) => return handle_variant(cx, res, extra_fragment), + Res::Def(DefKind::Variant, _) => { + return handle_variant(self.cx, res, extra_fragment); + } // Not a trait item; just return what we found. Res::Primitive(ty) => { if extra_fragment.is_some() { @@ -565,13 +567,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ) => { debug!("looking for associated item named {} for item {:?}", item_name, did); // Checks if item_name belongs to `impl SomeItem` - let assoc_item = cx - .tcx + let assoc_item = tcx .inherent_impls(did) .iter() .flat_map(|&imp| { - cx.tcx.associated_items(imp).find_by_name_and_namespace( - cx.tcx, + tcx.associated_items(imp).find_by_name_and_namespace( + tcx, Ident::with_dummy_span(item_name), ns, imp, @@ -587,7 +588,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // something like [`ambi_fn`](::ambi_fn) .or_else(|| { let kind = - resolve_associated_trait_item(did, module_id, item_name, ns, &self.cx); + resolve_associated_trait_item(did, module_id, item_name, ns, self.cx); debug!("got associated item kind {:?}", kind); kind }); @@ -611,7 +612,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { debug!("looking for variants or fields named {} for {:?}", item_name, did); // FIXME(jynelson): why is this different from // `variant_field`? - match cx.tcx.type_of(did).kind() { + match tcx.type_of(did).kind() { ty::Adt(def, _) => { let field = if def.is_enum() { def.all_fields().find(|item| item.ident.name == item_name) @@ -652,10 +653,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { None } } - Res::Def(DefKind::Trait, did) => cx - .tcx + Res::Def(DefKind::Trait, did) => tcx .associated_items(did) - .find_by_name_and_namespace(cx.tcx, Ident::with_dummy_span(item_name), ns, did) + .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, did) .map(|item| { let kind = match item.kind { ty::AssocKind::Const => "associatedconstant", @@ -699,7 +699,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { /// This returns the `Res` even if it was erroneous for some reason /// (such as having invalid URL fragments or being in the wrong namespace). fn check_full_res( - &self, + &mut self, ns: Namespace, path_str: &str, module_id: DefId, @@ -733,7 +733,7 @@ fn resolve_associated_trait_item( module: DefId, item_name: Symbol, ns: Namespace, - cx: &DocContext<'_>, + cx: &mut DocContext<'_>, ) -> Option<(ty::AssocKind, DefId)> { // FIXME: this should also consider blanket impls (`impl X for T`). Unfortunately // `get_auto_trait_and_blanket_impls` is broken because the caching behavior is wrong. In the @@ -758,10 +758,10 @@ fn resolve_associated_trait_item( /// /// NOTE: this cannot be a query because more traits could be available when more crates are compiled! /// So it is not stable to serialize cross-crate. -fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> FxHashSet { - let mut cache = cx.module_trait_cache.borrow_mut(); - let in_scope_traits = cache.entry(module).or_insert_with(|| { - cx.enter_resolver(|resolver| { +fn traits_implemented_by(cx: &mut DocContext<'_>, type_: DefId, module: DefId) -> FxHashSet { + let mut resolver = cx.resolver.borrow_mut(); + let in_scope_traits = cx.module_trait_cache.entry(module).or_insert_with(|| { + resolver.access(|resolver| { let parent_scope = &ParentScope::module(resolver.get_module(module), resolver); resolver .traits_in_scope(None, parent_scope, SyntaxContext::root(), None) @@ -771,13 +771,14 @@ fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> Fx }) }); - let ty = cx.tcx.type_of(type_); + let tcx = cx.tcx; + let ty = tcx.type_of(type_); let iter = in_scope_traits.iter().flat_map(|&trait_| { trace!("considering explicit impl for trait {:?}", trait_); // Look at each trait implementation to see if it's an impl for `did` - cx.tcx.find_map_relevant_impl(trait_, ty, |impl_| { - let trait_ref = cx.tcx.impl_trait_ref(impl_).expect("this is not an inherent impl"); + tcx.find_map_relevant_impl(trait_, ty, |impl_| { + let trait_ref = tcx.impl_trait_ref(impl_).expect("this is not an inherent impl"); // Check if these are the same type. let impl_type = trait_ref.self_ty(); trace!( @@ -1308,7 +1309,7 @@ impl LinkCollector<'_, '_> { /// After parsing the disambiguator, resolve the main part of the link. // FIXME(jynelson): wow this is just so much fn resolve_with_disambiguator( - &self, + &mut self, key: &ResolutionInfo, diag: DiagnosticInfo<'_>, ) -> Option<(Res, Option)> { @@ -1732,7 +1733,7 @@ fn report_diagnostic( /// handled earlier. For example, if passed `Item::Crate(std)` and `path_str` /// `std::io::Error::x`, this will resolve `std::io::Error`. fn resolution_failure( - collector: &LinkCollector<'_, '_>, + collector: &mut LinkCollector<'_, '_>, item: &Item, path_str: &str, disambiguator: Option, diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index 5fff7c8def31..23364b6fec9d 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -178,7 +178,8 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); if !dox.is_empty() { let report_diag = |msg: &str, range: &Range| { - let sp = match super::source_span_for_markdown_range(tcx, &dox, range, &item.attrs) { + let sp = match super::source_span_for_markdown_range(tcx, &dox, range, &item.attrs) + { Some(sp) => sp, None => span_of_attrs(&item.attrs).unwrap_or(item.source.span()), }; From bc18eb471772403de20cd9bc0a836ce1f5e09e98 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 16:12:11 +0300 Subject: [PATCH 12/25] expand: Remove obsolete `DirectoryOwnership::UnownedViaMod` This ownership kind is only constructed in the case of path attributes like `#[path = ".."]` without a file name segment, which always represent some kind of directories and will produce and error on attempt to parse them as a module file. --- compiler/rustc_expand/src/module.rs | 61 +++----------------- src/test/ui/modules/path-no-file-name.rs | 7 +++ src/test/ui/modules/path-no-file-name.stderr | 8 +++ 3 files changed, 24 insertions(+), 52 deletions(-) create mode 100644 src/test/ui/modules/path-no-file-name.rs create mode 100644 src/test/ui/modules/path-no-file-name.stderr diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 076d3b02be93..99c1591d40a8 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -22,7 +22,6 @@ pub enum DirectoryOwnership { relative: Option, }, UnownedViaBlock, - UnownedViaMod, } /// Information about the path to a module. @@ -134,23 +133,20 @@ fn submod_path<'a>( dir_path: &Path, ) -> PResult<'a, ModulePathSuccess> { if let Some(path) = submod_path_from_attr(sess, attrs, dir_path) { - let ownership = match path.file_name().and_then(|s| s.to_str()) { - // All `#[path]` files are treated as though they are a `mod.rs` file. - // This means that `mod foo;` declarations inside `#[path]`-included - // files are siblings, - // - // Note that this will produce weirdness when a file named `foo.rs` is - // `#[path]` included and contains a `mod foo;` declaration. - // If you encounter this, it's your own darn fault :P - Some(_) => DirectoryOwnership::Owned { relative: None }, - _ => DirectoryOwnership::UnownedViaMod, - }; + // All `#[path]` files are treated as though they are a `mod.rs` file. + // This means that `mod foo;` declarations inside `#[path]`-included + // files are siblings, + // + // Note that this will produce weirdness when a file named `foo.rs` is + // `#[path]` included and contains a `mod foo;` declaration. + // If you encounter this, it's your own darn fault :P + let ownership = DirectoryOwnership::Owned { relative: None }; return Ok(ModulePathSuccess { ownership, path }); } let relative = match ownership { DirectoryOwnership::Owned { relative } => relative, - DirectoryOwnership::UnownedViaBlock | DirectoryOwnership::UnownedViaMod => None, + DirectoryOwnership::UnownedViaBlock => None, }; let ModulePath { path_exists, name, result } = default_submod_path(&sess.parse_sess, id, span, relative, dir_path); @@ -160,10 +156,6 @@ fn submod_path<'a>( let _ = result.map_err(|mut err| err.cancel()); error_decl_mod_in_block(&sess.parse_sess, span, path_exists, &name) } - DirectoryOwnership::UnownedViaMod => { - let _ = result.map_err(|mut err| err.cancel()); - error_cannot_declare_mod_here(&sess.parse_sess, span, path_exists, &name) - } } } @@ -182,41 +174,6 @@ fn error_decl_mod_in_block<'a, T>( Err(err) } -fn error_cannot_declare_mod_here<'a, T>( - sess: &'a ParseSess, - span: Span, - path_exists: bool, - name: &str, -) -> PResult<'a, T> { - let mut err = - sess.span_diagnostic.struct_span_err(span, "cannot declare a new module at this location"); - if !span.is_dummy() { - if let FileName::Real(src_name) = sess.source_map().span_to_filename(span) { - let src_path = src_name.into_local_path(); - if let Some(stem) = src_path.file_stem() { - let mut dest_path = src_path.clone(); - dest_path.set_file_name(stem); - dest_path.push("mod.rs"); - err.span_note( - span, - &format!( - "maybe move this module `{}` to its own directory via `{}`", - src_path.display(), - dest_path.display() - ), - ); - } - } - } - if path_exists { - err.span_note( - span, - &format!("... or maybe `use` the module `{}` instead of possibly redeclaring it", name), - ); - } - Err(err) -} - /// Derive a submodule path from the first found `#[path = "path_string"]`. /// The provided `dir_path` is joined with the `path_string`. pub(super) fn submod_path_from_attr( diff --git a/src/test/ui/modules/path-no-file-name.rs b/src/test/ui/modules/path-no-file-name.rs new file mode 100644 index 000000000000..f62cd2a9eb4e --- /dev/null +++ b/src/test/ui/modules/path-no-file-name.rs @@ -0,0 +1,7 @@ +// normalize-stderr-test: "\.:.*\(" -> ".: $$ACCESS_DENIED_MSG (" +// normalize-stderr-test: "os error \d+" -> "os error $$ACCESS_DENIED_CODE" + +#[path = "."] +mod m; //~ ERROR couldn't read + +fn main() {} diff --git a/src/test/ui/modules/path-no-file-name.stderr b/src/test/ui/modules/path-no-file-name.stderr new file mode 100644 index 000000000000..32a213c68f65 --- /dev/null +++ b/src/test/ui/modules/path-no-file-name.stderr @@ -0,0 +1,8 @@ +error: couldn't read $DIR/.: $ACCESS_DENIED_MSG (os error $ACCESS_DENIED_CODE) + --> $DIR/path-no-file-name.rs:5:1 + | +LL | mod m; + | ^^^^^^ + +error: aborting due to previous error + From 39052c55bb0259930dd9e6b0fc27a715793b07c8 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 21 Feb 2021 19:15:43 +0300 Subject: [PATCH 13/25] expand: Move module file path stack from global session to expansion data Also don't push the paths on the stack directly in `fn parse_external_mod`, return them instead. --- .../rustc_builtin_macros/src/source_util.rs | 7 +- compiler/rustc_expand/src/base.rs | 22 +++++- compiler/rustc_expand/src/expand.rs | 71 ++++++++++--------- compiler/rustc_expand/src/module.rs | 22 +++--- compiler/rustc_session/src/parse.rs | 4 -- src/test/ui/parser/circular_modules_main.rs | 6 +- .../ui/parser/circular_modules_main.stderr | 18 ++--- 7 files changed, 80 insertions(+), 70 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/source_util.rs b/compiler/rustc_builtin_macros/src/source_util.rs index 28efd483c867..98659f15a7e2 100644 --- a/compiler/rustc_builtin_macros/src/source_util.rs +++ b/compiler/rustc_builtin_macros/src/source_util.rs @@ -101,7 +101,7 @@ pub fn expand_include<'cx>( None => return DummyResult::any(sp), }; // The file will be added to the code map by the parser - let mut file = match cx.resolve_path(file, sp) { + let file = match cx.resolve_path(file, sp) { Ok(f) => f, Err(mut err) => { err.emit(); @@ -114,10 +114,9 @@ pub fn expand_include<'cx>( // then the path of `bar.rs` should be relative to the directory of `file`. // See https://github.com/rust-lang/rust/pull/69838/files#r395217057 for a discussion. // `MacroExpander::fully_expand_fragment` later restores, so "stack discipline" is maintained. - file.pop(); + let dir_path = file.parent().unwrap_or(&file).to_owned(); + cx.current_expansion.module = Rc::new(cx.current_expansion.module.with_dir_path(dir_path)); cx.current_expansion.directory_ownership = DirectoryOwnership::Owned { relative: None }; - let mod_path = cx.current_expansion.module.mod_path.clone(); - cx.current_expansion.module = Rc::new(ModuleData { mod_path, directory: file }); struct ExpandResult<'a> { p: Parser<'a>, diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index ce8103c0f850..a0632522ca0e 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -894,10 +894,26 @@ pub trait ResolverExpand { fn cfg_accessible(&mut self, expn_id: ExpnId, path: &ast::Path) -> Result; } -#[derive(Clone)] +#[derive(Clone, Default)] pub struct ModuleData { + /// Path to the module starting from the crate name, like `my_crate::foo::bar`. pub mod_path: Vec, - pub directory: PathBuf, + /// Stack of paths to files loaded by out-of-line module items, + /// used to detect and report recursive module inclusions. + pub file_path_stack: Vec, + /// Directory to search child module files in, + /// often (but not necessarily) the parent of the top file path on the `file_path_stack`. + pub dir_path: PathBuf, +} + +impl ModuleData { + pub fn with_dir_path(&self, dir_path: PathBuf) -> ModuleData { + ModuleData { + mod_path: self.mod_path.clone(), + file_path_stack: self.file_path_stack.clone(), + dir_path, + } + } } #[derive(Clone)] @@ -946,7 +962,7 @@ impl<'a> ExtCtxt<'a> { current_expansion: ExpansionData { id: ExpnId::root(), depth: 0, - module: Rc::new(ModuleData { mod_path: Vec::new(), directory: PathBuf::new() }), + module: Default::default(), directory_ownership: DirectoryOwnership::Owned { relative: None }, prior_type_ascription: None, }, diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index b474cad1242e..8d633afacf47 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -355,16 +355,17 @@ impl<'a, 'b> MacroExpander<'a, 'b> { // FIXME: Avoid visiting the crate as a `Mod` item, // make crate a first class expansion target instead. pub fn expand_crate(&mut self, mut krate: ast::Crate) -> ast::Crate { - let mut module = ModuleData { - mod_path: vec![Ident::from_str(&self.cx.ecfg.crate_name)], - directory: match self.cx.source_map().span_to_unmapped_path(krate.span) { - FileName::Real(name) => name.into_local_path(), - other => PathBuf::from(other.to_string()), - }, + let file_path = match self.cx.source_map().span_to_unmapped_path(krate.span) { + FileName::Real(name) => name.into_local_path(), + other => PathBuf::from(other.to_string()), }; - module.directory.pop(); - self.cx.root_path = module.directory.clone(); - self.cx.current_expansion.module = Rc::new(module); + let dir_path = file_path.parent().unwrap_or(&file_path).to_owned(); + self.cx.root_path = dir_path.clone(); + self.cx.current_expansion.module = Rc::new(ModuleData { + mod_path: vec![Ident::from_str(&self.cx.ecfg.crate_name)], + file_path_stack: vec![file_path], + dir_path, + }); let krate_item = AstFragment::Items(smallvec![P(ast::Item { attrs: krate.attrs, @@ -1276,25 +1277,30 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { }) } ast::ItemKind::Mod(_, ref mut mod_kind) if ident != Ident::invalid() => { - let sess = &self.cx.sess.parse_sess; - let orig_ownership = self.cx.current_expansion.directory_ownership; - let mut module = (*self.cx.current_expansion.module).clone(); - - let pushed = &mut false; // Record `parse_external_mod` pushing so we can pop. - let dir = Directory { ownership: orig_ownership, path: module.directory }; - let Directory { ownership, path } = match mod_kind { + let dir = Directory { + ownership: self.cx.current_expansion.directory_ownership, + path: self.cx.current_expansion.module.dir_path.clone(), + }; + let (file_path, Directory { ownership, path }) = match mod_kind { ModKind::Loaded(_, Inline::Yes, _) => { // Inline `mod foo { ... }`, but we still need to push directories. + let dir_path = push_directory(&self.cx.sess, ident, &attrs, dir); item.attrs = attrs; - push_directory(&self.cx.sess, ident, &item.attrs, dir) + (None, dir_path) } ModKind::Loaded(_, Inline::No, _) => { panic!("`mod` item is loaded from a file for the second time") } ModKind::Unloaded => { // We have an outline `mod foo;` so we need to parse the file. - let (items, inner_span, dir) = - parse_external_mod(&self.cx.sess, ident, span, dir, &mut attrs, pushed); + let (items, inner_span, file_path, dir_path) = parse_external_mod( + &self.cx.sess, + ident, + span, + &self.cx.current_expansion.module.file_path_stack, + dir, + &mut attrs, + ); let krate = ast::Crate { attrs, items, span: inner_span, proc_macros: vec![] }; @@ -1305,34 +1311,29 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { *mod_kind = ModKind::Loaded(krate.items, Inline::No, inner_span); item.attrs = krate.attrs; // File can have inline attributes, e.g., `#![cfg(...)]` & co. => Reconfigure. - item = match self.configure(item) { - Some(node) => node, - None => { - if *pushed { - sess.included_mod_stack.borrow_mut().pop(); - } - return Default::default(); - } - }; - dir + item = configure!(self, item); + (Some(file_path), dir_path) } }; // Set the module info before we flat map. - self.cx.current_expansion.directory_ownership = ownership; - module.directory = path; + let mut module = self.cx.current_expansion.module.with_dir_path(path); module.mod_path.push(ident); + if let Some(file_path) = file_path { + module.file_path_stack.push(file_path); + } + let orig_module = mem::replace(&mut self.cx.current_expansion.module, Rc::new(module)); + let orig_dir_ownership = + mem::replace(&mut self.cx.current_expansion.directory_ownership, ownership); let result = noop_flat_map_item(item, self); // Restore the module info. + self.cx.current_expansion.directory_ownership = orig_dir_ownership; self.cx.current_expansion.module = orig_module; - self.cx.current_expansion.directory_ownership = orig_ownership; - if *pushed { - sess.included_mod_stack.borrow_mut().pop(); - } + result } _ => { diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 99c1591d40a8..55b48e4f6b25 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -42,10 +42,10 @@ crate fn parse_external_mod( sess: &Session, id: Ident, span: Span, // The span to blame on errors. + file_path_stack: &[PathBuf], Directory { mut ownership, path }: Directory, attrs: &mut Vec, - pop_mod_stack: &mut bool, -) -> (Vec>, Span, Directory) { +) -> (Vec>, Span, PathBuf, Directory) { // We bail on the first error, but that error does not cause a fatal error... (1) let result: PResult<'_, _> = try { // Extract the file path and the new ownership. @@ -53,20 +53,16 @@ crate fn parse_external_mod( ownership = mp.ownership; // Ensure file paths are acyclic. - let mut included_mod_stack = sess.parse_sess.included_mod_stack.borrow_mut(); - error_on_circular_module(&sess.parse_sess, span, &mp.path, &included_mod_stack)?; - included_mod_stack.push(mp.path.clone()); - *pop_mod_stack = true; // We have pushed, so notify caller. - drop(included_mod_stack); + error_on_circular_module(&sess.parse_sess, span, &mp.path, file_path_stack)?; // Actually parse the external file as a module. let mut parser = new_parser_from_file(&sess.parse_sess, &mp.path, Some(span)); let (mut inner_attrs, items, inner_span) = parser.parse_mod(&token::Eof)?; attrs.append(&mut inner_attrs); - (items, inner_span) + (items, inner_span, mp.path) }; // (1) ...instead, we return a dummy module. - let (items, inner_span) = result.map_err(|mut err| err.emit()).unwrap_or_default(); + let (items, inner_span, file_path) = result.map_err(|mut err| err.emit()).unwrap_or_default(); // Extract the directory path for submodules of the module. let path = sess.source_map().span_to_unmapped_path(inner_span); @@ -76,18 +72,18 @@ crate fn parse_external_mod( }; path.pop(); - (items, inner_span, Directory { ownership, path }) + (items, inner_span, file_path, Directory { ownership, path }) } fn error_on_circular_module<'a>( sess: &'a ParseSess, span: Span, path: &Path, - included_mod_stack: &[PathBuf], + file_path_stack: &[PathBuf], ) -> PResult<'a, ()> { - if let Some(i) = included_mod_stack.iter().position(|p| *p == path) { + if let Some(i) = file_path_stack.iter().position(|p| *p == path) { let mut err = String::from("circular modules: "); - for p in &included_mod_stack[i..] { + for p in &file_path_stack[i..] { err.push_str(&p.to_string_lossy()); err.push_str(" -> "); } diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index 81b38347414e..592773bfe1b4 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -13,7 +13,6 @@ use rustc_span::hygiene::ExpnId; use rustc_span::source_map::{FilePathMapping, SourceMap}; use rustc_span::{MultiSpan, Span, Symbol}; -use std::path::PathBuf; use std::str; /// The set of keys (and, optionally, values) that define the compilation @@ -122,8 +121,6 @@ pub struct ParseSess { pub missing_fragment_specifiers: Lock>, /// Places where raw identifiers were used. This is used for feature-gating raw identifiers. pub raw_identifier_spans: Lock>, - /// Used to determine and report recursive module inclusions. - pub included_mod_stack: Lock>, source_map: Lrc, pub buffered_lints: Lock>, /// Contains the spans of block expressions that could have been incomplete based on the @@ -157,7 +154,6 @@ impl ParseSess { edition: ExpnId::root().expn_data().edition, missing_fragment_specifiers: Default::default(), raw_identifier_spans: Lock::new(Vec::new()), - included_mod_stack: Lock::new(vec![]), source_map, buffered_lints: Lock::new(vec![]), ambiguous_block_expr_parse: Lock::new(FxHashMap::default()), diff --git a/src/test/ui/parser/circular_modules_main.rs b/src/test/ui/parser/circular_modules_main.rs index 1ae36a1f7605..d4b47efe6815 100644 --- a/src/test/ui/parser/circular_modules_main.rs +++ b/src/test/ui/parser/circular_modules_main.rs @@ -1,10 +1,12 @@ +// error-pattern: circular modules + #[path = "circular_modules_hello.rs"] -mod circular_modules_hello; //~ ERROR: circular modules +mod circular_modules_hello; pub fn hi_str() -> String { "Hi!".to_string() } fn main() { - circular_modules_hello::say_hello(); //~ ERROR cannot find function `say_hello` in module + circular_modules_hello::say_hello(); } diff --git a/src/test/ui/parser/circular_modules_main.stderr b/src/test/ui/parser/circular_modules_main.stderr index 5d4db8c31a2c..ee45f65a3bd5 100644 --- a/src/test/ui/parser/circular_modules_main.stderr +++ b/src/test/ui/parser/circular_modules_main.stderr @@ -1,18 +1,18 @@ -error: circular modules: $DIR/circular_modules_hello.rs -> $DIR/circular_modules_main.rs -> $DIR/circular_modules_hello.rs - --> $DIR/circular_modules_main.rs:2:1 +error: circular modules: $DIR/circular_modules_main.rs -> $DIR/circular_modules_hello.rs -> $DIR/circular_modules_main.rs + --> $DIR/circular_modules_hello.rs:4:1 | -LL | mod circular_modules_hello; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | mod circular_modules_main; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error[E0425]: cannot find function `say_hello` in module `circular_modules_hello` - --> $DIR/circular_modules_main.rs:9:29 +error[E0425]: cannot find function `hi_str` in module `circular_modules_main` + --> $DIR/circular_modules_hello.rs:7:43 | -LL | circular_modules_hello::say_hello(); - | ^^^^^^^^^ not found in `circular_modules_hello` +LL | println!("{}", circular_modules_main::hi_str()); + | ^^^^^^ not found in `circular_modules_main` | help: consider importing this function | -LL | use circular_modules_hello::say_hello; +LL | use hi_str; | error: aborting due to 2 previous errors From 5bdf81d5fa7ada2274617ebfa97e6bd157ae5a52 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 18:42:57 +0300 Subject: [PATCH 14/25] expand: Determine module directory path directly instead of relying on span --- compiler/rustc_expand/src/module.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 55b48e4f6b25..e744ab27e8f5 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -4,8 +4,8 @@ use rustc_errors::{struct_span_err, PResult}; use rustc_parse::new_parser_from_file; use rustc_session::parse::ParseSess; use rustc_session::Session; -use rustc_span::source_map::{FileName, Span}; use rustc_span::symbol::{sym, Ident}; +use rustc_span::Span; use std::path::{self, Path, PathBuf}; @@ -64,13 +64,8 @@ crate fn parse_external_mod( // (1) ...instead, we return a dummy module. let (items, inner_span, file_path) = result.map_err(|mut err| err.emit()).unwrap_or_default(); - // Extract the directory path for submodules of the module. - let path = sess.source_map().span_to_unmapped_path(inner_span); - let mut path = match path { - FileName::Real(name) => name.into_local_path(), - other => PathBuf::from(other.to_string()), - }; - path.pop(); + // Extract the directory path for submodules of the module. + let path = file_path.parent().unwrap_or(&file_path).to_owned(); (items, inner_span, file_path, Directory { ownership, path }) } From 3d0b622ab7ae3f803d757f73f2a9a7c857d771bb Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 18:58:45 +0300 Subject: [PATCH 15/25] expand: Less path cloning during module loading --- compiler/rustc_expand/src/expand.rs | 36 ++++++++++++-------- compiler/rustc_expand/src/module.rs | 52 ++++++++++++++++------------- 2 files changed, 51 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 8d633afacf47..a81bc381c508 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -3,7 +3,7 @@ use crate::config::StripUnconfigured; use crate::configure; use crate::hygiene::SyntaxContext; use crate::mbe::macro_rules::annotate_err_with_kind; -use crate::module::{parse_external_mod, push_directory, Directory, DirectoryOwnership}; +use crate::module::{parse_external_mod, push_directory, DirectoryOwnership, ParsedExternalMod}; use crate::placeholders::{placeholder, PlaceholderExpander}; use rustc_ast as ast; @@ -1277,28 +1277,36 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { }) } ast::ItemKind::Mod(_, ref mut mod_kind) if ident != Ident::invalid() => { - let dir = Directory { - ownership: self.cx.current_expansion.directory_ownership, - path: self.cx.current_expansion.module.dir_path.clone(), - }; - let (file_path, Directory { ownership, path }) = match mod_kind { + let (file_path, dir_path, dir_ownership) = match mod_kind { ModKind::Loaded(_, Inline::Yes, _) => { // Inline `mod foo { ... }`, but we still need to push directories. - let dir_path = push_directory(&self.cx.sess, ident, &attrs, dir); + let (dir_path, dir_ownership) = push_directory( + &self.cx.sess, + ident, + &attrs, + &self.cx.current_expansion.module, + self.cx.current_expansion.directory_ownership, + ); item.attrs = attrs; - (None, dir_path) + (None, dir_path, dir_ownership) } ModKind::Loaded(_, Inline::No, _) => { panic!("`mod` item is loaded from a file for the second time") } ModKind::Unloaded => { // We have an outline `mod foo;` so we need to parse the file. - let (items, inner_span, file_path, dir_path) = parse_external_mod( + let ParsedExternalMod { + items, + inner_span, + file_path, + dir_path, + dir_ownership, + } = parse_external_mod( &self.cx.sess, ident, span, - &self.cx.current_expansion.module.file_path_stack, - dir, + &self.cx.current_expansion.module, + self.cx.current_expansion.directory_ownership, &mut attrs, ); @@ -1312,12 +1320,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { item.attrs = krate.attrs; // File can have inline attributes, e.g., `#![cfg(...)]` & co. => Reconfigure. item = configure!(self, item); - (Some(file_path), dir_path) + (Some(file_path), dir_path, dir_ownership) } }; // Set the module info before we flat map. - let mut module = self.cx.current_expansion.module.with_dir_path(path); + let mut module = self.cx.current_expansion.module.with_dir_path(dir_path); module.mod_path.push(ident); if let Some(file_path) = file_path { module.file_path_stack.push(file_path); @@ -1326,7 +1334,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { let orig_module = mem::replace(&mut self.cx.current_expansion.module, Rc::new(module)); let orig_dir_ownership = - mem::replace(&mut self.cx.current_expansion.directory_ownership, ownership); + mem::replace(&mut self.cx.current_expansion.directory_ownership, dir_ownership); let result = noop_flat_map_item(item, self); diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index e744ab27e8f5..4ba24ace8eb8 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -1,3 +1,4 @@ +use crate::base::ModuleData; use rustc_ast::ptr::P; use rustc_ast::{token, Attribute, Item}; use rustc_errors::{struct_span_err, PResult}; @@ -9,12 +10,6 @@ use rustc_span::Span; use std::path::{self, Path, PathBuf}; -#[derive(Clone)] -pub struct Directory { - pub path: PathBuf, - pub ownership: DirectoryOwnership, -} - #[derive(Copy, Clone)] pub enum DirectoryOwnership { Owned { @@ -38,22 +33,30 @@ pub struct ModulePathSuccess { pub ownership: DirectoryOwnership, } +crate struct ParsedExternalMod { + pub items: Vec>, + pub inner_span: Span, + pub file_path: PathBuf, + pub dir_path: PathBuf, + pub dir_ownership: DirectoryOwnership, +} + crate fn parse_external_mod( sess: &Session, id: Ident, span: Span, // The span to blame on errors. - file_path_stack: &[PathBuf], - Directory { mut ownership, path }: Directory, + module: &ModuleData, + mut dir_ownership: DirectoryOwnership, attrs: &mut Vec, -) -> (Vec>, Span, PathBuf, Directory) { +) -> ParsedExternalMod { // We bail on the first error, but that error does not cause a fatal error... (1) let result: PResult<'_, _> = try { // Extract the file path and the new ownership. - let mp = submod_path(sess, id, span, &attrs, ownership, &path)?; - ownership = mp.ownership; + let mp = submod_path(sess, id, span, &attrs, dir_ownership, &module.dir_path)?; + dir_ownership = mp.ownership; // Ensure file paths are acyclic. - error_on_circular_module(&sess.parse_sess, span, &mp.path, file_path_stack)?; + error_on_circular_module(&sess.parse_sess, span, &mp.path, &module.file_path_stack)?; // Actually parse the external file as a module. let mut parser = new_parser_from_file(&sess.parse_sess, &mp.path, Some(span)); @@ -65,9 +68,9 @@ crate fn parse_external_mod( let (items, inner_span, file_path) = result.map_err(|mut err| err.emit()).unwrap_or_default(); // Extract the directory path for submodules of the module. - let path = file_path.parent().unwrap_or(&file_path).to_owned(); + let dir_path = file_path.parent().unwrap_or(&file_path).to_owned(); - (items, inner_span, file_path, Directory { ownership, path }) + ParsedExternalMod { items, inner_span, file_path, dir_path, dir_ownership } } fn error_on_circular_module<'a>( @@ -92,11 +95,13 @@ crate fn push_directory( sess: &Session, id: Ident, attrs: &[Attribute], - Directory { mut ownership, mut path }: Directory, -) -> Directory { - if let Some(filename) = sess.first_attr_value_str_by_name(attrs, sym::path) { - path.push(&*filename.as_str()); - ownership = DirectoryOwnership::Owned { relative: None }; + module: &ModuleData, + mut dir_ownership: DirectoryOwnership, +) -> (PathBuf, DirectoryOwnership) { + let mut dir_path = module.dir_path.clone(); + if let Some(file_path) = sess.first_attr_value_str_by_name(attrs, sym::path) { + dir_path.push(&*file_path.as_str()); + dir_ownership = DirectoryOwnership::Owned { relative: None }; } else { // We have to push on the current module name in the case of relative // paths in order to ensure that any additional module paths from inline @@ -104,15 +109,16 @@ crate fn push_directory( // // For example, a `mod z { ... }` inside `x/y.rs` should set the current // directory path to `/x/y/z`, not `/x/z` with a relative offset of `y`. - if let DirectoryOwnership::Owned { relative } = &mut ownership { + if let DirectoryOwnership::Owned { relative } = &mut dir_ownership { if let Some(ident) = relative.take() { // Remove the relative offset. - path.push(&*ident.as_str()); + dir_path.push(&*ident.as_str()); } } - path.push(&*id.as_str()); + dir_path.push(&*id.as_str()); } - Directory { ownership, path } + + (dir_path, dir_ownership) } fn submod_path<'a>( From 46b67aa74d8ee7d9c41983e15f8cd0f17ee27ae7 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 19:06:36 +0300 Subject: [PATCH 16/25] expand: Some more consistent naming in module loading --- .../rustc_builtin_macros/src/source_util.rs | 4 +- compiler/rustc_expand/src/base.rs | 6 +- compiler/rustc_expand/src/expand.rs | 20 +++-- compiler/rustc_expand/src/module.rs | 86 +++++++++---------- 4 files changed, 59 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/source_util.rs b/compiler/rustc_builtin_macros/src/source_util.rs index 98659f15a7e2..4aafcb2fb6df 100644 --- a/compiler/rustc_builtin_macros/src/source_util.rs +++ b/compiler/rustc_builtin_macros/src/source_util.rs @@ -4,7 +4,7 @@ use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_ast_pretty::pprust; use rustc_expand::base::{self, *}; -use rustc_expand::module::DirectoryOwnership; +use rustc_expand::module::DirOwnership; use rustc_parse::parser::{ForceCollect, Parser}; use rustc_parse::{self, new_parser_from_file}; use rustc_session::lint::builtin::INCOMPLETE_INCLUDE; @@ -116,7 +116,7 @@ pub fn expand_include<'cx>( // `MacroExpander::fully_expand_fragment` later restores, so "stack discipline" is maintained. let dir_path = file.parent().unwrap_or(&file).to_owned(); cx.current_expansion.module = Rc::new(cx.current_expansion.module.with_dir_path(dir_path)); - cx.current_expansion.directory_ownership = DirectoryOwnership::Owned { relative: None }; + cx.current_expansion.dir_ownership = DirOwnership::Owned { relative: None }; struct ExpandResult<'a> { p: Parser<'a>, diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index a0632522ca0e..f88110cf106a 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -1,5 +1,5 @@ use crate::expand::{self, AstFragment, Invocation}; -use crate::module::DirectoryOwnership; +use crate::module::DirOwnership; use rustc_ast::ptr::P; use rustc_ast::token::{self, Nonterminal}; @@ -921,7 +921,7 @@ pub struct ExpansionData { pub id: ExpnId, pub depth: usize, pub module: Rc, - pub directory_ownership: DirectoryOwnership, + pub dir_ownership: DirOwnership, pub prior_type_ascription: Option<(Span, bool)>, } @@ -963,7 +963,7 @@ impl<'a> ExtCtxt<'a> { id: ExpnId::root(), depth: 0, module: Default::default(), - directory_ownership: DirectoryOwnership::Owned { relative: None }, + dir_ownership: DirOwnership::Owned { relative: None }, prior_type_ascription: None, }, force_mode: false, diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index a81bc381c508..eee2c6ff8086 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -3,7 +3,7 @@ use crate::config::StripUnconfigured; use crate::configure; use crate::hygiene::SyntaxContext; use crate::mbe::macro_rules::annotate_err_with_kind; -use crate::module::{parse_external_mod, push_directory, DirectoryOwnership, ParsedExternalMod}; +use crate::module::{mod_dir_path, parse_external_mod, DirOwnership, ParsedExternalMod}; use crate::placeholders::{placeholder, PlaceholderExpander}; use rustc_ast as ast; @@ -1246,10 +1246,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { } fn visit_block(&mut self, block: &mut P) { - let old_directory_ownership = self.cx.current_expansion.directory_ownership; - self.cx.current_expansion.directory_ownership = DirectoryOwnership::UnownedViaBlock; + let orig_dir_ownership = mem::replace( + &mut self.cx.current_expansion.dir_ownership, + DirOwnership::UnownedViaBlock, + ); noop_visit_block(block, self); - self.cx.current_expansion.directory_ownership = old_directory_ownership; + self.cx.current_expansion.dir_ownership = orig_dir_ownership; } fn flat_map_item(&mut self, item: P) -> SmallVec<[P; 1]> { @@ -1280,12 +1282,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { let (file_path, dir_path, dir_ownership) = match mod_kind { ModKind::Loaded(_, Inline::Yes, _) => { // Inline `mod foo { ... }`, but we still need to push directories. - let (dir_path, dir_ownership) = push_directory( + let (dir_path, dir_ownership) = mod_dir_path( &self.cx.sess, ident, &attrs, &self.cx.current_expansion.module, - self.cx.current_expansion.directory_ownership, + self.cx.current_expansion.dir_ownership, ); item.attrs = attrs; (None, dir_path, dir_ownership) @@ -1306,7 +1308,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { ident, span, &self.cx.current_expansion.module, - self.cx.current_expansion.directory_ownership, + self.cx.current_expansion.dir_ownership, &mut attrs, ); @@ -1334,12 +1336,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { let orig_module = mem::replace(&mut self.cx.current_expansion.module, Rc::new(module)); let orig_dir_ownership = - mem::replace(&mut self.cx.current_expansion.directory_ownership, dir_ownership); + mem::replace(&mut self.cx.current_expansion.dir_ownership, dir_ownership); let result = noop_flat_map_item(item, self); // Restore the module info. - self.cx.current_expansion.directory_ownership = orig_dir_ownership; + self.cx.current_expansion.dir_ownership = orig_dir_ownership; self.cx.current_expansion.module = orig_module; result diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 4ba24ace8eb8..607c68f82dfa 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -11,7 +11,7 @@ use rustc_span::Span; use std::path::{self, Path, PathBuf}; #[derive(Copy, Clone)] -pub enum DirectoryOwnership { +pub enum DirOwnership { Owned { // None if `mod.rs`, `Some("foo")` if we're in `foo.rs`. relative: Option, @@ -29,8 +29,8 @@ pub struct ModulePath<'a> { // Public for rustfmt usage. pub struct ModulePathSuccess { - pub path: PathBuf, - pub ownership: DirectoryOwnership, + pub file_path: PathBuf, + pub dir_ownership: DirOwnership, } crate struct ParsedExternalMod { @@ -38,31 +38,31 @@ crate struct ParsedExternalMod { pub inner_span: Span, pub file_path: PathBuf, pub dir_path: PathBuf, - pub dir_ownership: DirectoryOwnership, + pub dir_ownership: DirOwnership, } crate fn parse_external_mod( sess: &Session, - id: Ident, + ident: Ident, span: Span, // The span to blame on errors. module: &ModuleData, - mut dir_ownership: DirectoryOwnership, + mut dir_ownership: DirOwnership, attrs: &mut Vec, ) -> ParsedExternalMod { // We bail on the first error, but that error does not cause a fatal error... (1) let result: PResult<'_, _> = try { // Extract the file path and the new ownership. - let mp = submod_path(sess, id, span, &attrs, dir_ownership, &module.dir_path)?; - dir_ownership = mp.ownership; + let mp = mod_file_path(sess, ident, span, &attrs, &module.dir_path, dir_ownership)?; + dir_ownership = mp.dir_ownership; // Ensure file paths are acyclic. - error_on_circular_module(&sess.parse_sess, span, &mp.path, &module.file_path_stack)?; + error_on_circular_module(&sess.parse_sess, span, &mp.file_path, &module.file_path_stack)?; // Actually parse the external file as a module. - let mut parser = new_parser_from_file(&sess.parse_sess, &mp.path, Some(span)); + let mut parser = new_parser_from_file(&sess.parse_sess, &mp.file_path, Some(span)); let (mut inner_attrs, items, inner_span) = parser.parse_mod(&token::Eof)?; attrs.append(&mut inner_attrs); - (items, inner_span, mp.path) + (items, inner_span, mp.file_path) }; // (1) ...instead, we return a dummy module. let (items, inner_span, file_path) = result.map_err(|mut err| err.emit()).unwrap_or_default(); @@ -76,32 +76,32 @@ crate fn parse_external_mod( fn error_on_circular_module<'a>( sess: &'a ParseSess, span: Span, - path: &Path, + file_path: &Path, file_path_stack: &[PathBuf], ) -> PResult<'a, ()> { - if let Some(i) = file_path_stack.iter().position(|p| *p == path) { + if let Some(i) = file_path_stack.iter().position(|p| *p == file_path) { let mut err = String::from("circular modules: "); for p in &file_path_stack[i..] { err.push_str(&p.to_string_lossy()); err.push_str(" -> "); } - err.push_str(&path.to_string_lossy()); + err.push_str(&file_path.to_string_lossy()); return Err(sess.span_diagnostic.struct_span_err(span, &err[..])); } Ok(()) } -crate fn push_directory( +crate fn mod_dir_path( sess: &Session, - id: Ident, + ident: Ident, attrs: &[Attribute], module: &ModuleData, - mut dir_ownership: DirectoryOwnership, -) -> (PathBuf, DirectoryOwnership) { + mut dir_ownership: DirOwnership, +) -> (PathBuf, DirOwnership) { let mut dir_path = module.dir_path.clone(); if let Some(file_path) = sess.first_attr_value_str_by_name(attrs, sym::path) { dir_path.push(&*file_path.as_str()); - dir_ownership = DirectoryOwnership::Owned { relative: None }; + dir_ownership = DirOwnership::Owned { relative: None }; } else { // We have to push on the current module name in the case of relative // paths in order to ensure that any additional module paths from inline @@ -109,27 +109,27 @@ crate fn push_directory( // // For example, a `mod z { ... }` inside `x/y.rs` should set the current // directory path to `/x/y/z`, not `/x/z` with a relative offset of `y`. - if let DirectoryOwnership::Owned { relative } = &mut dir_ownership { + if let DirOwnership::Owned { relative } = &mut dir_ownership { if let Some(ident) = relative.take() { // Remove the relative offset. dir_path.push(&*ident.as_str()); } } - dir_path.push(&*id.as_str()); + dir_path.push(&*ident.as_str()); } (dir_path, dir_ownership) } -fn submod_path<'a>( +fn mod_file_path<'a>( sess: &'a Session, - id: Ident, + ident: Ident, span: Span, attrs: &[Attribute], - ownership: DirectoryOwnership, dir_path: &Path, + dir_ownership: DirOwnership, ) -> PResult<'a, ModulePathSuccess> { - if let Some(path) = submod_path_from_attr(sess, attrs, dir_path) { + if let Some(file_path) = mod_file_path_from_attr(sess, attrs, dir_path) { // All `#[path]` files are treated as though they are a `mod.rs` file. // This means that `mod foo;` declarations inside `#[path]`-included // files are siblings, @@ -137,19 +137,19 @@ fn submod_path<'a>( // Note that this will produce weirdness when a file named `foo.rs` is // `#[path]` included and contains a `mod foo;` declaration. // If you encounter this, it's your own darn fault :P - let ownership = DirectoryOwnership::Owned { relative: None }; - return Ok(ModulePathSuccess { ownership, path }); + let dir_ownership = DirOwnership::Owned { relative: None }; + return Ok(ModulePathSuccess { file_path, dir_ownership }); } - let relative = match ownership { - DirectoryOwnership::Owned { relative } => relative, - DirectoryOwnership::UnownedViaBlock => None, + let relative = match dir_ownership { + DirOwnership::Owned { relative } => relative, + DirOwnership::UnownedViaBlock => None, }; let ModulePath { path_exists, name, result } = - default_submod_path(&sess.parse_sess, id, span, relative, dir_path); - match ownership { - DirectoryOwnership::Owned { .. } => Ok(result?), - DirectoryOwnership::UnownedViaBlock => { + default_submod_path(&sess.parse_sess, ident, span, relative, dir_path); + match dir_ownership { + DirOwnership::Owned { .. } => Ok(result?), + DirOwnership::UnownedViaBlock => { let _ = result.map_err(|mut err| err.cancel()); error_decl_mod_in_block(&sess.parse_sess, span, path_exists, &name) } @@ -173,7 +173,7 @@ fn error_decl_mod_in_block<'a, T>( /// Derive a submodule path from the first found `#[path = "path_string"]`. /// The provided `dir_path` is joined with the `path_string`. -pub(super) fn submod_path_from_attr( +fn mod_file_path_from_attr( sess: &Session, attrs: &[Attribute], dir_path: &Path, @@ -196,15 +196,15 @@ pub(super) fn submod_path_from_attr( // Public for rustfmt usage. pub fn default_submod_path<'a>( sess: &'a ParseSess, - id: Ident, + ident: Ident, span: Span, relative: Option, dir_path: &Path, ) -> ModulePath<'a> { // If we're in a foo.rs file instead of a mod.rs file, // we need to look for submodules in - // `./foo/.rs` and `./foo//mod.rs` rather than - // `./.rs` and `.//mod.rs`. + // `./foo/.rs` and `./foo//mod.rs` rather than + // `./.rs` and `.//mod.rs`. let relative_prefix_string; let relative_prefix = if let Some(ident) = relative { relative_prefix_string = format!("{}{}", ident.name, path::MAIN_SEPARATOR); @@ -213,7 +213,7 @@ pub fn default_submod_path<'a>( "" }; - let mod_name = id.name.to_string(); + let mod_name = ident.name.to_string(); let default_path_str = format!("{}{}.rs", relative_prefix, mod_name); let secondary_path_str = format!("{}{}{}mod.rs", relative_prefix, mod_name, path::MAIN_SEPARATOR); @@ -224,12 +224,12 @@ pub fn default_submod_path<'a>( let result = match (default_exists, secondary_exists) { (true, false) => Ok(ModulePathSuccess { - path: default_path, - ownership: DirectoryOwnership::Owned { relative: Some(id) }, + file_path: default_path, + dir_ownership: DirOwnership::Owned { relative: Some(ident) }, }), (false, true) => Ok(ModulePathSuccess { - path: secondary_path, - ownership: DirectoryOwnership::Owned { relative: None }, + file_path: secondary_path, + dir_ownership: DirOwnership::Owned { relative: None }, }), (false, false) => { let mut err = struct_span_err!( From da3419e18c68f4eb8beb3765fc9fde4c2e689d43 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 19:49:09 +0300 Subject: [PATCH 17/25] rustc_interface: Hide some hacky details of early linting from expand --- compiler/rustc_expand/src/base.rs | 9 ++++++--- compiler/rustc_expand/src/expand.rs | 10 ++++------ compiler/rustc_expand/src/lib.rs | 1 + compiler/rustc_interface/src/passes.rs | 6 ++++-- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index f88110cf106a..05c01deef5e1 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -5,7 +5,7 @@ use rustc_ast::ptr::P; use rustc_ast::token::{self, Nonterminal}; use rustc_ast::tokenstream::{CanSynthesizeMissingTokens, LazyTokenStream, TokenStream}; use rustc_ast::visit::{AssocCtxt, Visitor}; -use rustc_ast::{self as ast, AstLike, Attribute, NodeId, PatKind}; +use rustc_ast::{self as ast, AstLike, Attribute, Item, NodeId, PatKind}; use rustc_attr::{self as attr, Deprecation, Stability}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::{self, Lrc}; @@ -925,6 +925,9 @@ pub struct ExpansionData { pub prior_type_ascription: Option<(Span, bool)>, } +type OnExternModLoaded<'a> = + Option<&'a dyn Fn(Ident, Vec, Vec>, Span) -> (Vec, Vec>)>; + /// One of these is made during expansion and incrementally updated as we go; /// when a macro expansion occurs, the resulting nodes have the `backtrace() /// -> expn_data` of their expansion context stored into their span. @@ -942,7 +945,7 @@ pub struct ExtCtxt<'a> { /// Called directly after having parsed an external `mod foo;` in expansion. /// /// `Ident` is the module name. - pub(super) extern_mod_loaded: Option<&'a dyn Fn(&ast::Crate, Ident)>, + pub(super) extern_mod_loaded: OnExternModLoaded<'a>, } impl<'a> ExtCtxt<'a> { @@ -950,7 +953,7 @@ impl<'a> ExtCtxt<'a> { sess: &'a Session, ecfg: expand::ExpansionConfig<'a>, resolver: &'a mut dyn ResolverExpand, - extern_mod_loaded: Option<&'a dyn Fn(&ast::Crate, Ident)>, + extern_mod_loaded: OnExternModLoaded<'a>, ) -> ExtCtxt<'a> { ExtCtxt { sess, diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index eee2c6ff8086..a42a60fe9a52 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1298,7 +1298,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { ModKind::Unloaded => { // We have an outline `mod foo;` so we need to parse the file. let ParsedExternalMod { - items, + mut items, inner_span, file_path, dir_path, @@ -1312,14 +1312,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { &mut attrs, ); - let krate = - ast::Crate { attrs, items, span: inner_span, proc_macros: vec![] }; if let Some(extern_mod_loaded) = self.cx.extern_mod_loaded { - extern_mod_loaded(&krate, ident); + (attrs, items) = extern_mod_loaded(ident, attrs, items, inner_span); } - *mod_kind = ModKind::Loaded(krate.items, Inline::No, inner_span); - item.attrs = krate.attrs; + *mod_kind = ModKind::Loaded(items, Inline::No, inner_span); + item.attrs = attrs; // File can have inline attributes, e.g., `#![cfg(...)]` & co. => Reconfigure. item = configure!(self, item); (Some(file_path), dir_path, dir_ownership) diff --git a/compiler/rustc_expand/src/lib.rs b/compiler/rustc_expand/src/lib.rs index c5d8ff25ea94..bcccb04c9efd 100644 --- a/compiler/rustc_expand/src/lib.rs +++ b/compiler/rustc_expand/src/lib.rs @@ -1,5 +1,6 @@ #![feature(crate_visibility_modifier)] #![feature(decl_macro)] +#![feature(destructuring_assignment)] #![feature(or_patterns)] #![feature(proc_macro_diagnostic)] #![feature(proc_macro_internals)] diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 5217066bbefd..8328c07fbb73 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -302,8 +302,10 @@ fn configure_and_expand_inner<'a>( ..rustc_expand::expand::ExpansionConfig::default(crate_name.to_string()) }; - let extern_mod_loaded = |k: &ast::Crate, ident: Ident| { - pre_expansion_lint(sess, lint_store, k, &*ident.name.as_str()) + let extern_mod_loaded = |ident: Ident, attrs, items, span| { + let krate = ast::Crate { attrs, items, span, proc_macros: vec![] }; + pre_expansion_lint(sess, lint_store, &krate, &ident.name.as_str()); + (krate.attrs, krate.items) }; let mut ecx = ExtCtxt::new(&sess, cfg, &mut resolver, Some(&extern_mod_loaded)); From 29a9ef28188e2ebdd3c56177672cd437dd11e25d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 20:11:30 +0300 Subject: [PATCH 18/25] expand: Align some code with the PR fixing inner attributes on out-of-line modules --- compiler/rustc_expand/src/expand.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index a42a60fe9a52..dd2eaa0f3d53 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1280,8 +1280,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { } ast::ItemKind::Mod(_, ref mut mod_kind) if ident != Ident::invalid() => { let (file_path, dir_path, dir_ownership) = match mod_kind { - ModKind::Loaded(_, Inline::Yes, _) => { + ModKind::Loaded(_, inline, _) => { // Inline `mod foo { ... }`, but we still need to push directories. + assert!( + *inline == Inline::Yes, + "`mod` item is loaded from a file for the second time" + ); let (dir_path, dir_ownership) = mod_dir_path( &self.cx.sess, ident, @@ -1292,11 +1296,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { item.attrs = attrs; (None, dir_path, dir_ownership) } - ModKind::Loaded(_, Inline::No, _) => { - panic!("`mod` item is loaded from a file for the second time") - } ModKind::Unloaded => { // We have an outline `mod foo;` so we need to parse the file. + let old_attrs_len = attrs.len(); let ParsedExternalMod { mut items, inner_span, @@ -1318,8 +1320,13 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { *mod_kind = ModKind::Loaded(items, Inline::No, inner_span); item.attrs = attrs; - // File can have inline attributes, e.g., `#![cfg(...)]` & co. => Reconfigure. - item = configure!(self, item); + if item.attrs.len() > old_attrs_len { + // If we loaded an out-of-line module and added some inner attributes, + // then we need to re-configure it. + // FIXME: Attributes also need to be recollected + // for resolution and expansion. + item = configure!(self, item); + } (Some(file_path), dir_path, dir_ownership) } }; From 1e1d574aeac3fae44dac430ecfe8086abcdf9a5f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 20:56:49 +0300 Subject: [PATCH 19/25] expand: Share some code between inline and out-of-line module treatment --- compiler/rustc_expand/src/module.rs | 36 ++++++++++++++--------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 607c68f82dfa..8f5200804e0d 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -98,25 +98,26 @@ crate fn mod_dir_path( module: &ModuleData, mut dir_ownership: DirOwnership, ) -> (PathBuf, DirOwnership) { + if let Some(file_path) = mod_file_path_from_attr(sess, attrs, &module.dir_path) { + // For inline modules file path from `#[path]` is actually the directory path + // for historical reasons, so we don't pop the last segment here. + return (file_path, DirOwnership::Owned { relative: None }); + } + + // We have to push on the current module name in the case of relative + // paths in order to ensure that any additional module paths from inline + // `mod x { ... }` come after the relative extension. + // + // For example, a `mod z { ... }` inside `x/y.rs` should set the current + // directory path to `/x/y/z`, not `/x/z` with a relative offset of `y`. let mut dir_path = module.dir_path.clone(); - if let Some(file_path) = sess.first_attr_value_str_by_name(attrs, sym::path) { - dir_path.push(&*file_path.as_str()); - dir_ownership = DirOwnership::Owned { relative: None }; - } else { - // We have to push on the current module name in the case of relative - // paths in order to ensure that any additional module paths from inline - // `mod x { ... }` come after the relative extension. - // - // For example, a `mod z { ... }` inside `x/y.rs` should set the current - // directory path to `/x/y/z`, not `/x/z` with a relative offset of `y`. - if let DirOwnership::Owned { relative } = &mut dir_ownership { - if let Some(ident) = relative.take() { - // Remove the relative offset. - dir_path.push(&*ident.as_str()); - } + if let DirOwnership::Owned { relative } = &mut dir_ownership { + if let Some(ident) = relative.take() { + // Remove the relative offset. + dir_path.push(&*ident.as_str()); } - dir_path.push(&*ident.as_str()); } + dir_path.push(&*ident.as_str()); (dir_path, dir_ownership) } @@ -179,8 +180,7 @@ fn mod_file_path_from_attr( dir_path: &Path, ) -> Option { // Extract path string from first `#[path = "path_string"]` attribute. - let path_string = sess.first_attr_value_str_by_name(attrs, sym::path)?; - let path_string = path_string.as_str(); + let path_string = sess.first_attr_value_str_by_name(attrs, sym::path)?.as_str(); // On windows, the base path might have the form // `\\?\foo\bar` in which case it does not tolerate From 1fe2eb83ecfed43874b4cdf39d0be8910995dd1d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 21:22:40 +0300 Subject: [PATCH 20/25] expand: Introduce enum for module loading errors and make module loading speculative --- compiler/rustc_expand/src/lib.rs | 1 + compiler/rustc_expand/src/module.rs | 172 +++++++++--------- .../directory_ownership/macro-expanded-mod.rs | 2 +- .../macro-expanded-mod.stderr | 2 +- .../non-inline-mod-restriction.rs | 2 +- .../non-inline-mod-restriction.stderr | 2 +- 6 files changed, 90 insertions(+), 91 deletions(-) diff --git a/compiler/rustc_expand/src/lib.rs b/compiler/rustc_expand/src/lib.rs index bcccb04c9efd..1a93975533de 100644 --- a/compiler/rustc_expand/src/lib.rs +++ b/compiler/rustc_expand/src/lib.rs @@ -1,3 +1,4 @@ +#![feature(bool_to_option)] #![feature(crate_visibility_modifier)] #![feature(decl_macro)] #![feature(destructuring_assignment)] diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 8f5200804e0d..2ec656d4895e 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -1,7 +1,7 @@ use crate::base::ModuleData; use rustc_ast::ptr::P; use rustc_ast::{token, Attribute, Item}; -use rustc_errors::{struct_span_err, PResult}; +use rustc_errors::{struct_span_err, DiagnosticBuilder}; use rustc_parse::new_parser_from_file; use rustc_session::parse::ParseSess; use rustc_session::Session; @@ -19,14 +19,6 @@ pub enum DirOwnership { UnownedViaBlock, } -/// Information about the path to a module. -// Public for rustfmt usage. -pub struct ModulePath<'a> { - name: String, - path_exists: bool, - pub result: PResult<'a, ModulePathSuccess>, -} - // Public for rustfmt usage. pub struct ModulePathSuccess { pub file_path: PathBuf, @@ -41,6 +33,14 @@ crate struct ParsedExternalMod { pub dir_ownership: DirOwnership, } +pub enum ModError<'a> { + CircularInclusion(Vec), + ModInBlock(Option), + FileNotFound(Ident, PathBuf), + MultipleCandidates(Ident, String, String), + ParserError(DiagnosticBuilder<'a>), +} + crate fn parse_external_mod( sess: &Session, ident: Ident, @@ -50,22 +50,26 @@ crate fn parse_external_mod( attrs: &mut Vec, ) -> ParsedExternalMod { // We bail on the first error, but that error does not cause a fatal error... (1) - let result: PResult<'_, _> = try { + let result: Result<_, ModError<'_>> = try { // Extract the file path and the new ownership. - let mp = mod_file_path(sess, ident, span, &attrs, &module.dir_path, dir_ownership)?; + let mp = mod_file_path(sess, ident, &attrs, &module.dir_path, dir_ownership)?; dir_ownership = mp.dir_ownership; // Ensure file paths are acyclic. - error_on_circular_module(&sess.parse_sess, span, &mp.file_path, &module.file_path_stack)?; + if let Some(pos) = module.file_path_stack.iter().position(|p| p == &mp.file_path) { + Err(ModError::CircularInclusion(module.file_path_stack[pos..].to_vec()))?; + } // Actually parse the external file as a module. let mut parser = new_parser_from_file(&sess.parse_sess, &mp.file_path, Some(span)); - let (mut inner_attrs, items, inner_span) = parser.parse_mod(&token::Eof)?; + let (mut inner_attrs, items, inner_span) = + parser.parse_mod(&token::Eof).map_err(|err| ModError::ParserError(err))?; attrs.append(&mut inner_attrs); (items, inner_span, mp.file_path) }; // (1) ...instead, we return a dummy module. - let (items, inner_span, file_path) = result.map_err(|mut err| err.emit()).unwrap_or_default(); + let (items, inner_span, file_path) = + result.map_err(|err| err.report(sess, span)).unwrap_or_default(); // Extract the directory path for submodules of the module. let dir_path = file_path.parent().unwrap_or(&file_path).to_owned(); @@ -73,24 +77,6 @@ crate fn parse_external_mod( ParsedExternalMod { items, inner_span, file_path, dir_path, dir_ownership } } -fn error_on_circular_module<'a>( - sess: &'a ParseSess, - span: Span, - file_path: &Path, - file_path_stack: &[PathBuf], -) -> PResult<'a, ()> { - if let Some(i) = file_path_stack.iter().position(|p| *p == file_path) { - let mut err = String::from("circular modules: "); - for p in &file_path_stack[i..] { - err.push_str(&p.to_string_lossy()); - err.push_str(" -> "); - } - err.push_str(&file_path.to_string_lossy()); - return Err(sess.span_diagnostic.struct_span_err(span, &err[..])); - } - Ok(()) -} - crate fn mod_dir_path( sess: &Session, ident: Ident, @@ -125,11 +111,10 @@ crate fn mod_dir_path( fn mod_file_path<'a>( sess: &'a Session, ident: Ident, - span: Span, attrs: &[Attribute], dir_path: &Path, dir_ownership: DirOwnership, -) -> PResult<'a, ModulePathSuccess> { +) -> Result> { if let Some(file_path) = mod_file_path_from_attr(sess, attrs, dir_path) { // All `#[path]` files are treated as though they are a `mod.rs` file. // This means that `mod foo;` declarations inside `#[path]`-included @@ -146,30 +131,14 @@ fn mod_file_path<'a>( DirOwnership::Owned { relative } => relative, DirOwnership::UnownedViaBlock => None, }; - let ModulePath { path_exists, name, result } = - default_submod_path(&sess.parse_sess, ident, span, relative, dir_path); + let result = default_submod_path(&sess.parse_sess, ident, relative, dir_path); match dir_ownership { - DirOwnership::Owned { .. } => Ok(result?), - DirOwnership::UnownedViaBlock => { - let _ = result.map_err(|mut err| err.cancel()); - error_decl_mod_in_block(&sess.parse_sess, span, path_exists, &name) - } - } -} - -fn error_decl_mod_in_block<'a, T>( - sess: &'a ParseSess, - span: Span, - path_exists: bool, - name: &str, -) -> PResult<'a, T> { - let msg = "Cannot declare a non-inline module inside a block unless it has a path attribute"; - let mut err = sess.span_diagnostic.struct_span_err(span, msg); - if path_exists { - let msg = format!("Maybe `use` the module `{}` instead of redeclaring it", name); - err.span_note(span, &msg); + DirOwnership::Owned { .. } => result, + DirOwnership::UnownedViaBlock => Err(ModError::ModInBlock(match result { + Ok(_) | Err(ModError::MultipleCandidates(..)) => Some(ident), + _ => None, + })), } - Err(err) } /// Derive a submodule path from the first found `#[path = "path_string"]`. @@ -197,10 +166,9 @@ fn mod_file_path_from_attr( pub fn default_submod_path<'a>( sess: &'a ParseSess, ident: Ident, - span: Span, relative: Option, dir_path: &Path, -) -> ModulePath<'a> { +) -> Result> { // If we're in a foo.rs file instead of a mod.rs file, // we need to look for submodules in // `./foo/.rs` and `./foo//mod.rs` rather than @@ -222,7 +190,7 @@ pub fn default_submod_path<'a>( let default_exists = sess.source_map().file_exists(&default_path); let secondary_exists = sess.source_map().file_exists(&secondary_path); - let result = match (default_exists, secondary_exists) { + match (default_exists, secondary_exists) { (true, false) => Ok(ModulePathSuccess { file_path: default_path, dir_ownership: DirOwnership::Owned { relative: Some(ident) }, @@ -231,35 +199,65 @@ pub fn default_submod_path<'a>( file_path: secondary_path, dir_ownership: DirOwnership::Owned { relative: None }, }), - (false, false) => { - let mut err = struct_span_err!( - sess.span_diagnostic, - span, - E0583, - "file not found for module `{}`", - mod_name, - ); - err.help(&format!( - "to create the module `{}`, create file \"{}\"", - mod_name, - default_path.display(), - )); - Err(err) - } + (false, false) => Err(ModError::FileNotFound(ident, default_path)), (true, true) => { - let mut err = struct_span_err!( - sess.span_diagnostic, - span, - E0761, - "file for module `{}` found at both {} and {}", - mod_name, - default_path_str, - secondary_path_str, - ); - err.help("delete or rename one of them to remove the ambiguity"); - Err(err) + Err(ModError::MultipleCandidates(ident, default_path_str, secondary_path_str)) } - }; + } +} - ModulePath { name: mod_name, path_exists: default_exists || secondary_exists, result } +impl ModError<'_> { + fn report(self, sess: &Session, span: Span) { + let diag = &sess.parse_sess.span_diagnostic; + match self { + ModError::CircularInclusion(file_paths) => { + let mut msg = String::from("circular modules: "); + for file_path in &file_paths { + msg.push_str(&file_path.display().to_string()); + msg.push_str(" -> "); + } + msg.push_str(&file_paths[0].display().to_string()); + diag.struct_span_err(span, &msg) + } + ModError::ModInBlock(ident) => { + let msg = "cannot declare a non-inline module inside a block unless it has a path attribute"; + let mut err = diag.struct_span_err(span, msg); + if let Some(ident) = ident { + let note = + format!("maybe `use` the module `{}` instead of redeclaring it", ident); + err.span_note(span, ¬e); + } + err + } + ModError::FileNotFound(ident, default_path) => { + let mut err = struct_span_err!( + diag, + span, + E0583, + "file not found for module `{}`", + ident, + ); + err.help(&format!( + "to create the module `{}`, create file \"{}\"", + ident, + default_path.display(), + )); + err + } + ModError::MultipleCandidates(ident, default_path_short, secondary_path_short) => { + let mut err = struct_span_err!( + diag, + span, + E0761, + "file for module `{}` found at both {} and {}", + ident, + default_path_short, + secondary_path_short, + ); + err.help("delete or rename one of them to remove the ambiguity"); + err + } + ModError::ParserError(err) => err, + }.emit() + } } diff --git a/src/test/ui/directory_ownership/macro-expanded-mod.rs b/src/test/ui/directory_ownership/macro-expanded-mod.rs index 9cb159603a8c..fa81769e5a80 100644 --- a/src/test/ui/directory_ownership/macro-expanded-mod.rs +++ b/src/test/ui/directory_ownership/macro-expanded-mod.rs @@ -2,7 +2,7 @@ macro_rules! mod_decl { ($i:ident) => { - mod $i; //~ ERROR Cannot declare a non-inline module inside a block + mod $i; //~ ERROR cannot declare a non-inline module inside a block }; } diff --git a/src/test/ui/directory_ownership/macro-expanded-mod.stderr b/src/test/ui/directory_ownership/macro-expanded-mod.stderr index f90419247c92..4039728e18ac 100644 --- a/src/test/ui/directory_ownership/macro-expanded-mod.stderr +++ b/src/test/ui/directory_ownership/macro-expanded-mod.stderr @@ -1,4 +1,4 @@ -error: Cannot declare a non-inline module inside a block unless it has a path attribute +error: cannot declare a non-inline module inside a block unless it has a path attribute --> $DIR/macro-expanded-mod.rs:5:9 | LL | mod $i; diff --git a/src/test/ui/directory_ownership/non-inline-mod-restriction.rs b/src/test/ui/directory_ownership/non-inline-mod-restriction.rs index af31b8a49281..de4f816656cc 100644 --- a/src/test/ui/directory_ownership/non-inline-mod-restriction.rs +++ b/src/test/ui/directory_ownership/non-inline-mod-restriction.rs @@ -1,5 +1,5 @@ // Test that non-inline modules are not allowed inside blocks. fn main() { - mod foo; //~ ERROR Cannot declare a non-inline module inside a block + mod foo; //~ ERROR cannot declare a non-inline module inside a block } diff --git a/src/test/ui/directory_ownership/non-inline-mod-restriction.stderr b/src/test/ui/directory_ownership/non-inline-mod-restriction.stderr index d034942ca5d4..64189bee43f6 100644 --- a/src/test/ui/directory_ownership/non-inline-mod-restriction.stderr +++ b/src/test/ui/directory_ownership/non-inline-mod-restriction.stderr @@ -1,4 +1,4 @@ -error: Cannot declare a non-inline module inside a block unless it has a path attribute +error: cannot declare a non-inline module inside a block unless it has a path attribute --> $DIR/non-inline-mod-restriction.rs:4:5 | LL | mod foo; From 2371914a05f8f2763dffe6e2511d0870bcd6b461 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Wed, 3 Mar 2021 21:09:01 +0100 Subject: [PATCH 21/25] Prevent Zip specialization from calling __iterator_get_unchecked twice with the same index after calling next_back --- library/core/src/iter/adapters/zip.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 817fc2a51e98..ea7a809c6bad 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -13,9 +13,10 @@ use crate::iter::{InPlaceIterable, SourceIter, TrustedLen}; pub struct Zip { a: A, b: B, - // index and len are only used by the specialized version of zip + // index, len and a_len are only used by the specialized version of zip index: usize, len: usize, + a_len: usize, } impl Zip { pub(in crate::iter) fn new(a: A, b: B) -> Zip { @@ -110,6 +111,7 @@ where b, index: 0, // unused len: 0, // unused + a_len: 0, // unused } } @@ -184,8 +186,9 @@ where B: TrustedRandomAccess + Iterator, { fn new(a: A, b: B) -> Self { - let len = cmp::min(a.size(), b.size()); - Zip { a, b, index: 0, len } + let a_len = a.size(); + let len = cmp::min(a_len, b.size()); + Zip { a, b, index: 0, len, a_len } } #[inline] @@ -197,7 +200,7 @@ where unsafe { Some((self.a.__iterator_get_unchecked(i), self.b.__iterator_get_unchecked(i))) } - } else if A::MAY_HAVE_SIDE_EFFECT && self.index < self.a.size() { + } else if A::MAY_HAVE_SIDE_EFFECT && self.index < self.a_len { let i = self.index; self.index += 1; self.len += 1; @@ -262,6 +265,7 @@ where for _ in 0..sz_a - self.len { self.a.next_back(); } + self.a_len = self.len; } let sz_b = self.b.size(); if B::MAY_HAVE_SIDE_EFFECT && sz_b > self.len { @@ -273,6 +277,7 @@ where } if self.index < self.len { self.len -= 1; + self.a_len -= 1; let i = self.len; // SAFETY: `i` is smaller than the previous value of `self.len`, // which is also smaller than or equal to `self.a.len()` and `self.b.len()` From c1bfb9a78db6d481be1d03355672712c766e20b0 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Fri, 19 Feb 2021 15:25:09 +0100 Subject: [PATCH 22/25] Add relevant test --- library/core/tests/iter/adapters/zip.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/library/core/tests/iter/adapters/zip.rs b/library/core/tests/iter/adapters/zip.rs index a59771039295..000c15f72c88 100644 --- a/library/core/tests/iter/adapters/zip.rs +++ b/library/core/tests/iter/adapters/zip.rs @@ -265,3 +265,26 @@ fn test_issue_82282() { panic!(); } } + +#[test] +fn test_issue_82291() { + use std::cell::Cell; + + let mut v1 = [()]; + let v2 = [()]; + + let called = Cell::new(0); + + let mut zip = v1 + .iter_mut() + .map(|r| { + called.set(called.get() + 1); + r + }) + .zip(&v2); + + zip.next_back(); + assert_eq!(called.get(), 1); + zip.next(); + assert_eq!(called.get(), 1); +} From 7d3a6f16551fa9ff54fbfa9c648c21ae0f6d550f Mon Sep 17 00:00:00 2001 From: Henry Boisdequin <65845077+henryboisdequin@users.noreply.github.com> Date: Wed, 3 Mar 2021 17:09:40 +0530 Subject: [PATCH 23/25] address comments --- compiler/rustc_typeck/src/collect/type_of.rs | 6 ++++-- src/test/ui/79040.rs | 5 ----- src/test/ui/parser/item-free-const-no-body-semantic-fail.rs | 1 - .../ui/parser/item-free-static-no-body-semantic-fail.rs | 2 -- .../ui/parser/item-free-static-no-body-semantic-fail.stderr | 4 ++-- src/test/ui/typeck/issue-79040.rs | 5 +++++ src/test/ui/{79040.stderr => typeck/issue-79040.stderr} | 4 ++-- 7 files changed, 13 insertions(+), 14 deletions(-) delete mode 100644 src/test/ui/79040.rs create mode 100644 src/test/ui/typeck/issue-79040.rs rename src/test/ui/{79040.stderr => typeck/issue-79040.stderr} (88%) diff --git a/compiler/rustc_typeck/src/collect/type_of.rs b/compiler/rustc_typeck/src/collect/type_of.rs index 92e2b5232027..1f47e4899f18 100644 --- a/compiler/rustc_typeck/src/collect/type_of.rs +++ b/compiler/rustc_typeck/src/collect/type_of.rs @@ -659,11 +659,12 @@ fn infer_placeholder_type( format!("{}: {}", item_ident, ty), Applicability::MachineApplicable, ) - .emit_unless(matches!(ty.kind(), ty::Error(_))); + .emit_unless(ty.references_error()); } None => { let mut diag = bad_placeholder_type(tcx, vec![span]); - if !matches!(ty.kind(), ty::Error(_)) { + + if !ty.references_error() { diag.span_suggestion( span, "replace `_` with the correct type", @@ -671,6 +672,7 @@ fn infer_placeholder_type( Applicability::MaybeIncorrect, ); } + diag.emit(); } } diff --git a/src/test/ui/79040.rs b/src/test/ui/79040.rs deleted file mode 100644 index adc9dc8b4265..000000000000 --- a/src/test/ui/79040.rs +++ /dev/null @@ -1,5 +0,0 @@ -fn main() { - const FOO = "hello" + 1; - //^~ ERROR cannot add `{integer}` to `&str` - println!("{}", FOO); -} \ No newline at end of file diff --git a/src/test/ui/parser/item-free-const-no-body-semantic-fail.rs b/src/test/ui/parser/item-free-const-no-body-semantic-fail.rs index 613b3c985617..15a15a207b1c 100644 --- a/src/test/ui/parser/item-free-const-no-body-semantic-fail.rs +++ b/src/test/ui/parser/item-free-const-no-body-semantic-fail.rs @@ -4,4 +4,3 @@ fn main() {} const A: u8; //~ ERROR free constant item without body const B; //~ ERROR free constant item without body -//~^ ERROR missing type for `const` item diff --git a/src/test/ui/parser/item-free-static-no-body-semantic-fail.rs b/src/test/ui/parser/item-free-static-no-body-semantic-fail.rs index 780479e3d26a..61d3eab24d8c 100644 --- a/src/test/ui/parser/item-free-static-no-body-semantic-fail.rs +++ b/src/test/ui/parser/item-free-static-no-body-semantic-fail.rs @@ -4,8 +4,6 @@ fn main() {} static A: u8; //~ ERROR free static item without body static B; //~ ERROR free static item without body -//~^ ERROR missing type for `static` item static mut C: u8; //~ ERROR free static item without body static mut D; //~ ERROR free static item without body -//~^ ERROR missing type for `static mut` item diff --git a/src/test/ui/parser/item-free-static-no-body-semantic-fail.stderr b/src/test/ui/parser/item-free-static-no-body-semantic-fail.stderr index 2a270c8290fd..7b408323674d 100644 --- a/src/test/ui/parser/item-free-static-no-body-semantic-fail.stderr +++ b/src/test/ui/parser/item-free-static-no-body-semantic-fail.stderr @@ -15,7 +15,7 @@ LL | static B; | help: provide a definition for the static: `= ;` error: free static item without body - --> $DIR/item-free-static-no-body-semantic-fail.rs:9:1 + --> $DIR/item-free-static-no-body-semantic-fail.rs:8:1 | LL | static mut C: u8; | ^^^^^^^^^^^^^^^^- @@ -23,7 +23,7 @@ LL | static mut C: u8; | help: provide a definition for the static: `= ;` error: free static item without body - --> $DIR/item-free-static-no-body-semantic-fail.rs:10:1 + --> $DIR/item-free-static-no-body-semantic-fail.rs:9:1 | LL | static mut D; | ^^^^^^^^^^^^- diff --git a/src/test/ui/typeck/issue-79040.rs b/src/test/ui/typeck/issue-79040.rs new file mode 100644 index 000000000000..af2a9c1ba87e --- /dev/null +++ b/src/test/ui/typeck/issue-79040.rs @@ -0,0 +1,5 @@ +fn main() { + const FOO = "hello" + 1; //~ ERROR cannot add `{integer}` to `&str` + //~^ ERROR cannot add `{integer}` to `&str` + println!("{}", FOO); +} diff --git a/src/test/ui/79040.stderr b/src/test/ui/typeck/issue-79040.stderr similarity index 88% rename from src/test/ui/79040.stderr rename to src/test/ui/typeck/issue-79040.stderr index be7d2363c2d5..32049e5d9686 100644 --- a/src/test/ui/79040.stderr +++ b/src/test/ui/typeck/issue-79040.stderr @@ -1,5 +1,5 @@ error[E0369]: cannot add `{integer}` to `&str` - --> $DIR/79040.rs:2:25 + --> $DIR/issue-79040.rs:2:25 | LL | const FOO = "hello" + 1; | ------- ^ - {integer} @@ -7,7 +7,7 @@ LL | const FOO = "hello" + 1; | &str error[E0369]: cannot add `{integer}` to `&str` - --> $DIR/79040.rs:2:25 + --> $DIR/issue-79040.rs:2:25 | LL | const FOO = "hello" + 1; | ------- ^ - {integer} From fbc17410b24719f2b3ff18f622bd677c76f59fde Mon Sep 17 00:00:00 2001 From: Lonami Date: Sat, 27 Feb 2021 16:16:41 +0100 Subject: [PATCH 24/25] Improve transmute docs with further clarifications Closes #82493. --- library/core/src/intrinsics.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 5274262ded2b..9ca29b81c7d4 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -846,6 +846,12 @@ extern "rust-intrinsic" { /// destination value, then forgets the original. It's equivalent to C's /// `memcpy` under the hood, just like `transmute_copy`. /// + /// Because `transmute` is a by-value operation, alignment of the *transmuted values + /// themselves* is not a concern. As with any other function, the compiler already ensures + /// both `T` and `U` are properly aligned. However, when transmuting values that *point + /// elsewhere* (such as pointers, references, boxes…), the caller has to ensure proper + /// alignment of the pointed-to values. + /// /// `transmute` is **incredibly** unsafe. There are a vast number of ways to /// cause [undefined behavior][ub] with this function. `transmute` should be /// the absolute last resort. @@ -965,7 +971,13 @@ extern "rust-intrinsic" { /// assert_eq!(b"Rust", &[82, 117, 115, 116]); /// ``` /// - /// Turning a `Vec<&T>` into a `Vec>`: + /// Turning a `Vec<&T>` into a `Vec>`. + /// + /// To transmute the inner type of the contents of a container, you must make sure to not + /// violate any of the container's invariants. For `Vec`, this means that both the size + /// *and alignment* of the inner types have to match. Other containers might rely on the + /// size of the type, alignment, or even the `TypeId`, in which case transmuting wouldn't + /// be possible at all without violating the container invariants. /// /// ``` /// let store = [0, 1, 2, 3]; @@ -991,14 +1003,11 @@ extern "rust-intrinsic" { /// /// let v_clone = v_orig.clone(); /// - /// // The no-copy, unsafe way, still using transmute, but not relying on the data layout. - /// // Like the first approach, this reuses the `Vec` internals. - /// // Therefore, the new inner type must have the - /// // exact same size, *and the same alignment*, as the old type. - /// // The same caveats exist for this method as transmute, for - /// // the original inner type (`&i32`) to the converted inner type - /// // (`Option<&i32>`), so read the nomicon pages linked above and also - /// // consult the [`from_raw_parts`] documentation. + /// // This is the proper no-copy, unsafe way of "transmuting" a `Vec`, without relying on the + /// // data layout. Instead of literally calling `transmute`, we perform a pointer cast, but + /// // in terms of converting the original inner type (`&i32`) to the new one (`Option<&i32>`), + /// // this has all the same caveats. Besides the information provided above, also consult the + /// // [`from_raw_parts`] documentation. /// let v_from_raw = unsafe { // FIXME Update this when vec_into_raw_parts is stabilized /// // Ensure the original vector is not dropped. From 79c2b75e88bdab5fccd7d63750ef11aff2f8eaba Mon Sep 17 00:00:00 2001 From: Jacob Pratt Date: Sun, 14 Feb 2021 22:11:46 -0500 Subject: [PATCH 25/25] Make some Option, Result methods unstably const The following functions are now unstably const: - Option::transpose - Option::flatten - Result::transpose --- library/core/src/option.rs | 13 +++++++++---- library/core/src/result.rs | 3 ++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/library/core/src/option.rs b/library/core/src/option.rs index 14e4e4da3b96..c5c7922f8192 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -150,7 +150,7 @@ use crate::iter::{FromIterator, FusedIterator, TrustedLen}; use crate::pin::Pin; use crate::{ - convert, fmt, hint, mem, + fmt, hint, mem, ops::{self, Deref, DerefMut}, }; @@ -1275,7 +1275,8 @@ impl Option> { /// ``` #[inline] #[stable(feature = "transpose_result", since = "1.33.0")] - pub fn transpose(self) -> Result, E> { + #[rustc_const_unstable(feature = "const_option", issue = "67441")] + pub const fn transpose(self) -> Result, E> { match self { Some(Ok(x)) => Ok(Some(x)), Some(Err(e)) => Err(e), @@ -1750,7 +1751,11 @@ impl Option> { /// ``` #[inline] #[stable(feature = "option_flattening", since = "1.40.0")] - pub fn flatten(self) -> Option { - self.and_then(convert::identity) + #[rustc_const_unstable(feature = "const_option", issue = "67441")] + pub const fn flatten(self) -> Option { + match self { + Some(inner) => inner, + None => None, + } } } diff --git a/library/core/src/result.rs b/library/core/src/result.rs index a43ba5882edc..a00cd98ae5c2 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1233,7 +1233,8 @@ impl Result, E> { /// ``` #[inline] #[stable(feature = "transpose_result", since = "1.33.0")] - pub fn transpose(self) -> Option> { + #[rustc_const_unstable(feature = "const_result", issue = "82814")] + pub const fn transpose(self) -> Option> { match self { Ok(Some(x)) => Some(Ok(x)), Ok(None) => None,