From 8553aeeb66afa1369548f9e7d88409459f5ff815 Mon Sep 17 00:00:00 2001 From: Shaheen Gandhi Date: Sat, 19 Dec 2020 19:32:07 -0800 Subject: [PATCH 01/17] Use -target when linking binaries for Mac Catalyst --- compiler/rustc_codegen_ssa/src/back/link.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index ccd4d103ddb7f..a3e230a7f692e 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -2197,8 +2197,13 @@ fn add_apple_sdk(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) { return; } }; - let arch_name = llvm_target.split('-').next().expect("LLVM target must have a hyphen"); - cmd.args(&["-arch", arch_name, "-isysroot", &sdk_root, "-Wl,-syslibroot", &sdk_root]); + if llvm_target.contains("macabi") { + cmd.args(&["-target", llvm_target]) + } else { + let arch_name = llvm_target.split('-').next().expect("LLVM target must have a hyphen"); + cmd.args(&["-arch", arch_name]) + } + cmd.args(&["-isysroot", &sdk_root, "-Wl,-syslibroot", &sdk_root]); } fn get_apple_sdk_root(sdk_name: &str) -> Result { From 5db5d8f87e2d224df6feea200af371d13394f2be Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 15 Jan 2021 20:45:38 +0000 Subject: [PATCH 02/17] Make hitting the recursion limit in projection non-fatal This is relied on by wundergraph. --- .../src/traits/project.rs | 11 +--- .../src/traits/select/mod.rs | 4 -- .../project-recursion-limit-non-fatal.rs | 58 +++++++++++++++++++ src/test/ui/issues/issue-23122-2.stderr | 3 +- 4 files changed, 63 insertions(+), 13 deletions(-) create mode 100644 src/test/ui/associated-types/project-recursion-limit-non-fatal.rs diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index fa0526445c194..f22b5b9661699 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -736,14 +736,9 @@ fn project_type<'cx, 'tcx>( if !selcx.tcx().sess.recursion_limit().value_within_limit(obligation.recursion_depth) { debug!("project: overflow!"); - match selcx.query_mode() { - super::TraitQueryMode::Standard => { - selcx.infcx().report_overflow_error(&obligation, true); - } - super::TraitQueryMode::Canonical => { - return Err(ProjectionTyError::TraitSelectionError(SelectionError::Overflow)); - } - } + // This should really be an immediate error, but some existing code + // relies on being able to recover from this. + return Err(ProjectionTyError::TraitSelectionError(SelectionError::Overflow)); } let obligation_trait_ref = &obligation.predicate.trait_ref(selcx.tcx()); diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 8ca540fc8933c..7221ce811d1e8 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -291,10 +291,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { self.infcx.tcx } - pub(super) fn query_mode(&self) -> TraitQueryMode { - self.query_mode - } - /////////////////////////////////////////////////////////////////////////// // Selection // diff --git a/src/test/ui/associated-types/project-recursion-limit-non-fatal.rs b/src/test/ui/associated-types/project-recursion-limit-non-fatal.rs new file mode 100644 index 0000000000000..3e68b1401020f --- /dev/null +++ b/src/test/ui/associated-types/project-recursion-limit-non-fatal.rs @@ -0,0 +1,58 @@ +// Regression test for #80953. Hitting the recursion limit in projection +// is non-fatal. The above code, minimised from wundergraph shows a case +// where this is relied on. + +// check-pass + +struct AlternateTable {} +struct AlternateQuery {} + +pub trait Query {} +pub trait AsQuery { + type Query; +} +impl AsQuery for T { + type Query = Self; +} +impl AsQuery for AlternateTable { + type Query = AlternateQuery; +} + +pub trait Table: AsQuery { + type PrimaryKey; +} +impl Table for AlternateTable { + type PrimaryKey = (); +} + +pub trait FilterDsl { + type Output; +} +pub type Filter = >::Output; +impl FilterDsl for T +where + T: Table, + T::Query: FilterDsl, +{ + type Output = Filter; +} +impl FilterDsl for AlternateQuery { + type Output = &'static str; +} + +pub trait HandleDelete { + type Filter; +} +impl HandleDelete for T +where + T: Table, + T::Query: FilterDsl, + Filter: , +{ + type Filter = Filter; +} + +fn main() { + let x: ::Filter = "Hello, world"; + println!("{}", x); +} diff --git a/src/test/ui/issues/issue-23122-2.stderr b/src/test/ui/issues/issue-23122-2.stderr index ff7e884ea6f83..ce3bffe602ca0 100644 --- a/src/test/ui/issues/issue-23122-2.stderr +++ b/src/test/ui/issues/issue-23122-2.stderr @@ -1,10 +1,11 @@ -error[E0275]: overflow evaluating the requirement `<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next` +error[E0275]: overflow evaluating the requirement `<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next: Sized` --> $DIR/issue-23122-2.rs:9:5 | LL | type Next = as Next>::Next; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`issue_23122_2`) + = note: required because of the requirements on the impl of `Next` for `GetNext<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next>` error: aborting due to previous error From 63a1eeea234105fd9c1ed30ac2b6e0d9bf41a1e9 Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 18 Jan 2021 11:55:23 -0600 Subject: [PATCH 03/17] Reset LateContext enclosing body in nested items Prevents LateContext::maybe_typeck_results() from returning data in a nested item without a body. Consequently, LateContext::qpath_res is less likely to ICE when called in a nested item. Would have prevented rust-lang/rust-clippy#4545, presumably. --- compiler/rustc_lint/src/late.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_lint/src/late.rs b/compiler/rustc_lint/src/late.rs index 015e109871182..3821a393efb8b 100644 --- a/compiler/rustc_lint/src/late.rs +++ b/compiler/rustc_lint/src/late.rs @@ -140,6 +140,8 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas fn visit_item(&mut self, it: &'tcx hir::Item<'tcx>) { let generics = self.context.generics.take(); self.context.generics = it.kind.generics(); + let old_cached_typeck_results = self.context.cached_typeck_results.take(); + let old_enclosing_body = self.context.enclosing_body.take(); self.with_lint_attrs(it.hir_id, &it.attrs, |cx| { cx.with_param_env(it.hir_id, |cx| { lint_callback!(cx, check_item, it); @@ -147,6 +149,8 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas lint_callback!(cx, check_item_post, it); }); }); + self.context.enclosing_body = old_enclosing_body; + self.context.cached_typeck_results.set(old_cached_typeck_results); self.context.generics = generics; } From 21fb586c0c72a546bae445df1588cef464502ada Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 18 Jan 2021 13:29:37 -0600 Subject: [PATCH 04/17] Query for TypeckResults in LateContext::qpath_res Actually fulfills the documented guarantees. --- compiler/rustc_lint/src/context.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index 3971a3099823f..3f92d66d88bce 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -746,6 +746,14 @@ impl<'tcx> LateContext<'tcx> { hir::QPath::Resolved(_, ref path) => path.res, hir::QPath::TypeRelative(..) | hir::QPath::LangItem(..) => self .maybe_typeck_results() + .filter(|typeck_results| typeck_results.hir_owner == id.owner) + .or_else(|| { + if self.tcx.has_typeck_results(id.owner.to_def_id()) { + Some(self.tcx.typeck(id.owner)) + } else { + None + } + }) .and_then(|typeck_results| typeck_results.type_dependent_def(id)) .map_or(Res::Err, |(kind, def_id)| Res::Def(kind, def_id)), } From eaba3daa60d789997c0be3da11619df2469e9a7e Mon Sep 17 00:00:00 2001 From: Cameron Steffen Date: Mon, 18 Jan 2021 13:36:32 -0600 Subject: [PATCH 05/17] Remove qpath_res util function --- src/tools/clippy/clippy_lints/src/default.rs | 4 ++-- .../clippy_lints/src/drop_forget_ref.rs | 4 ++-- src/tools/clippy/clippy_lints/src/exit.rs | 4 ++-- .../clippy/clippy_lints/src/functions.rs | 8 +++---- .../clippy/clippy_lints/src/let_if_seq.rs | 4 ++-- src/tools/clippy/clippy_lints/src/loops.rs | 22 +++++++++---------- .../clippy/clippy_lints/src/manual_strip.rs | 8 +++---- .../clippy/clippy_lints/src/mem_forget.rs | 4 ++-- .../clippy/clippy_lints/src/no_effect.rs | 6 ++--- .../clippy/clippy_lints/src/non_copy_const.rs | 4 ++-- .../clippy_lints/src/to_string_in_display.rs | 4 ++-- src/tools/clippy/clippy_lints/src/types.rs | 12 +++++----- .../clippy_lints/src/utils/internal_lints.rs | 6 ++--- .../clippy/clippy_lints/src/utils/mod.rs | 13 ----------- 14 files changed, 45 insertions(+), 58 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/default.rs b/src/tools/clippy/clippy_lints/src/default.rs index f7224811e6e79..6fa1378b8c73d 100644 --- a/src/tools/clippy/clippy_lints/src/default.rs +++ b/src/tools/clippy/clippy_lints/src/default.rs @@ -1,5 +1,5 @@ use crate::utils::{ - any_parent_is_automatically_derived, contains_name, match_def_path, paths, qpath_res, snippet_with_macro_callsite, + any_parent_is_automatically_derived, contains_name, match_def_path, paths, snippet_with_macro_callsite, }; use crate::utils::{span_lint_and_note, span_lint_and_sugg}; use if_chain::if_chain; @@ -231,7 +231,7 @@ fn is_expr_default<'tcx>(expr: &'tcx Expr<'tcx>, cx: &LateContext<'tcx>) -> bool if_chain! { if let ExprKind::Call(ref fn_expr, _) = &expr.kind; if let ExprKind::Path(qpath) = &fn_expr.kind; - if let Res::Def(_, def_id) = qpath_res(cx, qpath, fn_expr.hir_id); + if let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id); then { // right hand side of assignment is `Default::default` match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD) diff --git a/src/tools/clippy/clippy_lints/src/drop_forget_ref.rs b/src/tools/clippy/clippy_lints/src/drop_forget_ref.rs index cf528d189b4b1..a84f9c4628716 100644 --- a/src/tools/clippy/clippy_lints/src/drop_forget_ref.rs +++ b/src/tools/clippy/clippy_lints/src/drop_forget_ref.rs @@ -1,4 +1,4 @@ -use crate::utils::{is_copy, match_def_path, paths, qpath_res, span_lint_and_note}; +use crate::utils::{is_copy, match_def_path, paths, span_lint_and_note}; use if_chain::if_chain; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -114,7 +114,7 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef { if let ExprKind::Call(ref path, ref args) = expr.kind; if let ExprKind::Path(ref qpath) = path.kind; if args.len() == 1; - if let Some(def_id) = qpath_res(cx, qpath, path.hir_id).opt_def_id(); + if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id(); then { let lint; let msg; diff --git a/src/tools/clippy/clippy_lints/src/exit.rs b/src/tools/clippy/clippy_lints/src/exit.rs index 7337d98c8be37..915859270009b 100644 --- a/src/tools/clippy/clippy_lints/src/exit.rs +++ b/src/tools/clippy/clippy_lints/src/exit.rs @@ -1,4 +1,4 @@ -use crate::utils::{is_entrypoint_fn, match_def_path, paths, qpath_res, span_lint}; +use crate::utils::{is_entrypoint_fn, match_def_path, paths, span_lint}; use if_chain::if_chain; use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; @@ -29,7 +29,7 @@ impl<'tcx> LateLintPass<'tcx> for Exit { if_chain! { if let ExprKind::Call(ref path_expr, ref _args) = e.kind; if let ExprKind::Path(ref path) = path_expr.kind; - if let Some(def_id) = qpath_res(cx, path, path_expr.hir_id).opt_def_id(); + if let Some(def_id) = cx.qpath_res(path, path_expr.hir_id).opt_def_id(); if match_def_path(cx, def_id, &paths::EXIT); then { let parent = cx.tcx.hir().get_parent_item(e.hir_id); diff --git a/src/tools/clippy/clippy_lints/src/functions.rs b/src/tools/clippy/clippy_lints/src/functions.rs index fd93548b55c6d..8795425461033 100644 --- a/src/tools/clippy/clippy_lints/src/functions.rs +++ b/src/tools/clippy/clippy_lints/src/functions.rs @@ -1,7 +1,7 @@ use crate::utils::{ attr_by_name, attrs::is_proc_macro, is_must_use_ty, is_trait_impl_item, is_type_diagnostic_item, iter_input_pats, - last_path_segment, match_def_path, must_use_attr, qpath_res, return_ty, snippet, snippet_opt, span_lint, - span_lint_and_help, span_lint_and_then, trait_ref_of_method, type_is_unsafe_function, + last_path_segment, match_def_path, must_use_attr, return_ty, snippet, snippet_opt, span_lint, span_lint_and_help, + span_lint_and_then, trait_ref_of_method, type_is_unsafe_function, }; use if_chain::if_chain; use rustc_ast::ast::Attribute; @@ -659,7 +659,7 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { impl<'a, 'tcx> DerefVisitor<'a, 'tcx> { fn check_arg(&self, ptr: &hir::Expr<'_>) { if let hir::ExprKind::Path(ref qpath) = ptr.kind { - if let Res::Local(id) = qpath_res(self.cx, qpath, ptr.hir_id) { + if let Res::Local(id) = self.cx.qpath_res(qpath, ptr.hir_id) { if self.ptrs.contains(&id) { span_lint( self.cx, @@ -722,7 +722,7 @@ fn is_mutated_static(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> bool { use hir::ExprKind::{Field, Index, Path}; match e.kind { - Path(ref qpath) => !matches!(qpath_res(cx, qpath, e.hir_id), Res::Local(_)), + Path(ref qpath) => !matches!(cx.qpath_res(qpath, e.hir_id), Res::Local(_)), Field(ref inner, _) | Index(ref inner, _) => is_mutated_static(cx, inner), _ => false, } diff --git a/src/tools/clippy/clippy_lints/src/let_if_seq.rs b/src/tools/clippy/clippy_lints/src/let_if_seq.rs index db717cd1240a4..5886c2360e362 100644 --- a/src/tools/clippy/clippy_lints/src/let_if_seq.rs +++ b/src/tools/clippy/clippy_lints/src/let_if_seq.rs @@ -1,4 +1,4 @@ -use crate::utils::{qpath_res, snippet, span_lint_and_then, visitors::LocalUsedVisitor}; +use crate::utils::{snippet, span_lint_and_then, visitors::LocalUsedVisitor}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; @@ -145,7 +145,7 @@ fn check_assign<'tcx>( if let hir::StmtKind::Semi(ref expr) = expr.kind; if let hir::ExprKind::Assign(ref var, ref value, _) = expr.kind; if let hir::ExprKind::Path(ref qpath) = var.kind; - if let Res::Local(local_id) = qpath_res(cx, qpath, var.hir_id); + if let Res::Local(local_id) = cx.qpath_res(qpath, var.hir_id); if decl == local_id; then { let mut v = LocalUsedVisitor::new(decl); diff --git a/src/tools/clippy/clippy_lints/src/loops.rs b/src/tools/clippy/clippy_lints/src/loops.rs index 1c5ab2874b048..1ae2c6ca95788 100644 --- a/src/tools/clippy/clippy_lints/src/loops.rs +++ b/src/tools/clippy/clippy_lints/src/loops.rs @@ -6,9 +6,9 @@ use crate::utils::visitors::LocalUsedVisitor; use crate::utils::{ contains_name, get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait, indent_of, is_in_panic_handler, is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, - last_path_segment, match_trait_method, match_type, match_var, multispan_sugg, qpath_res, single_segment_path, - snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, - span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq, + last_path_segment, match_trait_method, match_type, match_var, multispan_sugg, single_segment_path, snippet, + snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, + span_lint_and_then, sugg, SpanlessEq, }; use if_chain::if_chain; use rustc_ast::ast; @@ -848,7 +848,7 @@ fn same_var<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, var: HirId) -> bool { if let ExprKind::Path(qpath) = &expr.kind; if let QPath::Resolved(None, path) = qpath; if path.segments.len() == 1; - if let Res::Local(local_id) = qpath_res(cx, qpath, expr.hir_id); + if let Res::Local(local_id) = cx.qpath_res(qpath, expr.hir_id); then { // our variable! local_id == var @@ -1420,7 +1420,7 @@ fn detect_same_item_push<'tcx>( // Make sure that the push does not involve possibly mutating values match pushed_item.kind { ExprKind::Path(ref qpath) => { - match qpath_res(cx, qpath, pushed_item.hir_id) { + match cx.qpath_res(qpath, pushed_item.hir_id) { // immutable bindings that are initialized with literal or constant Res::Local(hir_id) => { if_chain! { @@ -1437,7 +1437,7 @@ fn detect_same_item_push<'tcx>( ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item), // immutable bindings that are initialized with constant ExprKind::Path(ref path) => { - if let Res::Def(DefKind::Const, ..) = qpath_res(cx, path, init.hir_id) { + if let Res::Def(DefKind::Const, ..) = cx.qpath_res(path, init.hir_id) { emit_lint(cx, vec, pushed_item); } } @@ -2028,7 +2028,7 @@ fn check_for_mutability(cx: &LateContext<'_>, bound: &Expr<'_>) -> Option if let ExprKind::Path(ref qpath) = bound.kind; if let QPath::Resolved(None, _) = *qpath; then { - let res = qpath_res(cx, qpath, bound.hir_id); + let res = cx.qpath_res(qpath, bound.hir_id); if let Res::Local(hir_id) = res { let node_str = cx.tcx.hir().get(hir_id); if_chain! { @@ -2120,7 +2120,7 @@ impl<'a, 'tcx> VarVisitor<'a, 'tcx> { if self.prefer_mutable { self.indexed_mut.insert(seqvar.segments[0].ident.name); } - let res = qpath_res(self.cx, seqpath, seqexpr.hir_id); + let res = self.cx.qpath_res(seqpath, seqexpr.hir_id); match res { Res::Local(hir_id) => { let parent_id = self.cx.tcx.hir().get_parent_item(expr.hir_id); @@ -2184,7 +2184,7 @@ impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> { if let QPath::Resolved(None, ref path) = *qpath; if path.segments.len() == 1; then { - if let Res::Local(local_id) = qpath_res(self.cx, qpath, expr.hir_id) { + if let Res::Local(local_id) = self.cx.qpath_res(qpath, expr.hir_id) { if local_id == self.var { self.nonindex = true; } else { @@ -2589,7 +2589,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> { fn var_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { if let ExprKind::Path(ref qpath) = expr.kind { - let path_res = qpath_res(cx, qpath, expr.hir_id); + let path_res = cx.qpath_res(qpath, expr.hir_id); if let Res::Local(hir_id) = path_res { return Some(hir_id); } @@ -2819,7 +2819,7 @@ impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> { if_chain! { if let ExprKind::Path(ref qpath) = ex.kind; if let QPath::Resolved(None, _) = *qpath; - let res = qpath_res(self.cx, qpath, ex.hir_id); + let res = self.cx.qpath_res(qpath, ex.hir_id); then { match res { Res::Local(hir_id) => { diff --git a/src/tools/clippy/clippy_lints/src/manual_strip.rs b/src/tools/clippy/clippy_lints/src/manual_strip.rs index a0cfe145a301c..42a92104a4919 100644 --- a/src/tools/clippy/clippy_lints/src/manual_strip.rs +++ b/src/tools/clippy/clippy_lints/src/manual_strip.rs @@ -1,7 +1,7 @@ use crate::consts::{constant, Constant}; use crate::utils::usage::mutated_variables; use crate::utils::{ - eq_expr_value, higher, match_def_path, meets_msrv, multispan_sugg, paths, qpath_res, snippet, span_lint_and_then, + eq_expr_value, higher, match_def_path, meets_msrv, multispan_sugg, paths, snippet, span_lint_and_then, }; use if_chain::if_chain; @@ -92,7 +92,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualStrip { } else { return; }; - let target_res = qpath_res(cx, &target_path, target_arg.hir_id); + let target_res = cx.qpath_res(&target_path, target_arg.hir_id); if target_res == Res::Err { return; }; @@ -221,7 +221,7 @@ fn find_stripping<'tcx>( if let ExprKind::Index(indexed, index) = &unref.kind; if let Some(higher::Range { start, end, .. }) = higher::range(index); if let ExprKind::Path(path) = &indexed.kind; - if qpath_res(self.cx, path, ex.hir_id) == self.target; + if self.cx.qpath_res(path, ex.hir_id) == self.target; then { match (self.strip_kind, start, end) { (StripKind::Prefix, Some(start), None) => { @@ -235,7 +235,7 @@ fn find_stripping<'tcx>( if let ExprKind::Binary(Spanned { node: BinOpKind::Sub, .. }, left, right) = end.kind; if let Some(left_arg) = len_arg(self.cx, left); if let ExprKind::Path(left_path) = &left_arg.kind; - if qpath_res(self.cx, left_path, left_arg.hir_id) == self.target; + if self.cx.qpath_res(left_path, left_arg.hir_id) == self.target; if eq_pattern_length(self.cx, self.pattern, right); then { self.results.push(ex.span); diff --git a/src/tools/clippy/clippy_lints/src/mem_forget.rs b/src/tools/clippy/clippy_lints/src/mem_forget.rs index 8c6fd10f98a1e..d34f9761e26f9 100644 --- a/src/tools/clippy/clippy_lints/src/mem_forget.rs +++ b/src/tools/clippy/clippy_lints/src/mem_forget.rs @@ -1,4 +1,4 @@ -use crate::utils::{match_def_path, paths, qpath_res, span_lint}; +use crate::utils::{match_def_path, paths, span_lint}; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -29,7 +29,7 @@ impl<'tcx> LateLintPass<'tcx> for MemForget { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { if let ExprKind::Call(ref path_expr, ref args) = e.kind { if let ExprKind::Path(ref qpath) = path_expr.kind { - if let Some(def_id) = qpath_res(cx, qpath, path_expr.hir_id).opt_def_id() { + if let Some(def_id) = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id() { if match_def_path(cx, def_id, &paths::MEM_FORGET) { let forgot_ty = cx.typeck_results().expr_ty(&args[0]); diff --git a/src/tools/clippy/clippy_lints/src/no_effect.rs b/src/tools/clippy/clippy_lints/src/no_effect.rs index b1b5b3439a0e3..69302d695ce0a 100644 --- a/src/tools/clippy/clippy_lints/src/no_effect.rs +++ b/src/tools/clippy/clippy_lints/src/no_effect.rs @@ -1,4 +1,4 @@ -use crate::utils::{has_drop, qpath_res, snippet_opt, span_lint, span_lint_and_sugg}; +use crate::utils::{has_drop, snippet_opt, span_lint, span_lint_and_sugg}; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::{BinOpKind, BlockCheckMode, Expr, ExprKind, Stmt, StmtKind, UnsafeSource}; @@ -67,7 +67,7 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { }, ExprKind::Call(ref callee, ref args) => { if let ExprKind::Path(ref qpath) = callee.kind { - let res = qpath_res(cx, qpath, callee.hir_id); + let res = cx.qpath_res(qpath, callee.hir_id); match res { Res::Def(DefKind::Struct | DefKind::Variant | DefKind::Ctor(..), ..) => { !has_drop(cx, cx.typeck_results().expr_ty(expr)) @@ -146,7 +146,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option { if let ExprKind::Path(ref qpath) = callee.kind { - let res = qpath_res(cx, qpath, callee.hir_id); + let res = cx.qpath_res(qpath, callee.hir_id); match res { Res::Def(DefKind::Struct | DefKind::Variant | DefKind::Ctor(..), ..) if !has_drop(cx, cx.typeck_results().expr_ty(expr)) => diff --git a/src/tools/clippy/clippy_lints/src/non_copy_const.rs b/src/tools/clippy/clippy_lints/src/non_copy_const.rs index 3a9aa6ced03ba..f57d753631755 100644 --- a/src/tools/clippy/clippy_lints/src/non_copy_const.rs +++ b/src/tools/clippy/clippy_lints/src/non_copy_const.rs @@ -18,7 +18,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{InnerSpan, Span, DUMMY_SP}; use rustc_typeck::hir_ty_to_ty; -use crate::utils::{in_constant, qpath_res, span_lint_and_then}; +use crate::utils::{in_constant, span_lint_and_then}; use if_chain::if_chain; // FIXME: this is a correctness problem but there's no suitable @@ -339,7 +339,7 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst { } // Make sure it is a const item. - let item_def_id = match qpath_res(cx, qpath, expr.hir_id) { + let item_def_id = match cx.qpath_res(qpath, expr.hir_id) { Res::Def(DefKind::Const | DefKind::AssocConst, did) => did, _ => return, }; diff --git a/src/tools/clippy/clippy_lints/src/to_string_in_display.rs b/src/tools/clippy/clippy_lints/src/to_string_in_display.rs index c53727ba16004..fa508df865e48 100644 --- a/src/tools/clippy/clippy_lints/src/to_string_in_display.rs +++ b/src/tools/clippy/clippy_lints/src/to_string_in_display.rs @@ -1,4 +1,4 @@ -use crate::utils::{match_def_path, match_trait_method, paths, qpath_res, span_lint}; +use crate::utils::{match_def_path, match_trait_method, paths, span_lint}; use if_chain::if_chain; use rustc_hir::def::Res; use rustc_hir::{Expr, ExprKind, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind}; @@ -94,7 +94,7 @@ impl LateLintPass<'_> for ToStringInDisplay { if match_trait_method(cx, expr, &paths::TO_STRING); if self.in_display_impl; if let ExprKind::Path(ref qpath) = args[0].kind; - if let Res::Local(hir_id) = qpath_res(cx, qpath, args[0].hir_id); + if let Res::Local(hir_id) = cx.qpath_res(qpath, args[0].hir_id); if let Some(self_hir_id) = self.self_hir_id; if hir_id == self_hir_id; then { diff --git a/src/tools/clippy/clippy_lints/src/types.rs b/src/tools/clippy/clippy_lints/src/types.rs index 3b5a83d2a0bec..624ea16f585d2 100644 --- a/src/tools/clippy/clippy_lints/src/types.rs +++ b/src/tools/clippy/clippy_lints/src/types.rs @@ -34,7 +34,7 @@ use crate::utils::sugg::Sugg; use crate::utils::{ clip, comparisons, differing_macro_contexts, higher, in_constant, indent_of, int_bits, is_hir_ty_cfg_dependant, is_type_diagnostic_item, last_path_segment, match_def_path, match_path, meets_msrv, method_chain_args, - multispan_sugg, numeric_literal::NumericLiteral, qpath_res, reindent_multiline, sext, snippet, snippet_opt, + multispan_sugg, numeric_literal::NumericLiteral, reindent_multiline, sext, snippet, snippet_opt, snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext, }; @@ -298,7 +298,7 @@ fn match_type_parameter(cx: &LateContext<'_>, qpath: &QPath<'_>, path: &[&str]) _ => None, }); if let TyKind::Path(ref qpath) = ty.kind; - if let Some(did) = qpath_res(cx, qpath, ty.hir_id).opt_def_id(); + if let Some(did) = cx.qpath_res(qpath, ty.hir_id).opt_def_id(); if match_def_path(cx, did, path); then { return Some(ty.span); @@ -365,7 +365,7 @@ impl Types { match hir_ty.kind { TyKind::Path(ref qpath) if !is_local => { let hir_id = hir_ty.hir_id; - let res = qpath_res(cx, qpath, hir_id); + let res = cx.qpath_res(qpath, hir_id); if let Some(def_id) = res.opt_def_id() { if Some(def_id) == cx.tcx.lang_items().owned_box() { if let Some(span) = match_borrows_parameter(cx, qpath) { @@ -535,7 +535,7 @@ impl Types { }); // ty is now _ at this point if let TyKind::Path(ref ty_qpath) = ty.kind; - let res = qpath_res(cx, ty_qpath, ty.hir_id); + let res = cx.qpath_res(ty_qpath, ty.hir_id); if let Some(def_id) = res.opt_def_id(); if Some(def_id) == cx.tcx.lang_items().owned_box(); // At this point, we know ty is Box, now get T @@ -652,7 +652,7 @@ impl Types { match mut_ty.ty.kind { TyKind::Path(ref qpath) => { let hir_id = mut_ty.ty.hir_id; - let def = qpath_res(cx, qpath, hir_id); + let def = cx.qpath_res(qpath, hir_id); if_chain! { if let Some(def_id) = def.opt_def_id(); if Some(def_id) == cx.tcx.lang_items().owned_box(); @@ -739,7 +739,7 @@ fn is_any_trait(t: &hir::Ty<'_>) -> bool { fn get_bounds_if_impl_trait<'tcx>(cx: &LateContext<'tcx>, qpath: &QPath<'_>, id: HirId) -> Option> { if_chain! { - if let Some(did) = qpath_res(cx, qpath, id).opt_def_id(); + if let Some(did) = cx.qpath_res(qpath, id).opt_def_id(); if let Some(Node::GenericParam(generic_param)) = cx.tcx.hir().get_if_local(did); if let GenericParamKind::Type { synthetic, .. } = generic_param.kind; if synthetic == Some(SyntheticTyParamKind::ImplTrait); diff --git a/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs b/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs index 7aa17520ba79f..822863ca3e279 100644 --- a/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs +++ b/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs @@ -1,7 +1,7 @@ use crate::consts::{constant_simple, Constant}; use crate::utils::{ - is_expn_of, match_def_path, match_qpath, match_type, method_calls, path_to_res, paths, qpath_res, run_lints, - snippet, span_lint, span_lint_and_help, span_lint_and_sugg, SpanlessEq, + is_expn_of, match_def_path, match_qpath, match_type, method_calls, path_to_res, paths, run_lints, snippet, + span_lint, span_lint_and_help, span_lint_and_sugg, SpanlessEq, }; use if_chain::if_chain; use rustc_ast::ast::{Crate as AstCrate, ItemKind, LitKind, NodeId}; @@ -787,7 +787,7 @@ fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option return path_to_matched_type(cx, expr), - ExprKind::Path(qpath) => match qpath_res(cx, qpath, expr.hir_id) { + ExprKind::Path(qpath) => match cx.qpath_res(qpath, expr.hir_id) { Res::Local(hir_id) => { let parent_id = cx.tcx.hir().get_parent_node(hir_id); if let Some(Node::Local(local)) = cx.tcx.hir().find(parent_id) { diff --git a/src/tools/clippy/clippy_lints/src/utils/mod.rs b/src/tools/clippy/clippy_lints/src/utils/mod.rs index 9b262517a9834..023fb0a7112c1 100644 --- a/src/tools/clippy/clippy_lints/src/utils/mod.rs +++ b/src/tools/clippy/clippy_lints/src/utils/mod.rs @@ -370,19 +370,6 @@ pub fn path_to_res(cx: &LateContext<'_>, path: &[&str]) -> Option { } } -pub fn qpath_res(cx: &LateContext<'_>, qpath: &hir::QPath<'_>, id: hir::HirId) -> Res { - match qpath { - hir::QPath::Resolved(_, path) => path.res, - hir::QPath::TypeRelative(..) | hir::QPath::LangItem(..) => { - if cx.tcx.has_typeck_results(id.owner.to_def_id()) { - cx.tcx.typeck(id.owner).qpath_res(qpath, id) - } else { - Res::Err - } - }, - } -} - /// Convenience function to get the `DefId` of a trait by path. /// It could be a trait or trait alias. pub fn get_trait_def_id(cx: &LateContext<'_>, path: &[&str]) -> Option { From 26b4baf46ea66d3651281c8b66d76853e6362c65 Mon Sep 17 00:00:00 2001 From: 1000teslas <47207223+1000teslas@users.noreply.github.com> Date: Mon, 18 Jan 2021 19:04:55 +1100 Subject: [PATCH 06/17] Point to span of upvar making closure FnMut Add expected error Add comment Tweak comment wording Fix after rebase to updated master Fix after rebase to updated master Distinguish mutation in normal and move closures Tweak error message Fix error message for nested closures Refactor code showing mutated upvar in closure Remove debug assert B --- .../diagnostics/mutability_errors.rs | 56 ++++++++++++++++++- .../borrow-raw-address-of-mutability.stderr | 4 +- .../issue-80313-mutable-borrow-in-closure.rs | 7 +++ ...sue-80313-mutable-borrow-in-closure.stderr | 14 +++++ ...ue-80313-mutable-borrow-in-move-closure.rs | 7 +++ ...0313-mutable-borrow-in-move-closure.stderr | 14 +++++ .../issue-80313-mutation-in-closure.rs | 7 +++ .../issue-80313-mutation-in-closure.stderr | 14 +++++ .../issue-80313-mutation-in-move-closure.rs | 7 +++ ...ssue-80313-mutation-in-move-closure.stderr | 14 +++++ ...es-infer-fnmut-calling-fnmut-no-mut.stderr | 4 ++ ...ed-closures-infer-fnmut-missing-mut.stderr | 4 +- ...osures-infer-fnmut-move-missing-mut.stderr | 4 +- 13 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/closures/issue-80313-mutable-borrow-in-closure.rs create mode 100644 src/test/ui/closures/issue-80313-mutable-borrow-in-closure.stderr create mode 100644 src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.rs create mode 100644 src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.stderr create mode 100644 src/test/ui/closures/issue-80313-mutation-in-closure.rs create mode 100644 src/test/ui/closures/issue-80313-mutation-in-closure.stderr create mode 100644 src/test/ui/closures/issue-80313-mutation-in-move-closure.rs create mode 100644 src/test/ui/closures/issue-80313-mutation-in-move-closure.stderr diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs index e1af6fc07cf8f..73196c732f5bb 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs @@ -1,9 +1,12 @@ use rustc_hir as hir; use rustc_hir::Node; use rustc_index::vec::Idx; -use rustc_middle::mir::{self, ClearCrossCrate, Local, LocalDecl, LocalInfo, Location}; use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem}; use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_middle::{ + hir::place::PlaceBase, + mir::{self, ClearCrossCrate, Local, LocalDecl, LocalInfo, Location}, +}; use rustc_span::source_map::DesugaringKind; use rustc_span::symbol::{kw, Symbol}; use rustc_span::Span; @@ -241,6 +244,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { format!("mut {}", self.local_names[local].unwrap()), Applicability::MachineApplicable, ); + let tcx = self.infcx.tcx; + if let ty::Closure(id, _) = the_place_err.ty(self.body, tcx).ty.kind() { + self.show_mutating_upvar(tcx, id, the_place_err, &mut err); + } } // Also suggest adding mut for upvars @@ -271,6 +278,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { ); } } + + let tcx = self.infcx.tcx; + if let ty::Ref(_, ty, Mutability::Mut) = the_place_err.ty(self.body, tcx).ty.kind() + { + if let ty::Closure(id, _) = ty.kind() { + self.show_mutating_upvar(tcx, id, the_place_err, &mut err); + } + } } // complete hack to approximate old AST-borrowck @@ -463,6 +478,45 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { err.buffer(&mut self.errors_buffer); } + // point to span of upvar making closure call require mutable borrow + fn show_mutating_upvar( + &self, + tcx: TyCtxt<'_>, + id: &hir::def_id::DefId, + the_place_err: PlaceRef<'tcx>, + err: &mut DiagnosticBuilder<'_>, + ) { + let id = id.expect_local(); + let tables = tcx.typeck(id); + let hir_id = tcx.hir().local_def_id_to_hir_id(id); + let (span, place) = &tables.closure_kind_origins()[hir_id]; + let reason = if let PlaceBase::Upvar(upvar_id) = place.base { + let upvar = ty::place_to_string_for_capture(tcx, place); + match tables.upvar_capture(upvar_id) { + ty::UpvarCapture::ByRef(ty::UpvarBorrow { + kind: ty::BorrowKind::MutBorrow, + .. + }) => { + format!("mutable borrow of `{}`", upvar) + } + ty::UpvarCapture::ByValue(_) => { + format!("possible mutation of `{}`", upvar) + } + _ => bug!("upvar `{}` borrowed, but not mutably", upvar), + } + } else { + bug!("not an upvar") + }; + err.span_label( + *span, + format!( + "calling `{}` requires mutable binding due to {}", + self.describe_place(the_place_err).unwrap(), + reason + ), + ); + } + /// Targeted error when encountering an `FnMut` closure where an `Fn` closure was expected. fn expected_fn_found_fn_mut_call(&self, err: &mut DiagnosticBuilder<'_>, sp: Span, act: &str) { err.span_label(sp, format!("cannot {}", act)); diff --git a/src/test/ui/borrowck/borrow-raw-address-of-mutability.stderr b/src/test/ui/borrowck/borrow-raw-address-of-mutability.stderr index 44dde0fd80b0d..ea74fb966846f 100644 --- a/src/test/ui/borrowck/borrow-raw-address-of-mutability.stderr +++ b/src/test/ui/borrowck/borrow-raw-address-of-mutability.stderr @@ -20,7 +20,9 @@ error[E0596]: cannot borrow `f` as mutable, as it is not declared as mutable | LL | let f = || { | - help: consider changing this to be mutable: `mut f` -... +LL | let y = &raw mut x; + | - calling `f` requires mutable binding due to mutable borrow of `x` +LL | }; LL | f(); | ^ cannot borrow as mutable diff --git a/src/test/ui/closures/issue-80313-mutable-borrow-in-closure.rs b/src/test/ui/closures/issue-80313-mutable-borrow-in-closure.rs new file mode 100644 index 0000000000000..ff210ae06a3bd --- /dev/null +++ b/src/test/ui/closures/issue-80313-mutable-borrow-in-closure.rs @@ -0,0 +1,7 @@ +fn main() { + let mut my_var = false; + let callback = || { + &mut my_var; + }; + callback(); //~ ERROR E0596 +} diff --git a/src/test/ui/closures/issue-80313-mutable-borrow-in-closure.stderr b/src/test/ui/closures/issue-80313-mutable-borrow-in-closure.stderr new file mode 100644 index 0000000000000..bf9e1febdbba4 --- /dev/null +++ b/src/test/ui/closures/issue-80313-mutable-borrow-in-closure.stderr @@ -0,0 +1,14 @@ +error[E0596]: cannot borrow `callback` as mutable, as it is not declared as mutable + --> $DIR/issue-80313-mutable-borrow-in-closure.rs:6:5 + | +LL | let callback = || { + | -------- help: consider changing this to be mutable: `mut callback` +LL | &mut my_var; + | ------ calling `callback` requires mutable binding due to mutable borrow of `my_var` +LL | }; +LL | callback(); + | ^^^^^^^^ cannot borrow as mutable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.rs b/src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.rs new file mode 100644 index 0000000000000..8f2d8a676302c --- /dev/null +++ b/src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.rs @@ -0,0 +1,7 @@ +fn main() { + let mut my_var = false; + let callback = move || { + &mut my_var; + }; + callback(); //~ ERROR E0596 +} diff --git a/src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.stderr b/src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.stderr new file mode 100644 index 0000000000000..b67cec6a609f0 --- /dev/null +++ b/src/test/ui/closures/issue-80313-mutable-borrow-in-move-closure.stderr @@ -0,0 +1,14 @@ +error[E0596]: cannot borrow `callback` as mutable, as it is not declared as mutable + --> $DIR/issue-80313-mutable-borrow-in-move-closure.rs:6:5 + | +LL | let callback = move || { + | -------- help: consider changing this to be mutable: `mut callback` +LL | &mut my_var; + | ------ calling `callback` requires mutable binding due to possible mutation of `my_var` +LL | }; +LL | callback(); + | ^^^^^^^^ cannot borrow as mutable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/closures/issue-80313-mutation-in-closure.rs b/src/test/ui/closures/issue-80313-mutation-in-closure.rs new file mode 100644 index 0000000000000..e082ea562ef22 --- /dev/null +++ b/src/test/ui/closures/issue-80313-mutation-in-closure.rs @@ -0,0 +1,7 @@ +fn main() { + let mut my_var = false; + let callback = || { + my_var = true; + }; + callback(); //~ ERROR E0596 +} diff --git a/src/test/ui/closures/issue-80313-mutation-in-closure.stderr b/src/test/ui/closures/issue-80313-mutation-in-closure.stderr new file mode 100644 index 0000000000000..6e98549f6b84f --- /dev/null +++ b/src/test/ui/closures/issue-80313-mutation-in-closure.stderr @@ -0,0 +1,14 @@ +error[E0596]: cannot borrow `callback` as mutable, as it is not declared as mutable + --> $DIR/issue-80313-mutation-in-closure.rs:6:5 + | +LL | let callback = || { + | -------- help: consider changing this to be mutable: `mut callback` +LL | my_var = true; + | ------ calling `callback` requires mutable binding due to mutable borrow of `my_var` +LL | }; +LL | callback(); + | ^^^^^^^^ cannot borrow as mutable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/closures/issue-80313-mutation-in-move-closure.rs b/src/test/ui/closures/issue-80313-mutation-in-move-closure.rs new file mode 100644 index 0000000000000..f66bf4e062831 --- /dev/null +++ b/src/test/ui/closures/issue-80313-mutation-in-move-closure.rs @@ -0,0 +1,7 @@ +fn main() { + let mut my_var = false; + let callback = move || { + my_var = true; + }; + callback(); //~ ERROR E0596 +} diff --git a/src/test/ui/closures/issue-80313-mutation-in-move-closure.stderr b/src/test/ui/closures/issue-80313-mutation-in-move-closure.stderr new file mode 100644 index 0000000000000..edd55422a0bd4 --- /dev/null +++ b/src/test/ui/closures/issue-80313-mutation-in-move-closure.stderr @@ -0,0 +1,14 @@ +error[E0596]: cannot borrow `callback` as mutable, as it is not declared as mutable + --> $DIR/issue-80313-mutation-in-move-closure.rs:6:5 + | +LL | let callback = move || { + | -------- help: consider changing this to be mutable: `mut callback` +LL | my_var = true; + | ------ calling `callback` requires mutable binding due to possible mutation of `my_var` +LL | }; +LL | callback(); + | ^^^^^^^^ cannot borrow as mutable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.stderr b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.stderr index 5dea424596e9c..a0ed56d4bcf7b 100644 --- a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.stderr +++ b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-calling-fnmut-no-mut.stderr @@ -3,6 +3,8 @@ error[E0596]: cannot borrow `tick1` as mutable, as it is not declared as mutable | LL | let tick1 = || { | ----- help: consider changing this to be mutable: `mut tick1` +LL | counter += 1; + | ------- calling `tick1` requires mutable binding due to mutable borrow of `counter` ... LL | tick1(); | ^^^^^ cannot borrow as mutable @@ -12,6 +14,8 @@ error[E0596]: cannot borrow `tick2` as mutable, as it is not declared as mutable | LL | let tick2 = || { | ----- help: consider changing this to be mutable: `mut tick2` +LL | tick1(); + | ----- calling `tick2` requires mutable binding due to mutable borrow of `tick1` ... LL | tick2(); | ^^^^^ cannot borrow as mutable diff --git a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-missing-mut.stderr b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-missing-mut.stderr index eb398628846dd..27d23e3fa044b 100644 --- a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-missing-mut.stderr +++ b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-missing-mut.stderr @@ -2,7 +2,9 @@ error[E0596]: cannot borrow `tick` as mutable, as it is not declared as mutable --> $DIR/unboxed-closures-infer-fnmut-missing-mut.rs:7:5 | LL | let tick = || counter += 1; - | ---- help: consider changing this to be mutable: `mut tick` + | ---- ------- calling `tick` requires mutable binding due to mutable borrow of `counter` + | | + | help: consider changing this to be mutable: `mut tick` LL | tick(); | ^^^^ cannot borrow as mutable diff --git a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-move-missing-mut.stderr b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-move-missing-mut.stderr index b9d76d9a752ce..c00f986c397a7 100644 --- a/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-move-missing-mut.stderr +++ b/src/test/ui/unboxed-closures/unboxed-closures-infer-fnmut-move-missing-mut.stderr @@ -2,7 +2,9 @@ error[E0596]: cannot borrow `tick` as mutable, as it is not declared as mutable --> $DIR/unboxed-closures-infer-fnmut-move-missing-mut.rs:7:5 | LL | let tick = move || counter += 1; - | ---- help: consider changing this to be mutable: `mut tick` + | ---- ------- calling `tick` requires mutable binding due to possible mutation of `counter` + | | + | help: consider changing this to be mutable: `mut tick` LL | tick(); | ^^^^ cannot borrow as mutable From d00c8502afca04c64cebecfcaec1203d5a0fba69 Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Wed, 20 Jan 2021 16:54:58 +0100 Subject: [PATCH 07/17] BTreeMap: correct tests for alternative choices of B --- library/alloc/benches/btree/map.rs | 78 +------------------ .../alloc/src/collections/btree/map/tests.rs | 5 +- .../alloc/src/collections/btree/node/tests.rs | 6 +- 3 files changed, 8 insertions(+), 81 deletions(-) diff --git a/library/alloc/benches/btree/map.rs b/library/alloc/benches/btree/map.rs index 7c2e5694a62fc..ccb04d09180d3 100644 --- a/library/alloc/benches/btree/map.rs +++ b/library/alloc/benches/btree/map.rs @@ -283,6 +283,8 @@ pub fn iter_1m(b: &mut Bencher) { bench_iter(b, 1_000, 1_000_000); } +// On Windows, this is a factor 11 away from causing stack overflow, +// where node size grows to roughly `node::CAPACITY * FAT * 8` = 300 KB. const FAT: usize = 256; // The returned map has small keys and values. @@ -296,11 +298,6 @@ fn fat_val_map(n: usize) -> BTreeMap { (0..n).map(|i| (i, [i; FAT])).collect::>() } -// The returned map has large keys and values. -fn fat_map(n: usize) -> BTreeMap<[usize; FAT], [usize; FAT]> { - (0..n).map(|i| ([i; FAT], [i; FAT])).collect::>() -} - #[bench] pub fn clone_slim_100(b: &mut Bencher) { let src = slim_map(100); @@ -513,74 +510,3 @@ pub fn clone_fat_val_100_and_remove_half(b: &mut Bencher) { map }) } - -#[bench] -pub fn clone_fat_100(b: &mut Bencher) { - let src = fat_map(100); - b.iter(|| src.clone()) -} - -#[bench] -pub fn clone_fat_100_and_clear(b: &mut Bencher) { - let src = fat_map(100); - b.iter(|| src.clone().clear()) -} - -#[bench] -pub fn clone_fat_100_and_drain_all(b: &mut Bencher) { - let src = fat_map(100); - b.iter(|| src.clone().drain_filter(|_, _| true).count()) -} - -#[bench] -pub fn clone_fat_100_and_drain_half(b: &mut Bencher) { - let src = fat_map(100); - b.iter(|| { - let mut map = src.clone(); - assert_eq!(map.drain_filter(|i, _| i[0] % 2 == 0).count(), 100 / 2); - assert_eq!(map.len(), 100 / 2); - }) -} - -#[bench] -pub fn clone_fat_100_and_into_iter(b: &mut Bencher) { - let src = fat_map(100); - b.iter(|| src.clone().into_iter().count()) -} - -#[bench] -pub fn clone_fat_100_and_pop_all(b: &mut Bencher) { - let src = fat_map(100); - b.iter(|| { - let mut map = src.clone(); - while map.pop_first().is_some() {} - map - }); -} - -#[bench] -pub fn clone_fat_100_and_remove_all(b: &mut Bencher) { - let src = fat_map(100); - b.iter(|| { - let mut map = src.clone(); - while let Some(elt) = map.iter().map(|(&i, _)| i).next() { - let v = map.remove(&elt); - debug_assert!(v.is_some()); - } - map - }); -} - -#[bench] -pub fn clone_fat_100_and_remove_half(b: &mut Bencher) { - let src = fat_map(100); - b.iter(|| { - let mut map = src.clone(); - for i in (0..100).step_by(2) { - let v = map.remove(&[i; FAT]); - debug_assert!(v.is_some()); - } - assert_eq!(map.len(), 100 / 2); - map - }) -} diff --git a/library/alloc/src/collections/btree/map/tests.rs b/library/alloc/src/collections/btree/map/tests.rs index f92aed8ce15bf..dcf88daae31ba 100644 --- a/library/alloc/src/collections/btree/map/tests.rs +++ b/library/alloc/src/collections/btree/map/tests.rs @@ -137,8 +137,9 @@ impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::LeafOrInternal> } } -// Tests our value of MIN_INSERTS_HEIGHT_2. It may change according to the -// implementation of insertion, but it's best to be aware of when it does. +// Tests our value of MIN_INSERTS_HEIGHT_2. Failure may mean you just need to +// adapt that value to match a change in node::CAPACITY or the choices made +// during insertion, otherwise other test cases may fail or be less useful. #[test] fn test_levels() { let mut map = BTreeMap::new(); diff --git a/library/alloc/src/collections/btree/node/tests.rs b/library/alloc/src/collections/btree/node/tests.rs index 48ce9f2bd89c8..43d24809251dc 100644 --- a/library/alloc/src/collections/btree/node/tests.rs +++ b/library/alloc/src/collections/btree/node/tests.rs @@ -113,7 +113,7 @@ fn test_partial_cmp_eq() { #[cfg(target_arch = "x86_64")] fn test_sizes() { assert_eq!(core::mem::size_of::>(), 16); - assert_eq!(core::mem::size_of::>(), 16 + CAPACITY * 8 * 2); - assert_eq!(core::mem::size_of::>(), 112); - assert_eq!(core::mem::size_of::>(), 112 + CAPACITY * 8 * 2); + assert_eq!(core::mem::size_of::>(), 16 + CAPACITY * 2 * 8); + assert_eq!(core::mem::size_of::>(), 16 + (CAPACITY + 1) * 8); + assert_eq!(core::mem::size_of::>(), 16 + (CAPACITY * 3 + 1) * 8); } From d9bee1c735c2a1c423a7a7492abb7e1e74ce4d7b Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Thu, 27 Aug 2020 11:49:18 -0400 Subject: [PATCH 08/17] 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. ### 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 | 33 +++++++++ .../rustc_codegen_cranelift/src/abi/mod.rs | 8 +-- compiler/rustc_feature/src/active.rs | 4 ++ 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 | 69 ++++++++++++++----- 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 ++++ .../c-unwind/feature-gate-c-unwind-enabled.rs | 12 ++++ src/test/ui/c-unwind/feature-gate-c-unwind.rs | 9 +++ .../ui/c-unwind/feature-gate-c-unwind.stderr | 12 ++++ .../c-unwind/feature-gate-stdcall-unwind.rs | 9 +++ .../feature-gate-stdcall-unwind.stderr | 12 ++++ .../ui/c-unwind/feature-gate-system-unwind.rs | 9 +++ .../feature-gate-system-unwind.stderr | 12 ++++ .../c-unwind/feature-gate-thiscall-unwind.rs | 9 +++ .../feature-gate-thiscall-unwind.stderr | 12 ++++ src/test/ui/codemap_tests/unicode.stderr | 2 +- src/test/ui/parser/issue-8537.stderr | 2 +- src/test/ui/symbol-names/impl1.rs | 6 +- 29 files changed, 270 insertions(+), 60 deletions(-) create mode 100644 src/doc/unstable-book/src/language-features/c-unwind.md create mode 100644 src/test/ui/c-unwind/feature-gate-c-unwind-enabled.rs create mode 100644 src/test/ui/c-unwind/feature-gate-c-unwind.rs create mode 100644 src/test/ui/c-unwind/feature-gate-c-unwind.stderr create mode 100644 src/test/ui/c-unwind/feature-gate-stdcall-unwind.rs create mode 100644 src/test/ui/c-unwind/feature-gate-stdcall-unwind.stderr create mode 100644 src/test/ui/c-unwind/feature-gate-system-unwind.rs create mode 100644 src/test/ui/c-unwind/feature-gate-system-unwind.stderr create mode 100644 src/test/ui/c-unwind/feature-gate-thiscall-unwind.rs create mode 100644 src/test/ui/c-unwind/feature-gate-thiscall-unwind.stderr diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 69257ce1c19e9..994553415b085 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -319,10 +319,10 @@ impl<'hir> LoweringContext<'_, 'hir> { ItemKind::Mod(ref m) => hir::ItemKind::Mod(self.lower_mod(m)), 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))), @@ -1315,8 +1315,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 7bd805f91c857..7b2d5726ae344 100644 --- a/compiler/rustc_ast_passes/src/feature_gate.rs +++ b/compiler/rustc_ast_passes/src/feature_gate.rs @@ -156,6 +156,39 @@ impl<'a> PostExpansionVisitor<'a> { "efiapi ABI is experimental and subject to change" ); } + // FFI Unwinding ABI's + "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 76e1987459f87..ef60009ad3e99 100644 --- a/compiler/rustc_codegen_cranelift/src/abi/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/abi/mod.rs @@ -101,7 +101,7 @@ fn clif_sig_from_fn_sig<'tcx>( requires_caller_location: bool, ) -> Signature { let abi = match sig.abi { - Abi::System => Abi::C, + Abi::System { unwind: false } => Abi::C { unwind: false }, abi => abi, }; let (call_conv, inputs, output): (CallConv, Vec>, Ty<'tcx>) = match abi { @@ -110,7 +110,7 @@ fn clif_sig_from_fn_sig<'tcx>( sig.inputs().to_vec(), sig.output(), ), - Abi::C | Abi::Unadjusted => ( + Abi::C { unwind: false } | Abi::Unadjusted => ( CallConv::triple_default(triple), sig.inputs().to_vec(), sig.output(), @@ -126,7 +126,7 @@ fn clif_sig_from_fn_sig<'tcx>( inputs.extend(extra_args.types()); (CallConv::triple_default(triple), inputs, sig.output()) } - Abi::System => unreachable!(), + Abi::System { .. } => unreachable!(), Abi::RustIntrinsic => ( CallConv::triple_default(triple), sig.inputs().to_vec(), @@ -664,7 +664,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 cd3c8fded633f..c80c4507a6992 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -631,6 +631,10 @@ declare_features! ( /// Allows using `pointer` and `reference` in intra-doc links (active, intra_doc_pointers, "1.51.0", Some(80896), None), + + /// Allows `extern "C-unwind" fn` to enable unwinding across ABI boundaries. + (active, c_unwind, "1.51.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 ef467ed651454..bc32f469d15ce 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2615,14 +2615,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 575667f9a9525..1fdbcfa068c3a 100644 --- a/compiler/rustc_mir/src/interpret/terminator.rs +++ b/compiler/rustc_mir/src/interpret/terminator.rs @@ -244,9 +244,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 7b90e5b611cd1..5100eaef38f6f 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -315,6 +315,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 7b6e6ad0696a1..510a99939c4f8 100644 --- a/compiler/rustc_symbol_mangling/src/v0.rs +++ b/compiler/rustc_symbol_mangling/src/v0.rs @@ -441,7 +441,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 1e45739ca22b4..387813f5d636a 100644 --- a/compiler/rustc_target/src/spec/abi.rs +++ b/compiler/rustc_target/src/spec/abi.rs @@ -8,24 +8,16 @@ 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, @@ -38,7 +30,7 @@ pub enum Abi { AvrNonBlockingInterrupt, // Multiplatform / generic ABIs - System, + System { unwind: bool }, RustIntrinsic, RustCall, PlatformIntrinsic, @@ -60,13 +52,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 }, @@ -82,7 +77,8 @@ const AbiDatas: &[AbiData] = &[ 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 }, @@ -101,7 +97,40 @@ 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, + // Cross-platform ABIs + System { unwind: false } => 20, + System { unwind: true } => 21, + RustIntrinsic => 22, + RustCall => 23, + PlatformIntrinsic => 24, + Unadjusted => 25, + } } #[inline] @@ -120,6 +149,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 b74d80dc6bb2b..01f573313c97f 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 0f9d3c3de1543..110c8dd80ea77 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 90d35efaa25bd..27d96c0c282a1 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -1208,24 +1208,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 3c9c7d578fbd4..15d8e4843f976 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 64cf890037e51..5bcbb2e621bd0 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 d589989511db1..ba86e5601edfa 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2569,7 +2569,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 dde4a62ffbf3d..e4dfec71c4f5b 100644 --- a/compiler/rustc_typeck/src/lib.rs +++ b/compiler/rustc_typeck/src/lib.rs @@ -117,14 +117,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 0000000000000..c1705d59acc21 --- /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/c-unwind/feature-gate-c-unwind-enabled.rs b/src/test/ui/c-unwind/feature-gate-c-unwind-enabled.rs new file mode 100644 index 0000000000000..6ff5dbda2d560 --- /dev/null +++ b/src/test/ui/c-unwind/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/c-unwind/feature-gate-c-unwind.rs b/src/test/ui/c-unwind/feature-gate-c-unwind.rs new file mode 100644 index 0000000000000..f02a368d4e097 --- /dev/null +++ b/src/test/ui/c-unwind/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/c-unwind/feature-gate-c-unwind.stderr b/src/test/ui/c-unwind/feature-gate-c-unwind.stderr new file mode 100644 index 0000000000000..f4c785a235f67 --- /dev/null +++ b/src/test/ui/c-unwind/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/c-unwind/feature-gate-stdcall-unwind.rs b/src/test/ui/c-unwind/feature-gate-stdcall-unwind.rs new file mode 100644 index 0000000000000..86c29d4146e91 --- /dev/null +++ b/src/test/ui/c-unwind/feature-gate-stdcall-unwind.rs @@ -0,0 +1,9 @@ +// 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/c-unwind/feature-gate-stdcall-unwind.stderr b/src/test/ui/c-unwind/feature-gate-stdcall-unwind.stderr new file mode 100644 index 0000000000000..8478811a1ad42 --- /dev/null +++ b/src/test/ui/c-unwind/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:4: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/c-unwind/feature-gate-system-unwind.rs b/src/test/ui/c-unwind/feature-gate-system-unwind.rs new file mode 100644 index 0000000000000..26c2de4e81767 --- /dev/null +++ b/src/test/ui/c-unwind/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/c-unwind/feature-gate-system-unwind.stderr b/src/test/ui/c-unwind/feature-gate-system-unwind.stderr new file mode 100644 index 0000000000000..87877336475b4 --- /dev/null +++ b/src/test/ui/c-unwind/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/c-unwind/feature-gate-thiscall-unwind.rs b/src/test/ui/c-unwind/feature-gate-thiscall-unwind.rs new file mode 100644 index 0000000000000..0d5331ca6ebec --- /dev/null +++ b/src/test/ui/c-unwind/feature-gate-thiscall-unwind.rs @@ -0,0 +1,9 @@ +// 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/c-unwind/feature-gate-thiscall-unwind.stderr b/src/test/ui/c-unwind/feature-gate-thiscall-unwind.stderr new file mode 100644 index 0000000000000..d6673ebe38421 --- /dev/null +++ b/src/test/ui/c-unwind/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:4: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`. diff --git a/src/test/ui/codemap_tests/unicode.stderr b/src/test/ui/codemap_tests/unicode.stderr index 8b85adb58453f..ef2dd1cfbb132 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, 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, 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 e33adb239d78d..c04e3242289b8 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, 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, 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 24bdf6d669e88..7dd74bd229d3f 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. From af78f302da058395a6061f7fa0eb655714a01a23 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Thu, 10 Sep 2020 13:38:39 -0400 Subject: [PATCH 09/17] 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. --- 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 | 29 ++++++++++++++++++ .../codegen/unwind-abis/system-unwind-abi.rs | 29 ++++++++++++++++++ .../unwind-abis/thiscall-unwind-abi.rs | 30 +++++++++++++++++++ 7 files changed, 170 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 bc32f469d15ce..b3f62b4365ec7 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2523,6 +2523,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 @@ -2547,17 +2548,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, + } } } } @@ -2783,7 +2783,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 996615995259d..75016b1a5ff10 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 0000000000000..f67ad2f292f41 --- /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 0000000000000..5a47e0ebb8cea --- /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 0000000000000..ec7ebf8621986 --- /dev/null +++ b/src/test/codegen/unwind-abis/stdcall-unwind-abi.rs @@ -0,0 +1,29 @@ +// compile-flags: -C opt-level=0 + +// 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 0000000000000..a7c8caad5082e --- /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 0000000000000..ce3b4adb2a251 --- /dev/null +++ b/src/test/codegen/unwind-abis/thiscall-unwind-abi.rs @@ -0,0 +1,30 @@ +// compile-flags: -C opt-level=0 + +// 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 b4884c61a9cc8e2f8bfe5cea14f9ad0adcd8fc13 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Fri, 23 Oct 2020 18:49:34 -0400 Subject: [PATCH 10/17] pr review: abi debug assertion, exhaustive matches ### 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. Co-authored-by: Niko Matsakis --- compiler/rustc_middle/src/ty/layout.rs | 19 ++++++++++++++++++- compiler/rustc_mir_build/src/build/mod.rs | 17 ++++++++++++++++- compiler/rustc_target/src/spec/abi.rs | 15 +++++++++++++-- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index b3f62b4365ec7..e1da37d4639e8 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2556,7 +2556,24 @@ 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 + | 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 75016b1a5ff10..12db7e3bda46d 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -573,7 +573,22 @@ 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 + | RustIntrinsic + | PlatformIntrinsic + | Unadjusted => true, } } } diff --git a/compiler/rustc_target/src/spec/abi.rs b/compiler/rustc_target/src/spec/abi.rs index 387813f5d636a..f31ca2f10d3c3 100644 --- a/compiler/rustc_target/src/spec/abi.rs +++ b/compiler/rustc_target/src/spec/abi.rs @@ -100,7 +100,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, @@ -130,7 +130,18 @@ impl Abi { RustCall => 23, PlatformIntrinsic => 24, Unadjusted => 25, - } + }; + 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] From 35c90a2ca8f5daa7c312934ed70f0c45071c1c2b Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Thu, 10 Dec 2020 18:56:09 -0500 Subject: [PATCH 11/17] add `rust_eh_personality` to `#[no_std]` alloc tests ### Changes 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'." --- .../ui/allocator/no_std-alloc-error-handler-custom.rs | 9 ++++++++- .../ui/allocator/no_std-alloc-error-handler-default.rs | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) 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 4d40c7d0d2237..c9b4abbfd3fd3 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 4f8c44f1763c8..d6cd4a6af855f 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]); From bfd67f093ca1aa0ff19e6bb78d0055d7db235701 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Fri, 11 Dec 2020 20:54:47 -0500 Subject: [PATCH 12/17] add integration tests, unwind across FFI boundary ### Changes This commit introduces some new fixtures to the `run-make-fulldeps` test suite. * c-unwind-abi-hello-world: A simple "happy path" fixture. This involves calling from Rust, into C, and back into a Rust `C-unwind` function. * c-unwind-abi-catch-panic: Exercise unwinding a panic. This reuses much of the same code, but instead catches a panic 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. --- .../c-unwind-abi-catch-lib-panic/Makefile | 18 +++++++++ .../c-unwind-abi-catch-lib-panic/add.c | 12 ++++++ .../c-unwind-abi-catch-lib-panic/main.rs | 30 ++++++++++++++ .../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 | 39 +++++++++++++++++++ .../c-unwind-abi-hello-world/Makefile | 5 +++ .../c-unwind-abi-hello-world/add.c | 12 ++++++ .../c-unwind-abi-hello-world/main.rs | 29 ++++++++++++++ 10 files changed, 174 insertions(+) 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 create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-hello-world/Makefile create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-hello-world/add.c create mode 100644 src/test/run-make-fulldeps/c-unwind-abi-hello-world/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 0000000000000..7156d02ec8a5a --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/Makefile @@ -0,0 +1,18 @@ +-include ../tools.mk + +all: + # Compile `panic.rs` and `add.c` into object files. + # + # Note that we invoke `rustc` directly, so we may emit an object rather + # than an archive. We'll do that later. + $(BARE_RUSTC) $(RUSTFLAGS) \ + --out-dir $(TMPDIR) \ + --emit=obj panic.rs + $(call COMPILE_OBJ,$(TMPDIR)/add.o,add.c) + + # Now, create an archive using these two objects. + $(AR) crus $(TMPDIR)/libadd.a $(TMPDIR)/add.o $(TMPDIR)/panic.o + + # Compile `main.rs`, which will link into our library, and run it. + $(RUSTC) main.rs + $(call RUN,main) 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 0000000000000..444359451f6ec --- /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 0000000000000..8b7f817d45435 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-lib-panic/main.rs @@ -0,0 +1,30 @@ +//! 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 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 panic_u32 = *panic_obj.downcast_ref::().unwrap(); + assert_eq!(panic_u32, 11); +} + +#[link(name = "add")] +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 0000000000000..c7146e8618414 --- /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 0000000000000..9553b7aeeb983 --- /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 0000000000000..444359451f6ec --- /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 0000000000000..f479e90ac69e1 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-catch-panic/main.rs @@ -0,0 +1,39 @@ +//! 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 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 panic_u32 = *panic_obj.downcast_ref::().unwrap(); + assert_eq!(panic_u32, 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/run-make-fulldeps/c-unwind-abi-hello-world/Makefile b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/Makefile new file mode 100644 index 0000000000000..9553b7aeeb983 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/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-hello-world/add.c b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/add.c new file mode 100644 index 0000000000000..444359451f6ec --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/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-hello-world/main.rs b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/main.rs new file mode 100644 index 0000000000000..786af22ddc5e3 --- /dev/null +++ b/src/test/run-make-fulldeps/c-unwind-abi-hello-world/main.rs @@ -0,0 +1,29 @@ +//! A test for calling `C-unwind` functions across foreign function boundaries. +//! +//! This test does *not* trigger a panic. The exercises the "happy path" when calling a foreign +//! function that calls *back* into Rust. +#![feature(c_unwind)] + +fn main() { + let (a, b) = (9, 1); + let c = unsafe { add_small_numbers(a, b) }; + assert_eq!(c, 10); +} + +#[link(name = "add", kind = "static")] +extern { + /// 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! + } +} From 6013541361b472d54a84da3b881e79558c5eb1af Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Mon, 14 Dec 2020 15:05:47 -0500 Subject: [PATCH 13/17] update 32-bit hash in `impl1` test --- src/test/ui/symbol-names/impl1.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/test/ui/symbol-names/impl1.rs b/src/test/ui/symbol-names/impl1.rs index 7dd74bd229d3f..a352d255c782d 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 15fb65e572656da034f6c44862575494dc6be908 Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Tue, 5 Jan 2021 11:44:33 -0500 Subject: [PATCH 14/17] pr review: list all `-unwind` ABI's in unstable book ### Changes This is a small commit, updating 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 --- src/doc/unstable-book/src/language-features/c-unwind.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 c1705d59acc21..2801d9b5e7778 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 5d75cde97a6221b8824377e07695b0194e47b08d Mon Sep 17 00:00:00 2001 From: "katelyn a. martin" Date: Wed, 27 Jan 2021 10:46:59 -0500 Subject: [PATCH 15/17] bump tidy's `src/test/ui` file limit This commit addresses a tidy error that currently occurs on this branch: ``` tidy error: following path contains more than 1458 entries, you should move the test to some relevant subdirectory (current: 1459): /checkout/src/test/ui ``` Co-authored-by: bjorn3 --- src/tools/tidy/src/ui_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index e687901f212b6..21d05226fb42c 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -7,7 +7,7 @@ use std::path::Path; const ENTRY_LIMIT: usize = 1000; // FIXME: The following limits should be reduced eventually. -const ROOT_ENTRY_LIMIT: usize = 1458; +const ROOT_ENTRY_LIMIT: usize = 1459; const ISSUES_ENTRY_LIMIT: usize = 2669; fn check_entries(path: &Path, bad: &mut bool) { From cd8dceef863378706bc27e0a3545c9251f02ec8b Mon Sep 17 00:00:00 2001 From: Camelid Date: Sat, 12 Dec 2020 11:33:45 -0800 Subject: [PATCH 16/17] rustdoc: Render HRTB correctly for bare functions The angle brackets were not rendered, so code like this: some_func: for<'a> fn(val: &'a i32) -> i32 would be rendered as: some_func: fn'a(val: &'a i32) -> i32 However, rendering with angle brackets is still invalid syntax: some_func: fn<'a>(val: &'a i32) -> i32 so now it renders correctly as: some_func: for<'a> fn(val: &'a i32) -> i32 ----- However, note that this code: some_trait: dyn for<'a> Trait<'a> will still render as: some_trait: dyn Trait<'a> which is not invalid syntax, but is still unclear. Unfortunately I think it's hard to fix that case because there isn't enough information in the `rustdoc::clean::Type` that this code operates on. Perhaps that case can be fixed in a later PR. --- src/librustdoc/html/format.rs | 21 +++++++++++++++------ src/test/rustdoc/fn-type.rs | 15 +++++++++++++++ src/test/rustdoc/for-lifetime.rs | 14 ++++++++++++++ 3 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 src/test/rustdoc/fn-type.rs create mode 100644 src/test/rustdoc/for-lifetime.rs diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 01915c33a07ad..74b61f1555c6f 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -659,6 +659,8 @@ fn fmt_type( use_absolute: bool, cache: &Cache, ) -> fmt::Result { + debug!("fmt_type(t = {:?})", t); + match *t { clean::Generic(name) => write!(f, "{}", name), clean::ResolvedPath { did, ref param_names, ref path, is_generic } => { @@ -675,21 +677,22 @@ fn fmt_type( if f.alternate() { write!( f, - "{}{:#}fn{:#}{:#}", + "{:#}{}{:#}fn{:#}", + decl.print_hrtb_with_space(cache), decl.unsafety.print_with_space(), print_abi_with_space(decl.abi), - decl.print_generic_params(cache), decl.decl.print(cache) ) } else { write!( f, - "{}{}", + "{}{}{}", + decl.print_hrtb_with_space(cache), decl.unsafety.print_with_space(), print_abi_with_space(decl.abi) )?; primitive_link(f, PrimitiveType::Fn, "fn", cache)?; - write!(f, "{}{}", decl.print_generic_params(cache), decl.decl.print(cache)) + write!(f, "{}", decl.decl.print(cache)) } } clean::Tuple(ref typs) => { @@ -992,8 +995,14 @@ impl clean::FnRetTy { } impl clean::BareFunctionDecl { - fn print_generic_params<'a>(&'a self, cache: &'a Cache) -> impl fmt::Display + 'a { - comma_sep(self.generic_params.iter().map(move |g| g.print(cache))) + fn print_hrtb_with_space<'a>(&'a self, cache: &'a Cache) -> impl fmt::Display + 'a { + display_fn(move |f| { + if !self.generic_params.is_empty() { + write!(f, "for<{}> ", comma_sep(self.generic_params.iter().map(|g| g.print(cache)))) + } else { + Ok(()) + } + }) } } diff --git a/src/test/rustdoc/fn-type.rs b/src/test/rustdoc/fn-type.rs new file mode 100644 index 0000000000000..f5e123aed9c47 --- /dev/null +++ b/src/test/rustdoc/fn-type.rs @@ -0,0 +1,15 @@ +// ignore-tidy-linelength + +#![crate_name = "foo"] +#![crate_type = "lib"] + +pub struct Foo<'a, T> { + pub generic: fn(val: &T) -> T, + + pub lifetime: fn(val: &'a i32) -> i32, + pub hrtb_lifetime: for<'b, 'c> fn(one: &'b i32, two: &'c &'b i32) -> (&'b i32, &'c i32), +} + +// @has 'foo/struct.Foo.html' '//span[@id="structfield.generic"]' "generic: fn(val: &T) -> T" +// @has 'foo/struct.Foo.html' '//span[@id="structfield.lifetime"]' "lifetime: fn(val: &'a i32) -> i32" +// @has 'foo/struct.Foo.html' '//span[@id="structfield.hrtb_lifetime"]' "hrtb_lifetime: for<'b, 'c> fn(one: &'b i32, two: &'c &'b i32) -> (&'b i32, &'c i32)" diff --git a/src/test/rustdoc/for-lifetime.rs b/src/test/rustdoc/for-lifetime.rs new file mode 100644 index 0000000000000..299794b63b2ea --- /dev/null +++ b/src/test/rustdoc/for-lifetime.rs @@ -0,0 +1,14 @@ +// ignore-tidy-linelength + +#![crate_name = "foo"] +#![crate_type = "lib"] + +pub struct Foo { + pub some_func: for<'a> fn(val: &'a i32) -> i32, + pub some_trait: dyn for<'a> Trait<'a>, +} + +// @has foo/struct.Foo.html '//span[@id="structfield.some_func"]' "some_func: for<'a> fn(val: &'a i32) -> i32" +// @has foo/struct.Foo.html '//span[@id="structfield.some_trait"]' "some_trait: dyn Trait<'a>" + +pub trait Trait<'a> {} From f9025512e7fd91684a27c7b7aef31f20a01092af Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 7 Dec 2020 18:55:00 -0500 Subject: [PATCH 17/17] Add `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` lint cc #79813 This PR adds an allow-by-default future-compatibility lint `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS`. It fires when a trailing semicolon in a macro body is ignored due to the macro being used in expression position: ```rust macro_rules! foo { () => { true; // WARN } } fn main() { let val = match true { true => false, _ => foo!() }; } ``` The lint takes its level from the macro call site, and can be allowed for a particular macro by adding `#[allow(semicolon_in_expressions_from_macros)]`. The lint is set to warn for all internal rustc crates (when being built by a stage1 compiler). After the next beta bump, we can enable the lint for the bootstrap compiler as well. --- Cargo.lock | 1 + compiler/rustc_expand/Cargo.toml | 1 + compiler/rustc_expand/src/mbe/macro_rules.rs | 14 +++++- compiler/rustc_lint_defs/src/builtin.rs | 48 +++++++++++++++++++ compiler/rustc_resolve/src/macros.rs | 2 + src/bootstrap/bootstrap.py | 1 + src/bootstrap/builder.rs | 6 +++ ...ow-semicolon-in-expressions-from-macros.rs | 15 ++++++ .../semicolon-in-expressions-from-macros.rs | 30 ++++++++++++ ...emicolon-in-expressions-from-macros.stderr | 33 +++++++++++++ 10 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/lint/semicolon-in-expressions-from-macros/allow-semicolon-in-expressions-from-macros.rs create mode 100644 src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs create mode 100644 src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr diff --git a/Cargo.lock b/Cargo.lock index fb5ae6ce66303..992ebdb801af7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3746,6 +3746,7 @@ dependencies = [ "rustc_errors", "rustc_feature", "rustc_lexer", + "rustc_lint_defs", "rustc_macros", "rustc_parse", "rustc_serialize", diff --git a/compiler/rustc_expand/Cargo.toml b/compiler/rustc_expand/Cargo.toml index 25c2851f6de59..7413b0d9431f9 100644 --- a/compiler/rustc_expand/Cargo.toml +++ b/compiler/rustc_expand/Cargo.toml @@ -18,6 +18,7 @@ rustc_attr = { path = "../rustc_attr" } rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } rustc_feature = { path = "../rustc_feature" } +rustc_lint_defs = { path = "../rustc_lint_defs" } rustc_macros = { path = "../rustc_macros" } rustc_lexer = { path = "../rustc_lexer" } rustc_parse = { path = "../rustc_parse" } diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index e8c711cae64ef..73fbde70bda9f 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -11,12 +11,14 @@ use crate::mbe::transcribe::transcribe; use rustc_ast as ast; use rustc_ast::token::{self, NonterminalKind, NtTT, Token, TokenKind::*}; use rustc_ast::tokenstream::{DelimSpan, TokenStream}; +use rustc_ast::NodeId; use rustc_ast_pretty::pprust; use rustc_attr::{self as attr, TransparencyError}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::Lrc; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_feature::Features; +use rustc_lint_defs::builtin::SEMICOLON_IN_EXPRESSIONS_FROM_MACROS; use rustc_parse::parser::Parser; use rustc_session::parse::ParseSess; use rustc_session::Session; @@ -37,6 +39,7 @@ crate struct ParserAnyMacro<'a> { site_span: Span, /// The ident of the macro we're parsing macro_ident: Ident, + lint_node_id: NodeId, arm_span: Span, } @@ -110,7 +113,8 @@ fn emit_frag_parse_err( impl<'a> ParserAnyMacro<'a> { crate fn make(mut self: Box>, kind: AstFragmentKind) -> AstFragment { - let ParserAnyMacro { site_span, macro_ident, ref mut parser, arm_span } = *self; + let ParserAnyMacro { site_span, macro_ident, ref mut parser, lint_node_id, arm_span } = + *self; let snapshot = &mut parser.clone(); let fragment = match parse_ast_fragment(parser, kind) { Ok(f) => f, @@ -124,6 +128,12 @@ impl<'a> ParserAnyMacro<'a> { // `macro_rules! m { () => { panic!(); } }` isn't parsed by `.parse_expr()`, // but `m!()` is allowed in expression positions (cf. issue #34706). if kind == AstFragmentKind::Expr && parser.token == token::Semi { + parser.sess.buffer_lint( + SEMICOLON_IN_EXPRESSIONS_FROM_MACROS, + parser.token.span, + lint_node_id, + "trailing semicolon in macro used in expression position", + ); parser.bump(); } @@ -276,6 +286,7 @@ fn generic_extension<'cx>( let mut p = Parser::new(sess, tts, false, None); p.last_type_ascription = cx.current_expansion.prior_type_ascription; + let lint_node_id = cx.resolver.lint_node_id(cx.current_expansion.id); // Let the context choose how to interpret the result. // Weird, but useful for X-macros. @@ -287,6 +298,7 @@ fn generic_extension<'cx>( // macro leaves unparsed tokens. site_span: sp, macro_ident: name, + lint_node_id, arm_span, }); } diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 658372ac336a8..a8bf1ce51bb74 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -1,3 +1,4 @@ +// ignore-tidy-filelength //! Some lints that are built in to the compiler. //! //! These are the built-in lints that are emitted direct in the main @@ -2833,6 +2834,52 @@ declare_lint! { "detects `#[unstable]` on stable trait implementations for stable types" } +declare_lint! { + /// The `semicolon_in_expressions_from_macros` lint detects trailing semicolons + /// in macro bodies when the macro is invoked in expression position. + /// This was previous accepted, but is being phased out. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// #![deny(semicolon_in_expressions_from_macros)] + /// macro_rules! foo { + /// () => { true; } + /// } + /// + /// fn main() { + /// let val = match true { + /// true => false, + /// _ => foo!() + /// }; + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Previous, Rust ignored trailing semicolon in a macro + /// body when a macro was invoked in expression position. + /// However, this makes the treatment of semicolons in the language + /// inconsistent, and could lead to unexpected runtime behavior + /// in some circumstances (e.g. if the macro author expects + /// a value to be dropped). + /// + /// This is a [future-incompatible] lint to transition this + /// to a hard error in the future. See [issue #79813] for more details. + /// + /// [issue #79813]: https://github.com/rust-lang/rust/issues/79813 + /// [future-incompatible]: ../index.md#future-incompatible-lints + pub SEMICOLON_IN_EXPRESSIONS_FROM_MACROS, + Allow, + "trailing semicolon in macro body used as expression", + @future_incompatible = FutureIncompatibleInfo { + reference: "issue #79813 ", + edition: None, + }; +} + declare_lint_pass! { /// Does nothing as a lint pass, but registers some `Lint`s /// that are used by other parts of the compiler. @@ -2920,6 +2967,7 @@ declare_lint_pass! { USELESS_DEPRECATED, UNSUPPORTED_NAKED_FUNCTIONS, MISSING_ABI, + SEMICOLON_IN_EXPRESSIONS_FROM_MACROS, ] } diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 5d6120cd31076..d0adee2429d9a 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -344,6 +344,8 @@ impl<'a> ResolverExpand for Resolver<'a> { } fn lint_node_id(&mut self, expn_id: ExpnId) -> NodeId { + // FIXME - make this more precise. This currently returns the NodeId of the + // nearest closing item - we should try to return the closest parent of the ExpnId self.invocation_parents .get(&expn_id) .map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[*id]) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index 5350c9eefe753..8576f57959a6f 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -833,6 +833,7 @@ def build_bootstrap(self): target_linker = self.get_toml("linker", build_section) if target_linker is not None: env["RUSTFLAGS"] += " -C linker=" + target_linker + # cfg(bootstrap): Add `-Wsemicolon_in_expressions_from_macros` after the next beta bump env["RUSTFLAGS"] += " -Wrust_2018_idioms -Wunused_lifetimes" if self.get_toml("deny-warnings", "rust") != "false": env["RUSTFLAGS"] += " -Dwarnings" diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 62065e27dd966..4b58e609f1560 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -1250,6 +1250,12 @@ impl<'a> Builder<'a> { // some code doesn't go through this `rustc` wrapper. lint_flags.push("-Wrust_2018_idioms"); lint_flags.push("-Wunused_lifetimes"); + // cfg(bootstrap): unconditionally enable this warning after the next beta bump + // This is currently disabled for the stage1 libstd, since build scripts + // will end up using the bootstrap compiler (which doesn't yet support this lint) + if compiler.stage != 0 && mode != Mode::Std { + lint_flags.push("-Wsemicolon_in_expressions_from_macros"); + } if self.config.deny_warnings { lint_flags.push("-Dwarnings"); diff --git a/src/test/ui/lint/semicolon-in-expressions-from-macros/allow-semicolon-in-expressions-from-macros.rs b/src/test/ui/lint/semicolon-in-expressions-from-macros/allow-semicolon-in-expressions-from-macros.rs new file mode 100644 index 0000000000000..6f9e6ec0a57ff --- /dev/null +++ b/src/test/ui/lint/semicolon-in-expressions-from-macros/allow-semicolon-in-expressions-from-macros.rs @@ -0,0 +1,15 @@ +// check-pass +// Ensure that trailing semicolons are allowed by default + +macro_rules! foo { + () => { + true; + } +} + +fn main() { + let val = match true { + true => false, + _ => foo!() + }; +} diff --git a/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs new file mode 100644 index 0000000000000..605d5a0309cfc --- /dev/null +++ b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.rs @@ -0,0 +1,30 @@ +// check-pass +#![warn(semicolon_in_expressions_from_macros)] + +#[allow(dead_code)] +macro_rules! foo { + ($val:ident) => { + true; //~ WARN trailing + //~| WARN this was previously + //~| WARN trailing + //~| WARN this was previously + } +} + +fn main() { + // This `allow` doesn't work + #[allow(semicolon_in_expressions_from_macros)] + let _ = { + foo!(first) + }; + + // This 'allow' doesn't work either + #[allow(semicolon_in_expressions_from_macros)] + let _ = foo!(second); + + // But this 'allow' does + #[allow(semicolon_in_expressions_from_macros)] + fn inner() { + let _ = foo!(third); + } +} diff --git a/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr new file mode 100644 index 0000000000000..6f9f879661a41 --- /dev/null +++ b/src/test/ui/lint/semicolon-in-expressions-from-macros/semicolon-in-expressions-from-macros.stderr @@ -0,0 +1,33 @@ +warning: trailing semicolon in macro used in expression position + --> $DIR/semicolon-in-expressions-from-macros.rs:7:13 + | +LL | true; + | ^ +... +LL | foo!(first) + | ----------- in this macro invocation + | +note: the lint level is defined here + --> $DIR/semicolon-in-expressions-from-macros.rs:2:9 + | +LL | #![warn(semicolon_in_expressions_from_macros)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #79813 + = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +warning: trailing semicolon in macro used in expression position + --> $DIR/semicolon-in-expressions-from-macros.rs:7:13 + | +LL | true; + | ^ +... +LL | let _ = foo!(second); + | ------------ in this macro invocation + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #79813 + = note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +warning: 2 warnings emitted +