From e7f8895359365403a955468156a35bf26a45e0bf Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 24 Mar 2020 15:20:19 -0400 Subject: [PATCH 01/25] save/restore `pessimistic_yield` when entering bodies This flag is used to make the execution order around `+=` operators pessimistic. Failure to save/restore the flag was causing independent async blocks to effect one another, leading to strange ICEs and failed assumptions. --- src/librustc_passes/region.rs | 2 ++ src/test/ui/async-await/issues/issue-69307.rs | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 src/test/ui/async-await/issues/issue-69307.rs diff --git a/src/librustc_passes/region.rs b/src/librustc_passes/region.rs index e771696a5b6bf..1807756fe524b 100644 --- a/src/librustc_passes/region.rs +++ b/src/librustc_passes/region.rs @@ -720,6 +720,7 @@ impl<'tcx> Visitor<'tcx> for RegionResolutionVisitor<'tcx> { let outer_ec = mem::replace(&mut self.expr_and_pat_count, 0); let outer_cx = self.cx; let outer_ts = mem::take(&mut self.terminating_scopes); + let outer_pessimistic_yield = mem::replace(&mut self.pessimistic_yield, false); self.terminating_scopes.insert(body.value.hir_id.local_id); if let Some(root_id) = self.cx.root_id { @@ -771,6 +772,7 @@ impl<'tcx> Visitor<'tcx> for RegionResolutionVisitor<'tcx> { self.expr_and_pat_count = outer_ec; self.cx = outer_cx; self.terminating_scopes = outer_ts; + self.pessimistic_yield = outer_pessimistic_yield; } fn visit_arm(&mut self, a: &'tcx Arm<'tcx>) { diff --git a/src/test/ui/async-await/issues/issue-69307.rs b/src/test/ui/async-await/issues/issue-69307.rs new file mode 100644 index 0000000000000..4dae96ec8a6a7 --- /dev/null +++ b/src/test/ui/async-await/issues/issue-69307.rs @@ -0,0 +1,23 @@ +// Regression test for #69307 +// +// Having a `async { .. foo.await .. }` block appear inside of a `+=` +// expression was causing an ICE due to a failure to save/restore +// state in the AST numbering pass when entering a nested body. +// +// check-pass +// edition:2018 + +fn block_on(_: F) -> usize { + 0 +} + +fn main() {} + +async fn bar() { + let mut sum = 0; + sum += block_on(async { + baz().await; + }); +} + +async fn baz() {} From 041e1704fcd9cc932a4fa587c43d32ed9dcb9712 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 29 Dec 2019 15:47:13 +0000 Subject: [PATCH 02/25] use of wmemchr for faster searching in [u16] --- src/libstd/sys/windows/mod.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/libstd/sys/windows/mod.rs b/src/libstd/sys/windows/mod.rs index 74dd41fd50147..cb75e8122fdd5 100644 --- a/src/libstd/sys/windows/mod.rs +++ b/src/libstd/sys/windows/mod.rs @@ -81,10 +81,20 @@ pub fn decode_error_kind(errno: i32) -> ErrorKind { } } +pub fn wmemchr(needle: u16, haystack: &[u16]) -> Option { + extern "C" { + fn wmemchr(s: *const u16, c: u16, n: usize) -> *mut u16; + } + let len = haystack.len(); + let ptr = haystack.as_ptr(); + let p = unsafe { wmemchr(ptr, needle, len) }; + if p.is_null() { None } else { Some((p as usize - ptr as usize) / 2) } +} + pub fn to_u16s>(s: S) -> crate::io::Result> { fn inner(s: &OsStr) -> crate::io::Result> { let mut maybe_result: Vec = s.encode_wide().collect(); - if maybe_result.iter().any(|&u| u == 0) { + if wmemchr(0, &maybe_result).is_some() { return Err(crate::io::Error::new( ErrorKind::InvalidInput, "strings passed to WinAPI cannot contain NULs", @@ -214,7 +224,7 @@ fn wide_char_to_multi_byte( } pub fn truncate_utf16_at_nul(v: &[u16]) -> &[u16] { - match v.iter().position(|c| *c == 0) { + match wmemchr(0, v) { // don't include the 0 Some(i) => &v[..i], None => v, From 89bc23643bc4ba979f28d6df8c091813c89c36a9 Mon Sep 17 00:00:00 2001 From: lzutao Date: Thu, 2 Apr 2020 02:43:23 +0000 Subject: [PATCH 03/25] Use unrolled loop --- src/libstd/sys/windows/mod.rs | 52 +++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/src/libstd/sys/windows/mod.rs b/src/libstd/sys/windows/mod.rs index cb75e8122fdd5..d745e87a07258 100644 --- a/src/libstd/sys/windows/mod.rs +++ b/src/libstd/sys/windows/mod.rs @@ -81,20 +81,54 @@ pub fn decode_error_kind(errno: i32) -> ErrorKind { } } -pub fn wmemchr(needle: u16, haystack: &[u16]) -> Option { - extern "C" { - fn wmemchr(s: *const u16, c: u16, n: usize) -> *mut u16; - } - let len = haystack.len(); +pub fn unrolled_find_u16s(needle: u16, haystack: &[u16]) -> Option { let ptr = haystack.as_ptr(); - let p = unsafe { wmemchr(ptr, needle, len) }; - if p.is_null() { None } else { Some((p as usize - ptr as usize) / 2) } + let mut len = haystack.len(); + let mut start = &haystack[..]; + + // For performance reasons unfold the loop eight times. + while len >= 8 { + if start[0] == needle { + return Some((start.as_ptr() as usize - ptr as usize) / 2); + } + if start[1] == needle { + return Some((start[1..].as_ptr() as usize - ptr as usize) / 2); + } + if start[2] == needle { + return Some((start[2..].as_ptr() as usize - ptr as usize) / 2); + } + if start[3] == needle { + return Some((start[3..].as_ptr() as usize - ptr as usize) / 2); + } + if start[4] == needle { + return Some((start[4..].as_ptr() as usize - ptr as usize) / 2); + } + if start[5] == needle { + return Some((start[5..].as_ptr() as usize - ptr as usize) / 2); + } + if start[6] == needle { + return Some((start[6..].as_ptr() as usize - ptr as usize) / 2); + } + if start[7] == needle { + return Some((start[7..].as_ptr() as usize - ptr as usize) / 2); + } + + start = &start[8..]; + len -= 8; + } + + for (i, c) in start.iter().enumerate() { + if *c == needle { + return Some((start.as_ptr() as usize - ptr as usize) / 2 + i); + } + } + None } pub fn to_u16s>(s: S) -> crate::io::Result> { fn inner(s: &OsStr) -> crate::io::Result> { let mut maybe_result: Vec = s.encode_wide().collect(); - if wmemchr(0, &maybe_result).is_some() { + if unrolled_find_u16s(0, &maybe_result).is_some() { return Err(crate::io::Error::new( ErrorKind::InvalidInput, "strings passed to WinAPI cannot contain NULs", @@ -224,7 +258,7 @@ fn wide_char_to_multi_byte( } pub fn truncate_utf16_at_nul(v: &[u16]) -> &[u16] { - match wmemchr(0, v) { + match unrolled_find_u16s(0, v) { // don't include the 0 Some(i) => &v[..i], None => v, From 87d859af24f49496e6aa2f65833e1cc2909ccf52 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 5 Apr 2020 21:04:31 +0200 Subject: [PATCH 04/25] Don't lint for self-recursion when the function can diverge --- src/librustc_mir_build/build/mod.rs | 4 +- src/librustc_mir_build/lints.rs | 202 ++++++++++-------- .../ui/lint/lint-unconditional-recursion.rs | 18 ++ 3 files changed, 132 insertions(+), 92 deletions(-) diff --git a/src/librustc_mir_build/build/mod.rs b/src/librustc_mir_build/build/mod.rs index 04cb509d44e4b..6e1a4ecf47a44 100644 --- a/src/librustc_mir_build/build/mod.rs +++ b/src/librustc_mir_build/build/mod.rs @@ -178,11 +178,11 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { build::construct_const(cx, body_id, return_ty, return_ty_span) }; - lints::check(tcx, &body, def_id); - let mut body = BodyAndCache::new(body); body.ensure_predecessors(); + lints::check(tcx, &body.unwrap_read_only(), def_id); + // The borrow checker will replace all the regions here with its own // inference variables. There's no point having non-erased regions here. // The exception is `body.user_type_annotations`, which is used unmodified diff --git a/src/librustc_mir_build/lints.rs b/src/librustc_mir_build/lints.rs index b1628ec58c3c6..01dd575c51ed1 100644 --- a/src/librustc_mir_build/lints.rs +++ b/src/librustc_mir_build/lints.rs @@ -1,13 +1,15 @@ use rustc_hir::def_id::DefId; use rustc_hir::intravisit::FnKind; use rustc_index::bit_set::BitSet; +use rustc_index::vec::IndexVec; use rustc_middle::hir::map::blocks::FnLikeNode; -use rustc_middle::mir::{self, Body, TerminatorKind}; +use rustc_middle::mir::{BasicBlock, Body, ReadOnlyBodyAndCache, TerminatorKind, START_BLOCK}; use rustc_middle::ty::subst::InternalSubsts; use rustc_middle::ty::{self, AssocItem, AssocItemContainer, Instance, TyCtxt}; use rustc_session::lint::builtin::UNCONDITIONAL_RECURSION; +use std::collections::VecDeque; -crate fn check<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId) { +crate fn check<'tcx>(tcx: TyCtxt<'tcx>, body: &ReadOnlyBodyAndCache<'_, 'tcx>, def_id: DefId) { let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap(); if let Some(fn_like_node) = FnLikeNode::from_node(tcx.hir().get(hir_id)) { @@ -18,7 +20,7 @@ crate fn check<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId) { fn check_fn_for_unconditional_recursion<'tcx>( tcx: TyCtxt<'tcx>, fn_kind: FnKind<'_>, - body: &Body<'tcx>, + body: &ReadOnlyBodyAndCache<'_, 'tcx>, def_id: DefId, ) { if let FnKind::Closure(_) = fn_kind { @@ -26,113 +28,133 @@ fn check_fn_for_unconditional_recursion<'tcx>( return; } - //FIXME(#54444) rewrite this lint to use the dataflow framework - - // Walk through this function (say `f`) looking to see if - // every possible path references itself, i.e., the function is - // called recursively unconditionally. This is done by trying - // to find a path from the entry node to the exit node that - // *doesn't* call `f` by traversing from the entry while - // pretending that calls of `f` are sinks (i.e., ignoring any - // exit edges from them). - // - // NB. this has an edge case with non-returning statements, - // like `loop {}` or `panic!()`: control flow never reaches - // the exit node through these, so one can have a function - // that never actually calls itself but is still picked up by - // this lint: - // - // fn f(cond: bool) { - // if !cond { panic!() } // could come from `assert!(cond)` - // f(false) - // } - // - // In general, functions of that form may be able to call - // itself a finite number of times and then diverge. The lint - // considers this to be an error for two reasons, (a) it is - // easier to implement, and (b) it seems rare to actually want - // to have behaviour like the above, rather than - // e.g., accidentally recursing after an assert. - - let basic_blocks = body.basic_blocks(); - let mut reachable_without_self_call_queue = vec![mir::START_BLOCK]; - let mut reached_exit_without_self_call = false; - let mut self_call_locations = vec![]; - let mut visited = BitSet::new_empty(basic_blocks.len()); + let self_calls = find_blocks_calling_self(tcx, &body, def_id); + let mut results = IndexVec::from_elem_n(vec![], body.basic_blocks().len()); + let mut queue: VecDeque<_> = self_calls.iter().collect(); - let param_env = tcx.param_env(def_id); - let trait_substs_count = match tcx.opt_associated_item(def_id) { - Some(AssocItem { container: AssocItemContainer::TraitContainer(trait_def_id), .. }) => { - tcx.generics_of(trait_def_id).count() - } - _ => 0, - }; - let caller_substs = &InternalSubsts::identity_for_item(tcx, def_id)[..trait_substs_count]; - - while let Some(bb) = reachable_without_self_call_queue.pop() { - if !visited.insert(bb) { - //already done + while let Some(bb) = queue.pop_front() { + if !results[bb].is_empty() { + // Already propagated. continue; } - let block = &basic_blocks[bb]; - - if let Some(ref terminator) = block.terminator { - match terminator.kind { - TerminatorKind::Call { ref func, .. } => { - let func_ty = func.ty(body, tcx); - - if let ty::FnDef(fn_def_id, substs) = func_ty.kind { - let (call_fn_id, call_substs) = if let Some(instance) = - Instance::resolve(tcx, param_env, fn_def_id, substs) - { - (instance.def_id(), instance.substs) - } else { - (fn_def_id, substs) - }; - - let is_self_call = call_fn_id == def_id - && &call_substs[..caller_substs.len()] == caller_substs; - - if is_self_call { - self_call_locations.push(terminator.source_info); - - //this is a self call so we shouldn't explore - //further down this path - continue; - } - } + let locations = if self_calls.contains(bb) { + // `bb` *is* a self-call. + vec![bb] + } else { + // If *all* successors of `bb` lead to a self-call, emit notes at all of their + // locations. + + // Converging successors without unwind paths. + let terminator = body[bb].terminator(); + let relevant_successors = match &terminator.kind { + TerminatorKind::Call { destination: Some((_, dest)), .. } => { + Some(dest).into_iter().chain(&[]) } - TerminatorKind::Abort | TerminatorKind::Return => { - //found a path! - reached_exit_without_self_call = true; - break; + TerminatorKind::Call { destination: None, .. } => None.into_iter().chain(&[]), + TerminatorKind::SwitchInt { targets, .. } => None.into_iter().chain(targets), + TerminatorKind::Goto { target } + | TerminatorKind::Drop { target, .. } + | TerminatorKind::DropAndReplace { target, .. } + | TerminatorKind::Assert { target, .. } => Some(target).into_iter().chain(&[]), + TerminatorKind::Yield { .. } | TerminatorKind::GeneratorDrop => { + None.into_iter().chain(&[]) } - _ => {} - } + TerminatorKind::FalseEdges { real_target, .. } + | TerminatorKind::FalseUnwind { real_target, .. } => { + Some(real_target).into_iter().chain(&[]) + } + TerminatorKind::Resume + | TerminatorKind::Abort + | TerminatorKind::Return + | TerminatorKind::Unreachable => { + unreachable!("unexpected terminator {:?}", terminator.kind) + } + }; + + let all_are_self_calls = + relevant_successors.clone().all(|&succ| !results[succ].is_empty()); - for successor in terminator.successors() { - reachable_without_self_call_queue.push(*successor); + if all_are_self_calls { + relevant_successors.flat_map(|&succ| results[succ].iter().copied()).collect() + } else { + vec![] } + }; + + if !locations.is_empty() { + // This is a newly confirmed-to-always-reach-self-call block. + results[bb] = locations; + + // Propagate backwards through the CFG. + debug!("propagate loc={:?} in {:?} -> {:?}", results[bb], bb, body.predecessors()[bb]); + queue.extend(body.predecessors()[bb].iter().copied()); } } - // Check the number of self calls because a function that - // doesn't return (e.g., calls a `-> !` function or `loop { /* - // no break */ }`) shouldn't be linted unless it actually - // recurs. - if !reached_exit_without_self_call && !self_call_locations.is_empty() { + debug!("unconditional recursion results: {:?}", results); + + let self_call_locations = &mut results[START_BLOCK]; + self_call_locations.sort(); + self_call_locations.dedup(); + + if !self_call_locations.is_empty() { let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap(); let sp = tcx.sess.source_map().guess_head_span(tcx.hir().span(hir_id)); tcx.struct_span_lint_hir(UNCONDITIONAL_RECURSION, hir_id, sp, |lint| { let mut db = lint.build("function cannot return without recursing"); db.span_label(sp, "cannot return without recursing"); // offer some help to the programmer. - for location in &self_call_locations { - db.span_label(location.span, "recursive call site"); + for bb in self_call_locations { + let span = body.basic_blocks()[*bb].terminator().source_info.span; + db.span_label(span, "recursive call site"); } db.help("a `loop` may express intention better if this is on purpose"); db.emit(); }); } } + +/// Finds blocks with `Call` terminators that would end up calling back into the same method. +fn find_blocks_calling_self<'tcx>( + tcx: TyCtxt<'tcx>, + body: &Body<'tcx>, + def_id: DefId, +) -> BitSet { + let param_env = tcx.param_env(def_id); + let trait_substs_count = match tcx.opt_associated_item(def_id) { + Some(AssocItem { container: AssocItemContainer::TraitContainer(trait_def_id), .. }) => { + tcx.generics_of(trait_def_id).count() + } + _ => 0, + }; + let caller_substs = &InternalSubsts::identity_for_item(tcx, def_id)[..trait_substs_count]; + + let mut self_calls = BitSet::new_empty(body.basic_blocks().len()); + + for (bb, data) in body.basic_blocks().iter_enumerated() { + if let TerminatorKind::Call { func, .. } = &data.terminator().kind { + let func_ty = func.ty(body, tcx); + + if let ty::FnDef(fn_def_id, substs) = func_ty.kind { + let (call_fn_id, call_substs) = + if let Some(instance) = Instance::resolve(tcx, param_env, fn_def_id, substs) { + (instance.def_id(), instance.substs) + } else { + (fn_def_id, substs) + }; + + // FIXME(#57965): Make this work across function boundaries + + let is_self_call = + call_fn_id == def_id && &call_substs[..caller_substs.len()] == caller_substs; + + if is_self_call { + self_calls.insert(bb); + } + } + } + } + + self_calls +} diff --git a/src/test/ui/lint/lint-unconditional-recursion.rs b/src/test/ui/lint/lint-unconditional-recursion.rs index ab60a326cd220..d2a0329585b71 100644 --- a/src/test/ui/lint/lint-unconditional-recursion.rs +++ b/src/test/ui/lint/lint-unconditional-recursion.rs @@ -131,4 +131,22 @@ trait Bar { } } +// Do not trigger on functions that may diverge instead of self-recursing (#54444) + +pub fn loops(x: bool) { + if x { + loops(x); + } else { + loop {} + } +} + +pub fn panics(x: bool) { + if x { + panics(!x); + } else { + panic!("panics"); + } +} + fn main() {} From 9dd18a31e266b773a8c2b2308f21e7e39139754a Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 5 Apr 2020 21:01:38 +0200 Subject: [PATCH 05/25] Move closure check upwards --- src/librustc_mir_build/lints.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/librustc_mir_build/lints.rs b/src/librustc_mir_build/lints.rs index 01dd575c51ed1..38cae5793d764 100644 --- a/src/librustc_mir_build/lints.rs +++ b/src/librustc_mir_build/lints.rs @@ -13,21 +13,20 @@ crate fn check<'tcx>(tcx: TyCtxt<'tcx>, body: &ReadOnlyBodyAndCache<'_, 'tcx>, d let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap(); if let Some(fn_like_node) = FnLikeNode::from_node(tcx.hir().get(hir_id)) { - check_fn_for_unconditional_recursion(tcx, fn_like_node.kind(), body, def_id); + if let FnKind::Closure(_) = fn_like_node.kind() { + // closures can't recur, so they don't matter. + return; + } + + check_fn_for_unconditional_recursion(tcx, body, def_id); } } fn check_fn_for_unconditional_recursion<'tcx>( tcx: TyCtxt<'tcx>, - fn_kind: FnKind<'_>, body: &ReadOnlyBodyAndCache<'_, 'tcx>, def_id: DefId, ) { - if let FnKind::Closure(_) = fn_kind { - // closures can't recur, so they don't matter. - return; - } - let self_calls = find_blocks_calling_self(tcx, &body, def_id); let mut results = IndexVec::from_elem_n(vec![], body.basic_blocks().len()); let mut queue: VecDeque<_> = self_calls.iter().collect(); From 60e9927125f6fb191275b7f93f3ed34241fc35ef Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 6 Apr 2020 01:07:33 +0200 Subject: [PATCH 06/25] Merge redundant match arms --- src/librustc_mir_build/lints.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir_build/lints.rs b/src/librustc_mir_build/lints.rs index 38cae5793d764..1b7a8e6d7291f 100644 --- a/src/librustc_mir_build/lints.rs +++ b/src/librustc_mir_build/lints.rs @@ -47,21 +47,18 @@ fn check_fn_for_unconditional_recursion<'tcx>( // Converging successors without unwind paths. let terminator = body[bb].terminator(); let relevant_successors = match &terminator.kind { - TerminatorKind::Call { destination: Some((_, dest)), .. } => { - Some(dest).into_iter().chain(&[]) - } - TerminatorKind::Call { destination: None, .. } => None.into_iter().chain(&[]), + TerminatorKind::Call { destination: None, .. } + | TerminatorKind::Yield { .. } + | TerminatorKind::GeneratorDrop => None.into_iter().chain(&[]), TerminatorKind::SwitchInt { targets, .. } => None.into_iter().chain(targets), TerminatorKind::Goto { target } | TerminatorKind::Drop { target, .. } | TerminatorKind::DropAndReplace { target, .. } - | TerminatorKind::Assert { target, .. } => Some(target).into_iter().chain(&[]), - TerminatorKind::Yield { .. } | TerminatorKind::GeneratorDrop => { - None.into_iter().chain(&[]) - } - TerminatorKind::FalseEdges { real_target, .. } - | TerminatorKind::FalseUnwind { real_target, .. } => { - Some(real_target).into_iter().chain(&[]) + | TerminatorKind::Assert { target, .. } + | TerminatorKind::FalseEdges { real_target: target, .. } + | TerminatorKind::FalseUnwind { real_target: target, .. } + | TerminatorKind::Call { destination: Some((_, target)), .. } => { + Some(target).into_iter().chain(&[]) } TerminatorKind::Resume | TerminatorKind::Abort From dec90784ac143aba2eed9363e212a9197482f0ed Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 6 Apr 2020 01:52:51 +0200 Subject: [PATCH 07/25] Add some comments and rename variable --- src/librustc_mir_build/lints.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir_build/lints.rs b/src/librustc_mir_build/lints.rs index 1b7a8e6d7291f..3e0028a66d452 100644 --- a/src/librustc_mir_build/lints.rs +++ b/src/librustc_mir_build/lints.rs @@ -64,6 +64,7 @@ fn check_fn_for_unconditional_recursion<'tcx>( | TerminatorKind::Abort | TerminatorKind::Return | TerminatorKind::Unreachable => { + // We propagate backwards, so these should never be encountered here. unreachable!("unexpected terminator {:?}", terminator.kind) } }; @@ -118,13 +119,15 @@ fn find_blocks_calling_self<'tcx>( def_id: DefId, ) -> BitSet { let param_env = tcx.param_env(def_id); + + // If this is trait/impl method, extract the trait's substs. let trait_substs_count = match tcx.opt_associated_item(def_id) { Some(AssocItem { container: AssocItemContainer::TraitContainer(trait_def_id), .. }) => { tcx.generics_of(trait_def_id).count() } _ => 0, }; - let caller_substs = &InternalSubsts::identity_for_item(tcx, def_id)[..trait_substs_count]; + let trait_substs = &InternalSubsts::identity_for_item(tcx, def_id)[..trait_substs_count]; let mut self_calls = BitSet::new_empty(body.basic_blocks().len()); @@ -142,8 +145,12 @@ fn find_blocks_calling_self<'tcx>( // FIXME(#57965): Make this work across function boundaries + // If this is a trait fn, the substs on the trait have to match, or we might be + // calling into an entirely different method (for example, a call from the default + // method in the trait to `>::method`, where `A` and/or `B` are + // specific types). let is_self_call = - call_fn_id == def_id && &call_substs[..caller_substs.len()] == caller_substs; + call_fn_id == def_id && &call_substs[..trait_substs.len()] == trait_substs; if is_self_call { self_calls.insert(bb); From cd9f709a33b952d52c34e037dc13adfe0a3efb8d Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 6 Apr 2020 14:51:00 +0000 Subject: [PATCH 08/25] add nested regression test --- .../async-await/issues/issue-69307-nested.rs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 src/test/ui/async-await/issues/issue-69307-nested.rs diff --git a/src/test/ui/async-await/issues/issue-69307-nested.rs b/src/test/ui/async-await/issues/issue-69307-nested.rs new file mode 100644 index 0000000000000..b7cdf3987f1cb --- /dev/null +++ b/src/test/ui/async-await/issues/issue-69307-nested.rs @@ -0,0 +1,30 @@ +// Regression test for #69307 +// +// Having a `async { .. foo.await .. }` block appear inside of a `+=` +// expression was causing an ICE due to a failure to save/restore +// state in the AST numbering pass when entering a nested body. +// +// check-pass +// edition:2018 + +fn block_on(_: F) -> usize { + 0 +} + +fn main() {} + +async fn bar() { + let mut sum = 0; + sum += { + block_on(async { + baz().await; + let mut inner = 1; + inner += block_on(async { + baz().await; + 0 + }) + }) + }; +} + +async fn baz() {} From ce25dabc66d4b7905dba3bf63ad766d9d6f421ab Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 5 Apr 2020 02:11:29 +0300 Subject: [PATCH 09/25] linker: Make argument building interface in `trait Linker` richer by redirecting everything to `Command` --- src/librustc_codegen_ssa/back/link.rs | 9 +++-- src/librustc_codegen_ssa/back/linker.rs | 47 +++++++++++++++---------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 49786bc3b068d..9ac318fbe94d8 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -787,7 +787,7 @@ fn link_sanitizer_runtime(sess: &Session, crate_type: config::CrateType, linker: // PR #41352 for details). let libname = format!("rustc{}_rt.{}", channel, name); let rpath = default_tlib.to_str().expect("non-utf8 component in path"); - linker.args(&["-Wl,-rpath".into(), "-Xlinker".into(), rpath.into()]); + linker.args(&["-Wl,-rpath", "-Xlinker", rpath]); linker.link_dylib(Symbol::intern(&libname)); } "x86_64-unknown-linux-gnu" | "x86_64-fuchsia" | "aarch64-fuchsia" => { @@ -1300,8 +1300,7 @@ fn link_args<'a, B: ArchiveBuilder<'a>>( } let attr_link_args = codegen_results.crate_info.link_args.iter(); - let user_link_args: Vec<_> = - sess.opts.cg.link_args.iter().chain(attr_link_args).cloned().collect(); + let user_link_args = sess.opts.cg.link_args.iter().chain(attr_link_args); if crate_type == config::CrateType::Executable { let mut position_independent_executable = false; @@ -1309,7 +1308,7 @@ fn link_args<'a, B: ArchiveBuilder<'a>>( if t.options.position_independent_executables { if is_pic(sess) && !sess.crt_static(Some(crate_type)) - && !user_link_args.iter().any(|x| x == "-static") + && !user_link_args.clone().any(|x| x == "-static") { position_independent_executable = true; } @@ -1440,7 +1439,7 @@ fn link_args<'a, B: ArchiveBuilder<'a>>( // Finally add all the linker arguments provided on the command line along // with any #[link_args] attributes found inside the crate - cmd.args(&user_link_args); + cmd.args(user_link_args); } // # Native library linking diff --git a/src/librustc_codegen_ssa/back/linker.rs b/src/librustc_codegen_ssa/back/linker.rs index c0c533524b098..da3e805698f97 100644 --- a/src/librustc_codegen_ssa/back/linker.rs +++ b/src/librustc_codegen_ssa/back/linker.rs @@ -87,6 +87,7 @@ impl LinkerInfo { /// used to dispatch on whether a GNU-like linker (generally `ld.exe`) or an /// MSVC linker (e.g., `link.exe`) is being used. pub trait Linker { + fn cmd(&mut self) -> &mut Command; fn link_dylib(&mut self, lib: Symbol); fn link_rust_dylib(&mut self, lib: Symbol, path: &Path); fn link_framework(&mut self, framework: Symbol); @@ -111,7 +112,6 @@ pub trait Linker { fn no_default_libraries(&mut self); fn build_dylib(&mut self, out_filename: &Path); fn build_static_executable(&mut self); - fn args(&mut self, args: &[String]); fn export_symbols(&mut self, tmpdir: &Path, crate_type: CrateType); fn subsystem(&mut self, subsystem: &str); fn group_start(&mut self); @@ -121,6 +121,16 @@ pub trait Linker { fn finalize(&mut self) -> Command; } +impl dyn Linker + '_ { + pub fn arg(&mut self, arg: impl AsRef) { + self.cmd().arg(arg); + } + + pub fn args(&mut self, args: impl IntoIterator>) { + self.cmd().args(args); + } +} + pub struct GccLinker<'a> { cmd: Command, sess: &'a Session, @@ -208,6 +218,9 @@ impl<'a> GccLinker<'a> { } impl<'a> Linker for GccLinker<'a> { + fn cmd(&mut self) -> &mut Command { + &mut self.cmd + } fn link_dylib(&mut self, lib: Symbol) { self.hint_dynamic(); self.cmd.arg(format!("-l{}", lib)); @@ -251,9 +264,6 @@ impl<'a> Linker for GccLinker<'a> { fn build_static_executable(&mut self) { self.cmd.arg("-static"); } - fn args(&mut self, args: &[String]) { - self.cmd.args(args); - } fn link_rust_dylib(&mut self, lib: Symbol, _path: &Path) { self.hint_dynamic(); @@ -545,15 +555,15 @@ pub struct MsvcLinker<'a> { } impl<'a> Linker for MsvcLinker<'a> { + fn cmd(&mut self) -> &mut Command { + &mut self.cmd + } fn link_rlib(&mut self, lib: &Path) { self.cmd.arg(lib); } fn add_object(&mut self, path: &Path) { self.cmd.arg(path); } - fn args(&mut self, args: &[String]) { - self.cmd.args(args); - } fn build_dylib(&mut self, out_filename: &Path) { self.cmd.arg("/DLL"); @@ -778,6 +788,9 @@ pub struct EmLinker<'a> { } impl<'a> Linker for EmLinker<'a> { + fn cmd(&mut self) -> &mut Command { + &mut self.cmd + } fn include_path(&mut self, path: &Path) { self.cmd.arg("-L").arg(path); } @@ -837,10 +850,6 @@ impl<'a> Linker for EmLinker<'a> { // noop } - fn args(&mut self, args: &[String]) { - self.cmd.args(args); - } - fn framework_path(&mut self, _path: &Path) { bug!("frameworks are not supported on Emscripten") } @@ -992,6 +1001,10 @@ impl<'a> WasmLd<'a> { } impl<'a> Linker for WasmLd<'a> { + fn cmd(&mut self) -> &mut Command { + &mut self.cmd + } + fn link_dylib(&mut self, lib: Symbol) { self.cmd.arg("-l").sym_arg(lib); } @@ -1030,10 +1043,6 @@ impl<'a> Linker for WasmLd<'a> { fn build_static_executable(&mut self) {} - fn args(&mut self, args: &[String]) { - self.cmd.args(args); - } - fn link_rust_dylib(&mut self, lib: Symbol, _path: &Path) { self.cmd.arg("-l").sym_arg(lib); } @@ -1162,6 +1171,10 @@ pub struct PtxLinker<'a> { } impl<'a> Linker for PtxLinker<'a> { + fn cmd(&mut self) -> &mut Command { + &mut self.cmd + } + fn link_rlib(&mut self, path: &Path) { self.cmd.arg("--rlib").arg(path); } @@ -1182,10 +1195,6 @@ impl<'a> Linker for PtxLinker<'a> { self.cmd.arg("--bitcode").arg(path); } - fn args(&mut self, args: &[String]) { - self.cmd.args(args); - } - fn optimize(&mut self) { match self.sess.lto() { Lto::Thin | Lto::Fat | Lto::ThinLocal => { From 032462e06f7ef393bac06a76a62fe9ad3f4290b7 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 5 Apr 2020 02:58:32 +0300 Subject: [PATCH 10/25] linker: Combine argument building into a single function --- src/librustc_codegen_ssa/back/link.rs | 205 +++++++++++++----------- src/librustc_codegen_ssa/back/linker.rs | 28 ++-- 2 files changed, 118 insertions(+), 115 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 9ac318fbe94d8..0c418b7e3d708 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -154,7 +154,7 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>( // The third parameter is for env vars, used on windows to set up the // path for MSVC to find its DLLs, and gcc to find its bundled // toolchain -pub fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> (PathBuf, Command) { +pub fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> Command { let msvc_tool = windows_registry::find_tool(&sess.opts.target_triple.triple(), "link.exe"); // If our linker looks like a batch script on Windows then to execute this @@ -232,7 +232,7 @@ pub fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> (PathB } cmd.env("PATH", env::join_paths(new_path).unwrap()); - (linker.to_path_buf(), cmd) + cmd } pub fn each_linked_rlib( @@ -487,95 +487,18 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>( target_cpu: &str, ) { info!("preparing {:?} to {:?}", crate_type, out_filename); - let (linker, flavor) = linker_and_flavor(sess); - - let any_dynamic_crate = crate_type == config::CrateType::Dylib - || codegen_results.crate_info.dependency_formats.iter().any(|(ty, list)| { - *ty == crate_type && list.iter().any(|&linkage| linkage == Linkage::Dynamic) - }); - - // The invocations of cc share some flags across platforms - let (pname, mut cmd) = get_linker(sess, &linker, flavor); - - if let Some(args) = sess.target.target.options.pre_link_args.get(&flavor) { - cmd.args(args); - } - if let Some(args) = sess.target.target.options.pre_link_args_crt.get(&flavor) { - if sess.crt_static(Some(crate_type)) { - cmd.args(args); - } - } - cmd.args(&sess.opts.debugging_opts.pre_link_args); - - if sess.target.target.options.is_like_fuchsia { - let prefix = match sess.opts.debugging_opts.sanitizer { - Some(Sanitizer::Address) => "asan/", - _ => "", - }; - cmd.arg(format!("--dynamic-linker={}ld.so.1", prefix)); - } - - let pre_link_objects = if crate_type == config::CrateType::Executable { - &sess.target.target.options.pre_link_objects_exe - } else { - &sess.target.target.options.pre_link_objects_dll - }; - for obj in pre_link_objects { - cmd.arg(get_file_path(sess, obj)); - } - - if crate_type == config::CrateType::Executable && sess.crt_static(Some(crate_type)) { - for obj in &sess.target.target.options.pre_link_objects_exe_crt { - cmd.arg(get_file_path(sess, obj)); - } - } - - if sess.target.target.options.is_like_emscripten { - cmd.arg("-s"); - cmd.arg(if sess.panic_strategy() == PanicStrategy::Abort { - "DISABLE_EXCEPTION_CATCHING=1" - } else { - "DISABLE_EXCEPTION_CATCHING=0" - }); - } + let (linker_path, flavor) = linker_and_flavor(sess); + let mut cmd = linker_with_args::( + &linker_path, + flavor, + sess, + crate_type, + tmpdir, + out_filename, + codegen_results, + target_cpu, + ); - { - let mut linker = codegen_results.linker_info.to_linker(cmd, &sess, flavor, target_cpu); - link_sanitizer_runtime(sess, crate_type, &mut *linker); - link_args::( - &mut *linker, - flavor, - sess, - crate_type, - tmpdir, - out_filename, - codegen_results, - ); - cmd = linker.finalize(); - } - if let Some(args) = sess.target.target.options.late_link_args.get(&flavor) { - cmd.args(args); - } - if any_dynamic_crate { - if let Some(args) = sess.target.target.options.late_link_args_dynamic.get(&flavor) { - cmd.args(args); - } - } else { - if let Some(args) = sess.target.target.options.late_link_args_static.get(&flavor) { - cmd.args(args); - } - } - for obj in &sess.target.target.options.post_link_objects { - cmd.arg(get_file_path(sess, obj)); - } - if sess.crt_static(Some(crate_type)) { - for obj in &sess.target.target.options.post_link_objects_crt { - cmd.arg(get_file_path(sess, obj)); - } - } - if let Some(args) = sess.target.target.options.post_link_args.get(&flavor) { - cmd.args(args); - } for &(ref k, ref v) in &sess.target.target.options.link_env { cmd.env(k, v); } @@ -597,7 +520,7 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>( let mut i = 0; loop { i += 1; - prog = sess.time("run_linker", || exec_linker(sess, &mut cmd, out_filename, tmpdir)); + prog = sess.time("run_linker", || exec_linker(sess, &cmd, out_filename, tmpdir)); let output = match prog { Ok(ref output) => output, Err(_) => break, @@ -698,7 +621,7 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>( output.extend_from_slice(&prog.stdout); sess.struct_err(&format!( "linking with `{}` failed: {}", - pname.display(), + linker_path.display(), prog.status )) .note(&format!("{:?}", &cmd)) @@ -714,9 +637,12 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>( let mut linker_error = { if linker_not_found { - sess.struct_err(&format!("linker `{}` not found", pname.display())) + sess.struct_err(&format!("linker `{}` not found", linker_path.display())) } else { - sess.struct_err(&format!("could not exec the linker `{}`", pname.display())) + sess.struct_err(&format!( + "could not exec the linker `{}`", + linker_path.display() + )) } }; @@ -1087,7 +1013,7 @@ pub fn get_file_path(sess: &Session, name: &str) -> PathBuf { pub fn exec_linker( sess: &Session, - cmd: &mut Command, + cmd: &Command, out_filename: &Path, tmpdir: &Path, ) -> io::Result { @@ -1233,15 +1159,66 @@ pub fn exec_linker( } } -fn link_args<'a, B: ArchiveBuilder<'a>>( - cmd: &mut dyn Linker, +fn linker_with_args<'a, B: ArchiveBuilder<'a>>( + path: &Path, flavor: LinkerFlavor, sess: &'a Session, crate_type: config::CrateType, tmpdir: &Path, out_filename: &Path, codegen_results: &CodegenResults, -) { + target_cpu: &str, +) -> Command { + let base_cmd = get_linker(sess, path, flavor); + // FIXME: Move `/LIBPATH` addition for uwp targets from the linker construction + // to the linker args construction. + assert!(base_cmd.get_args().is_empty() || sess.target.target.target_vendor == "uwp"); + let cmd = &mut *codegen_results.linker_info.to_linker(base_cmd, &sess, flavor, target_cpu); + + if let Some(args) = sess.target.target.options.pre_link_args.get(&flavor) { + cmd.args(args); + } + if let Some(args) = sess.target.target.options.pre_link_args_crt.get(&flavor) { + if sess.crt_static(Some(crate_type)) { + cmd.args(args); + } + } + cmd.args(&sess.opts.debugging_opts.pre_link_args); + + if sess.target.target.options.is_like_fuchsia { + let prefix = match sess.opts.debugging_opts.sanitizer { + Some(Sanitizer::Address) => "asan/", + _ => "", + }; + cmd.arg(format!("--dynamic-linker={}ld.so.1", prefix)); + } + + let pre_link_objects = if crate_type == config::CrateType::Executable { + &sess.target.target.options.pre_link_objects_exe + } else { + &sess.target.target.options.pre_link_objects_dll + }; + for obj in pre_link_objects { + cmd.arg(get_file_path(sess, obj)); + } + + if crate_type == config::CrateType::Executable && sess.crt_static(Some(crate_type)) { + for obj in &sess.target.target.options.pre_link_objects_exe_crt { + cmd.arg(get_file_path(sess, obj)); + } + } + + if sess.target.target.options.is_like_emscripten { + cmd.arg("-s"); + cmd.arg(if sess.panic_strategy() == PanicStrategy::Abort { + "DISABLE_EXCEPTION_CATCHING=1" + } else { + "DISABLE_EXCEPTION_CATCHING=0" + }); + } + + link_sanitizer_runtime(sess, crate_type, cmd); + // Linker plugins should be specified early in the list of arguments cmd.linker_plugin_lto(); @@ -1440,6 +1417,38 @@ fn link_args<'a, B: ArchiveBuilder<'a>>( // Finally add all the linker arguments provided on the command line along // with any #[link_args] attributes found inside the crate cmd.args(user_link_args); + + cmd.finalize(); + + if let Some(args) = sess.target.target.options.late_link_args.get(&flavor) { + cmd.args(args); + } + let any_dynamic_crate = crate_type == config::CrateType::Dylib + || codegen_results.crate_info.dependency_formats.iter().any(|(ty, list)| { + *ty == crate_type && list.iter().any(|&linkage| linkage == Linkage::Dynamic) + }); + if any_dynamic_crate { + if let Some(args) = sess.target.target.options.late_link_args_dynamic.get(&flavor) { + cmd.args(args); + } + } else { + if let Some(args) = sess.target.target.options.late_link_args_static.get(&flavor) { + cmd.args(args); + } + } + for obj in &sess.target.target.options.post_link_objects { + cmd.arg(get_file_path(sess, obj)); + } + if sess.crt_static(Some(crate_type)) { + for obj in &sess.target.target.options.post_link_objects_crt { + cmd.arg(get_file_path(sess, obj)); + } + } + if let Some(args) = sess.target.target.options.post_link_args.get(&flavor) { + cmd.args(args); + } + + cmd.take_cmd() } // # Native library linking diff --git a/src/librustc_codegen_ssa/back/linker.rs b/src/librustc_codegen_ssa/back/linker.rs index da3e805698f97..0baa37ae9f1ab 100644 --- a/src/librustc_codegen_ssa/back/linker.rs +++ b/src/librustc_codegen_ssa/back/linker.rs @@ -6,6 +6,7 @@ use std::ffi::{OsStr, OsString}; use std::fs::{self, File}; use std::io::prelude::*; use std::io::{self, BufWriter}; +use std::mem; use std::path::{Path, PathBuf}; use rustc_data_structures::fx::FxHashMap; @@ -117,8 +118,7 @@ pub trait Linker { fn group_start(&mut self); fn group_end(&mut self); fn linker_plugin_lto(&mut self); - // Should have been finalize(self), but we don't support self-by-value on trait objects (yet?). - fn finalize(&mut self) -> Command; + fn finalize(&mut self); } impl dyn Linker + '_ { @@ -129,6 +129,10 @@ impl dyn Linker + '_ { pub fn args(&mut self, args: impl IntoIterator>) { self.cmd().args(args); } + + pub fn take_cmd(&mut self) -> Command { + mem::replace(self.cmd(), Command::new("")) + } } pub struct GccLinker<'a> { @@ -515,10 +519,8 @@ impl<'a> Linker for GccLinker<'a> { self.linker_arg(&subsystem); } - fn finalize(&mut self) -> Command { + fn finalize(&mut self) { self.hint_dynamic(); // Reset to default before returning the composed command line. - - ::std::mem::replace(&mut self.cmd, Command::new("")) } fn group_start(&mut self) { @@ -768,9 +770,7 @@ impl<'a> Linker for MsvcLinker<'a> { } } - fn finalize(&mut self) -> Command { - ::std::mem::replace(&mut self.cmd, Command::new("")) - } + fn finalize(&mut self) {} // MSVC doesn't need group indicators fn group_start(&mut self) {} @@ -937,9 +937,7 @@ impl<'a> Linker for EmLinker<'a> { // noop } - fn finalize(&mut self) -> Command { - ::std::mem::replace(&mut self.cmd, Command::new("")) - } + fn finalize(&mut self) {} // Appears not necessary on Emscripten fn group_start(&mut self) {} @@ -1107,9 +1105,7 @@ impl<'a> Linker for WasmLd<'a> { fn no_position_independent_executable(&mut self) {} - fn finalize(&mut self) -> Command { - ::std::mem::replace(&mut self.cmd, Command::new("")) - } + fn finalize(&mut self) {} // Not needed for now with LLD fn group_start(&mut self) {} @@ -1209,14 +1205,12 @@ impl<'a> Linker for PtxLinker<'a> { self.cmd.arg("-o").arg(path); } - fn finalize(&mut self) -> Command { + fn finalize(&mut self) { // Provide the linker with fallback to internal `target-cpu`. self.cmd.arg("--fallback-arch").arg(match self.sess.opts.cg.target_cpu { Some(ref s) => s, None => &self.sess.target.target.options.cpu, }); - - ::std::mem::replace(&mut self.cmd, Command::new("")) } fn link_dylib(&mut self, _lib: Symbol) { From 927db7d3224646f7946d38de37cdbc388b925024 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 5 Apr 2020 17:03:30 +0300 Subject: [PATCH 11/25] linker: Factor out linking of pre- and post-link objects --- src/librustc_codegen_ssa/back/link.rs | 60 +++++++++++++++++---------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 0c418b7e3d708..7f2020f98024d 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -987,7 +987,7 @@ fn get_crt_libs_path(sess: &Session) -> Option { } } -pub fn get_file_path(sess: &Session, name: &str) -> PathBuf { +fn get_object_file_path(sess: &Session, name: &str) -> PathBuf { // prefer system {,dll}crt2.o libs, see get_crt_libs_path comment for more details if sess.target.target.llvm_target.contains("windows-gnu") { if let Some(compiler_libs_path) = get_crt_libs_path(sess) { @@ -1159,6 +1159,36 @@ pub fn exec_linker( } } +/// Add begin object files defined by the target spec. +fn add_pre_link_objects(cmd: &mut dyn Linker, sess: &'a Session, crate_type: config::CrateType) { + let pre_link_objects = if crate_type == config::CrateType::Executable { + &sess.target.target.options.pre_link_objects_exe + } else { + &sess.target.target.options.pre_link_objects_dll + }; + for obj in pre_link_objects { + cmd.add_object(&get_object_file_path(sess, obj)); + } + + if crate_type == config::CrateType::Executable && sess.crt_static(Some(crate_type)) { + for obj in &sess.target.target.options.pre_link_objects_exe_crt { + cmd.add_object(&get_object_file_path(sess, obj)); + } + } +} + +/// Add end object files defined by the target spec. +fn add_post_link_objects(cmd: &mut dyn Linker, sess: &'a Session, crate_type: config::CrateType) { + for obj in &sess.target.target.options.post_link_objects { + cmd.add_object(&get_object_file_path(sess, obj)); + } + if sess.crt_static(Some(crate_type)) { + for obj in &sess.target.target.options.post_link_objects_crt { + cmd.add_object(&get_object_file_path(sess, obj)); + } + } +} + fn linker_with_args<'a, B: ArchiveBuilder<'a>>( path: &Path, flavor: LinkerFlavor, @@ -1193,20 +1223,8 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( cmd.arg(format!("--dynamic-linker={}ld.so.1", prefix)); } - let pre_link_objects = if crate_type == config::CrateType::Executable { - &sess.target.target.options.pre_link_objects_exe - } else { - &sess.target.target.options.pre_link_objects_dll - }; - for obj in pre_link_objects { - cmd.arg(get_file_path(sess, obj)); - } - - if crate_type == config::CrateType::Executable && sess.crt_static(Some(crate_type)) { - for obj in &sess.target.target.options.pre_link_objects_exe_crt { - cmd.arg(get_file_path(sess, obj)); - } - } + // NO-OPT-OUT + add_pre_link_objects(cmd, sess, crate_type); if sess.target.target.options.is_like_emscripten { cmd.arg("-s"); @@ -1436,14 +1454,10 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( cmd.args(args); } } - for obj in &sess.target.target.options.post_link_objects { - cmd.arg(get_file_path(sess, obj)); - } - if sess.crt_static(Some(crate_type)) { - for obj in &sess.target.target.options.post_link_objects_crt { - cmd.arg(get_file_path(sess, obj)); - } - } + + // NO-OPT-OUT + add_post_link_objects(cmd, sess, crate_type); + if let Some(args) = sess.target.target.options.post_link_args.get(&flavor) { cmd.args(args); } From 7f42d81ea489c53a93f4d2b6e59b8ddeb4ee749a Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 5 Apr 2020 17:31:29 +0300 Subject: [PATCH 12/25] linker: Factor out addition of pre-, post- and late link args --- src/librustc_codegen_ssa/back/link.rs | 117 ++++++++++++++++++-------- 1 file changed, 82 insertions(+), 35 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 7f2020f98024d..39355baebee7e 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -1189,6 +1189,74 @@ fn add_post_link_objects(cmd: &mut dyn Linker, sess: &'a Session, crate_type: co } } +/// Add arbitrary "pre-link" args defined by the target spec or from command line. +/// FIXME: Determine where exactly these args need to be inserted. +fn add_pre_link_args( + cmd: &mut dyn Linker, + sess: &'a Session, + flavor: LinkerFlavor, + crate_type: config::CrateType, +) { + if let Some(args) = sess.target.target.options.pre_link_args.get(&flavor) { + cmd.args(args); + } + if let Some(args) = sess.target.target.options.pre_link_args_crt.get(&flavor) { + if sess.crt_static(Some(crate_type)) { + cmd.args(args); + } + } + cmd.args(&sess.opts.debugging_opts.pre_link_args); +} + +/// Add arbitrary "user defined" args defined from command line and by `#[link_args]` attributes. +/// FIXME: Determine where exactly these args need to be inserted. +fn add_user_defined_link_args( + cmd: &mut dyn Linker, + sess: &'a Session, + codegen_results: &CodegenResults, +) { + cmd.args(&sess.opts.cg.link_args); + cmd.args(&*codegen_results.crate_info.link_args); +} + +/// Add arbitrary "late link" args defined by the target spec. +/// FIXME: Determine where exactly these args need to be inserted. +fn add_late_link_args( + cmd: &mut dyn Linker, + sess: &'a Session, + flavor: LinkerFlavor, + crate_type: config::CrateType, + codegen_results: &CodegenResults, +) { + if let Some(args) = sess.target.target.options.late_link_args.get(&flavor) { + cmd.args(args); + } + let any_dynamic_crate = crate_type == config::CrateType::Dylib + || codegen_results.crate_info.dependency_formats.iter().any(|(ty, list)| { + *ty == crate_type && list.iter().any(|&linkage| linkage == Linkage::Dynamic) + }); + if any_dynamic_crate { + if let Some(args) = sess.target.target.options.late_link_args_dynamic.get(&flavor) { + cmd.args(args); + } + } else { + if let Some(args) = sess.target.target.options.late_link_args_static.get(&flavor) { + cmd.args(args); + } + } +} + +/// Add arbitrary "post-link" args defined by the target spec. +/// FIXME: Determine where exactly these args need to be inserted. +fn add_post_link_args(cmd: &mut dyn Linker, sess: &'a Session, flavor: LinkerFlavor) { + if let Some(args) = sess.target.target.options.post_link_args.get(&flavor) { + cmd.args(args); + } +} + +/// Produce the linker command line containing linker path and arguments. +/// `NO-OPT-OUT` marks the arguments that cannot be removed from the command line +/// by the user without creating a custom target specification. fn linker_with_args<'a, B: ArchiveBuilder<'a>>( path: &Path, flavor: LinkerFlavor, @@ -1205,15 +1273,8 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( assert!(base_cmd.get_args().is_empty() || sess.target.target.target_vendor == "uwp"); let cmd = &mut *codegen_results.linker_info.to_linker(base_cmd, &sess, flavor, target_cpu); - if let Some(args) = sess.target.target.options.pre_link_args.get(&flavor) { - cmd.args(args); - } - if let Some(args) = sess.target.target.options.pre_link_args_crt.get(&flavor) { - if sess.crt_static(Some(crate_type)) { - cmd.args(args); - } - } - cmd.args(&sess.opts.debugging_opts.pre_link_args); + // NO-OPT-OUT + add_pre_link_args(cmd, sess, flavor, crate_type); if sess.target.target.options.is_like_fuchsia { let prefix = match sess.opts.debugging_opts.sanitizer { @@ -1294,16 +1355,19 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( cmd.gc_sections(keep_metadata); } - let attr_link_args = codegen_results.crate_info.link_args.iter(); - let user_link_args = sess.opts.cg.link_args.iter().chain(attr_link_args); - if crate_type == config::CrateType::Executable { let mut position_independent_executable = false; if t.options.position_independent_executables { if is_pic(sess) && !sess.crt_static(Some(crate_type)) - && !user_link_args.clone().any(|x| x == "-static") + && !sess + .opts + .cg + .link_args + .iter() + .chain(&*codegen_results.crate_info.link_args) + .any(|x| x == "-static") { position_independent_executable = true; } @@ -1432,35 +1496,18 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( cmd.args(&rpath::get_rpath_flags(&mut rpath_config)); } - // Finally add all the linker arguments provided on the command line along - // with any #[link_args] attributes found inside the crate - cmd.args(user_link_args); + add_user_defined_link_args(cmd, sess, codegen_results); cmd.finalize(); - if let Some(args) = sess.target.target.options.late_link_args.get(&flavor) { - cmd.args(args); - } - let any_dynamic_crate = crate_type == config::CrateType::Dylib - || codegen_results.crate_info.dependency_formats.iter().any(|(ty, list)| { - *ty == crate_type && list.iter().any(|&linkage| linkage == Linkage::Dynamic) - }); - if any_dynamic_crate { - if let Some(args) = sess.target.target.options.late_link_args_dynamic.get(&flavor) { - cmd.args(args); - } - } else { - if let Some(args) = sess.target.target.options.late_link_args_static.get(&flavor) { - cmd.args(args); - } - } + // NO-OPT-OUT + add_late_link_args(cmd, sess, flavor, crate_type, codegen_results); // NO-OPT-OUT add_post_link_objects(cmd, sess, crate_type); - if let Some(args) = sess.target.target.options.post_link_args.get(&flavor) { - cmd.args(args); - } + // NO-OPT-OUT + add_post_link_args(cmd, sess, flavor); cmd.take_cmd() } From fd6fa686df4ec661048e39c14c9295a61a19e447 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 5 Apr 2020 18:10:55 +0300 Subject: [PATCH 13/25] linker: Add more markup and comments to code producing linker arguments --- src/librustc_codegen_ssa/back/link.rs | 84 +++++++++++++++++++-------- 1 file changed, 61 insertions(+), 23 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 39355baebee7e..556a7b85cca70 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -193,6 +193,7 @@ pub fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> Comman _ => None, }; if let Some(ref a) = arch { + // FIXME: Move this to `fn linker_with_args`. let mut arg = OsString::from("/LIBPATH:"); arg.push(format!("{}\\lib\\{}\\store", root_lib_path.display(), a.to_string())); cmd.arg(&arg); @@ -1254,9 +1255,29 @@ fn add_post_link_args(cmd: &mut dyn Linker, sess: &'a Session, flavor: LinkerFla } } +/// Add sysroot and other globally set directories to the directory search list. +fn add_library_search_dirs(cmd: &mut dyn Linker, sess: &'a Session) { + // The default library location, we need this to find the runtime. + // The location of crates will be determined as needed. + let lib_path = sess.target_filesearch(PathKind::All).get_lib_path(); + + // prefer system mingw-w64 libs, see get_crt_libs_path comment for more details + if cfg!(windows) && sess.target.target.llvm_target.contains("windows-gnu") { + if let Some(compiler_libs_path) = get_crt_libs_path(sess) { + cmd.include_path(&compiler_libs_path); + } + } + + cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); +} + /// Produce the linker command line containing linker path and arguments. /// `NO-OPT-OUT` marks the arguments that cannot be removed from the command line /// by the user without creating a custom target specification. +/// `OBJECT-FILES` specify whether the arguments can add object files. +/// `CUSTOMIZATION-POINT` means that arbitrary arguments defined by the user +/// or by the target spec can be inserted here. +/// `AUDIT-ORDER` - need to figure out whether the option is order-dependent or not. fn linker_with_args<'a, B: ArchiveBuilder<'a>>( path: &Path, flavor: LinkerFlavor, @@ -1273,9 +1294,10 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( assert!(base_cmd.get_args().is_empty() || sess.target.target.target_vendor == "uwp"); let cmd = &mut *codegen_results.linker_info.to_linker(base_cmd, &sess, flavor, target_cpu); - // NO-OPT-OUT + // NO-OPT-OUT, OBJECT-FILES-MAYBE, CUSTOMIZATION-POINT add_pre_link_args(cmd, sess, flavor, crate_type); + // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER if sess.target.target.options.is_like_fuchsia { let prefix = match sess.opts.debugging_opts.sanitizer { Some(Sanitizer::Address) => "asan/", @@ -1284,9 +1306,10 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( cmd.arg(format!("--dynamic-linker={}ld.so.1", prefix)); } - // NO-OPT-OUT + // NO-OPT-OUT, OBJECT-FILES-YES add_pre_link_objects(cmd, sess, crate_type); + // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER if sess.target.target.options.is_like_emscripten { cmd.arg("-s"); cmd.arg(if sess.panic_strategy() == PanicStrategy::Abort { @@ -1296,43 +1319,40 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( }); } + // OBJECT-FILES-YES, AUDIT-ORDER link_sanitizer_runtime(sess, crate_type, cmd); + // OBJECT-FILES-NO, AUDIT-ORDER // Linker plugins should be specified early in the list of arguments + // FIXME: How "early" exactly? cmd.linker_plugin_lto(); - // The default library location, we need this to find the runtime. - // The location of crates will be determined as needed. - let lib_path = sess.target_filesearch(PathKind::All).get_lib_path(); - - // target descriptor - let t = &sess.target.target; - - // prefer system mingw-w64 libs, see get_crt_libs_path comment for more details - if cfg!(windows) && sess.target.target.llvm_target.contains("windows-gnu") { - if let Some(compiler_libs_path) = get_crt_libs_path(sess) { - cmd.include_path(&compiler_libs_path); - } - } - - cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); + // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER + // FIXME: Order-dependent, at least relatively to other args adding searh directories. + add_library_search_dirs(cmd, sess); + // OBJECT-FILES-YES for obj in codegen_results.modules.iter().filter_map(|m| m.object.as_ref()) { cmd.add_object(obj); } + + // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER cmd.output_filename(out_filename); + // OBJECT-FILES-NO, AUDIT-ORDER if crate_type == config::CrateType::Executable && sess.target.target.options.is_like_windows { if let Some(ref s) = codegen_results.windows_subsystem { cmd.subsystem(s); } } + // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER // If we're building something like a dynamic library then some platforms // need to make sure that all symbols are exported correctly from the // dynamic library. cmd.export_symbols(tmpdir, crate_type); + // OBJECT-FILES-YES // When linking a dynamic library, we put the metadata into a section of the // executable. This metadata is in a separate object file from the main // object file, so we link that in here. @@ -1343,11 +1363,14 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( } } + // OBJECT-FILES-YES let obj = codegen_results.allocator_module.as_ref().and_then(|m| m.object.as_ref()); if let Some(obj) = obj { cmd.add_object(obj); } + // OBJECT-FILES-NO, AUDIT-ORDER + // FIXME: Order dependent, applies to the following objects. Where should it be placed? // Try to strip as much out of the generated object by removing unused // sections if possible. See more comments in linker.rs if !sess.opts.cg.link_dead_code { @@ -1355,10 +1378,11 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( cmd.gc_sections(keep_metadata); } + // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER if crate_type == config::CrateType::Executable { let mut position_independent_executable = false; - if t.options.position_independent_executables { + if sess.target.target.options.position_independent_executables { if is_pic(sess) && !sess.crt_static(Some(crate_type)) && !sess @@ -1385,9 +1409,10 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( } } + // OBJECT-FILES-NO, AUDIT-ORDER let relro_level = match sess.opts.debugging_opts.relro_level { Some(level) => level, - None => t.options.relro_level, + None => sess.target.target.options.relro_level, }; match relro_level { RelroLevel::Full => { @@ -1402,12 +1427,15 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( RelroLevel::None => {} } + // OBJECT-FILES-NO, AUDIT-ORDER // Pass optimization flags down to the linker. cmd.optimize(); + // OBJECT-FILES-NO, AUDIT-ORDER // Pass debuginfo flags down to the linker. cmd.debuginfo(); + // OBJECT-FILES-NO, AUDIT-ORDER // We want to, by default, prevent the compiler from accidentally leaking in // any system libraries, so we may explicitly ask linkers to not link to any // libraries by default. Note that this does not happen for windows because @@ -1415,10 +1443,13 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( // figure out which subset we wanted. // // This is all naturally configurable via the standard methods as well. - if !sess.opts.cg.default_linker_libraries.unwrap_or(false) && t.options.no_default_libraries { + if !sess.opts.cg.default_linker_libraries.unwrap_or(false) + && sess.target.target.options.no_default_libraries + { cmd.no_default_libraries(); } + // OBJECT-FILES-YES // Take careful note of the ordering of the arguments we pass to the linker // here. Linkers will assume that things on the left depend on things to the // right. Things on the right cannot depend on things on the left. This is @@ -1456,6 +1487,8 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( if sess.opts.debugging_opts.link_native_libraries.unwrap_or(true) { add_upstream_native_libraries(cmd, sess, codegen_results, crate_type); } + + // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER // Tell the linker what we're doing. if crate_type != config::CrateType::Executable { cmd.build_dylib(out_filename); @@ -1464,14 +1497,17 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( cmd.build_static_executable(); } + // OBJECT-FILES-NO, AUDIT-ORDER if sess.opts.cg.profile_generate.enabled() { cmd.pgo_gen(); } + // OBJECT-FILES-NO, AUDIT-ORDER if sess.opts.debugging_opts.control_flow_guard != CFGuard::Disabled { cmd.control_flow_guard(); } + // OBJECT-FILES-NO, AUDIT-ORDER // FIXME (#2397): At some point we want to rpath our guesses as to // where extern libraries might live, based on the // addl_lib_search_paths @@ -1496,17 +1532,19 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( cmd.args(&rpath::get_rpath_flags(&mut rpath_config)); } + // OBJECT-FILES-MAYBE, CUSTOMIZATION-POINT add_user_defined_link_args(cmd, sess, codegen_results); + // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER cmd.finalize(); - // NO-OPT-OUT + // NO-OPT-OUT, OBJECT-FILES-MAYBE, CUSTOMIZATION-POINT add_late_link_args(cmd, sess, flavor, crate_type, codegen_results); - // NO-OPT-OUT + // NO-OPT-OUT, OBJECT-FILES-YES add_post_link_objects(cmd, sess, crate_type); - // NO-OPT-OUT + // NO-OPT-OUT, OBJECT-FILES-MAYBE, CUSTOMIZATION-POINT add_post_link_args(cmd, sess, flavor); cmd.take_cmd() From 379c255eb926e4bb577121c84437ab3934aa4475 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 6 Apr 2020 01:33:03 +0300 Subject: [PATCH 14/25] linker: Factor out more parts of `linker_with_args` and add some docs --- src/librustc_codegen_ssa/back/link.rs | 304 ++++++++++++++++---------- 1 file changed, 183 insertions(+), 121 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 556a7b85cca70..9a434b39b9a14 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -1255,6 +1255,87 @@ fn add_post_link_args(cmd: &mut dyn Linker, sess: &'a Session, flavor: LinkerFla } } +/// Add object files containing code from the current crate. +fn add_local_crate_regular_objects(cmd: &mut dyn Linker, codegen_results: &CodegenResults) { + for obj in codegen_results.modules.iter().filter_map(|m| m.object.as_ref()) { + cmd.add_object(obj); + } +} + +/// Add object files for allocator code linked once for the whole crate tree. +fn add_local_crate_allocator_objects(cmd: &mut dyn Linker, codegen_results: &CodegenResults) { + let obj = codegen_results.allocator_module.as_ref().and_then(|m| m.object.as_ref()); + if let Some(obj) = obj { + cmd.add_object(obj); + } +} + +/// Add object files containing metadata for the current crate. +fn add_local_crate_metadata_objects( + cmd: &mut dyn Linker, + crate_type: config::CrateType, + codegen_results: &CodegenResults, +) { + // When linking a dynamic library, we put the metadata into a section of the + // executable. This metadata is in a separate object file from the main + // object file, so we link that in here. + if crate_type == config::CrateType::Dylib || crate_type == config::CrateType::ProcMacro { + let obj = codegen_results.metadata_module.as_ref().and_then(|m| m.object.as_ref()); + if let Some(obj) = obj { + cmd.add_object(obj); + } + } +} + +/// Link native libraries corresponding to the current crate and all libraries corresponding to +/// all its dependency crates. +/// FIXME: Consider combining this with the functions above adding object files for the local crate. +fn link_local_crate_native_libs_and_dependent_crate_libs<'a, B: ArchiveBuilder<'a>>( + cmd: &mut dyn Linker, + sess: &'a Session, + crate_type: config::CrateType, + codegen_results: &CodegenResults, + tmpdir: &Path, +) { + // Take careful note of the ordering of the arguments we pass to the linker + // here. Linkers will assume that things on the left depend on things to the + // right. Things on the right cannot depend on things on the left. This is + // all formally implemented in terms of resolving symbols (libs on the right + // resolve unknown symbols of libs on the left, but not vice versa). + // + // For this reason, we have organized the arguments we pass to the linker as + // such: + // + // 1. The local object that LLVM just generated + // 2. Local native libraries + // 3. Upstream rust libraries + // 4. Upstream native libraries + // + // The rationale behind this ordering is that those items lower down in the + // list can't depend on items higher up in the list. For example nothing can + // depend on what we just generated (e.g., that'd be a circular dependency). + // Upstream rust libraries are not allowed to depend on our local native + // libraries as that would violate the structure of the DAG, in that + // scenario they are required to link to them as well in a shared fashion. + // + // Note that upstream rust libraries may contain native dependencies as + // well, but they also can't depend on what we just started to add to the + // link line. And finally upstream native libraries can't depend on anything + // in this DAG so far because they're only dylibs and dylibs can only depend + // on other dylibs (e.g., other native deps). + // + // If -Zlink-native-libraries=false is set, then the assumption is that an + // external build system already has the native dependencies defined, and it + // will provide them to the linker itself. + if sess.opts.debugging_opts.link_native_libraries.unwrap_or(true) { + add_local_native_libraries(cmd, sess, codegen_results); + } + add_upstream_rust_crates::(cmd, sess, codegen_results, crate_type, tmpdir); + if sess.opts.debugging_opts.link_native_libraries.unwrap_or(true) { + add_upstream_native_libraries(cmd, sess, codegen_results, crate_type); + } +} + /// Add sysroot and other globally set directories to the directory search list. fn add_library_search_dirs(cmd: &mut dyn Linker, sess: &'a Session) { // The default library location, we need this to find the runtime. @@ -1271,6 +1352,95 @@ fn add_library_search_dirs(cmd: &mut dyn Linker, sess: &'a Session) { cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); } +/// Add options requesting executables to be position-independent or not position-independent. +fn add_position_independent_executable_args( + cmd: &mut dyn Linker, + sess: &'a Session, + flavor: LinkerFlavor, + crate_type: config::CrateType, + codegen_results: &CodegenResults, +) { + if crate_type != config::CrateType::Executable { + return; + } + + let mut position_independent_executable = false; + if sess.target.target.options.position_independent_executables { + let attr_link_args = &*codegen_results.crate_info.link_args; + let mut user_defined_link_args = sess.opts.cg.link_args.iter().chain(attr_link_args); + if is_pic(sess) + && !sess.crt_static(Some(crate_type)) + && !user_defined_link_args.any(|x| x == "-static") + { + position_independent_executable = true; + } + } + + if position_independent_executable { + cmd.position_independent_executable(); + } else { + // recent versions of gcc can be configured to generate position + // independent executables by default. We have to pass -no-pie to + // explicitly turn that off. Not applicable to ld. + if sess.target.target.options.linker_is_gnu && flavor != LinkerFlavor::Ld { + cmd.no_position_independent_executable(); + } + } +} + +/// Add options making relocation sections in the produced ELF files read-only +/// and suppressing lazy binding. +fn add_relro_args(cmd: &mut dyn Linker, sess: &'a Session) { + let relro_level = match sess.opts.debugging_opts.relro_level { + Some(level) => level, + None => sess.target.target.options.relro_level, + }; + match relro_level { + RelroLevel::Full => { + cmd.full_relro(); + } + RelroLevel::Partial => { + cmd.partial_relro(); + } + RelroLevel::Off => { + cmd.no_relro(); + } + RelroLevel::None => {} + } +} + +/// Add library search paths used at runtime by dynamic linkers. +fn add_rpath_args( + cmd: &mut dyn Linker, + sess: &'a Session, + codegen_results: &CodegenResults, + out_filename: &Path, +) { + // FIXME (#2397): At some point we want to rpath our guesses as to + // where extern libraries might live, based on the + // addl_lib_search_paths + if sess.opts.cg.rpath { + let target_triple = sess.opts.target_triple.triple(); + let mut get_install_prefix_lib_path = || { + let install_prefix = option_env!("CFG_PREFIX").expect("CFG_PREFIX"); + let tlib = filesearch::relative_target_lib_path(&sess.sysroot, target_triple); + let mut path = PathBuf::from(install_prefix); + path.push(&tlib); + + path + }; + let mut rpath_config = RPathConfig { + used_crates: &codegen_results.crate_info.used_crates_dynamic, + out_filename: out_filename.to_path_buf(), + has_rpath: sess.target.target.options.has_rpath, + is_like_osx: sess.target.target.options.is_like_osx, + linker_is_gnu: sess.target.target.options.linker_is_gnu, + get_install_prefix_lib_path: &mut get_install_prefix_lib_path, + }; + cmd.args(&rpath::get_rpath_flags(&mut rpath_config)); + } +} + /// Produce the linker command line containing linker path and arguments. /// `NO-OPT-OUT` marks the arguments that cannot be removed from the command line /// by the user without creating a custom target specification. @@ -1332,9 +1502,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( add_library_search_dirs(cmd, sess); // OBJECT-FILES-YES - for obj in codegen_results.modules.iter().filter_map(|m| m.object.as_ref()) { - cmd.add_object(obj); - } + add_local_crate_regular_objects(cmd, codegen_results); // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER cmd.output_filename(out_filename); @@ -1353,21 +1521,10 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( cmd.export_symbols(tmpdir, crate_type); // OBJECT-FILES-YES - // When linking a dynamic library, we put the metadata into a section of the - // executable. This metadata is in a separate object file from the main - // object file, so we link that in here. - if crate_type == config::CrateType::Dylib || crate_type == config::CrateType::ProcMacro { - let obj = codegen_results.metadata_module.as_ref().and_then(|m| m.object.as_ref()); - if let Some(obj) = obj { - cmd.add_object(obj); - } - } + add_local_crate_metadata_objects(cmd, crate_type, codegen_results); // OBJECT-FILES-YES - let obj = codegen_results.allocator_module.as_ref().and_then(|m| m.object.as_ref()); - if let Some(obj) = obj { - cmd.add_object(obj); - } + add_local_crate_allocator_objects(cmd, codegen_results); // OBJECT-FILES-NO, AUDIT-ORDER // FIXME: Order dependent, applies to the following objects. Where should it be placed? @@ -1379,53 +1536,10 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( } // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER - if crate_type == config::CrateType::Executable { - let mut position_independent_executable = false; - - if sess.target.target.options.position_independent_executables { - if is_pic(sess) - && !sess.crt_static(Some(crate_type)) - && !sess - .opts - .cg - .link_args - .iter() - .chain(&*codegen_results.crate_info.link_args) - .any(|x| x == "-static") - { - position_independent_executable = true; - } - } - - if position_independent_executable { - cmd.position_independent_executable(); - } else { - // recent versions of gcc can be configured to generate position - // independent executables by default. We have to pass -no-pie to - // explicitly turn that off. Not applicable to ld. - if sess.target.target.options.linker_is_gnu && flavor != LinkerFlavor::Ld { - cmd.no_position_independent_executable(); - } - } - } + add_position_independent_executable_args(cmd, sess, flavor, crate_type, codegen_results); // OBJECT-FILES-NO, AUDIT-ORDER - let relro_level = match sess.opts.debugging_opts.relro_level { - Some(level) => level, - None => sess.target.target.options.relro_level, - }; - match relro_level { - RelroLevel::Full => { - cmd.full_relro(); - } - RelroLevel::Partial => { - cmd.partial_relro(); - } - RelroLevel::Off => { - cmd.no_relro(); - } - RelroLevel::None => {} - } + add_relro_args(cmd, sess); // OBJECT-FILES-NO, AUDIT-ORDER // Pass optimization flags down to the linker. @@ -1450,43 +1564,13 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( } // OBJECT-FILES-YES - // Take careful note of the ordering of the arguments we pass to the linker - // here. Linkers will assume that things on the left depend on things to the - // right. Things on the right cannot depend on things on the left. This is - // all formally implemented in terms of resolving symbols (libs on the right - // resolve unknown symbols of libs on the left, but not vice versa). - // - // For this reason, we have organized the arguments we pass to the linker as - // such: - // - // 1. The local object that LLVM just generated - // 2. Local native libraries - // 3. Upstream rust libraries - // 4. Upstream native libraries - // - // The rationale behind this ordering is that those items lower down in the - // list can't depend on items higher up in the list. For example nothing can - // depend on what we just generated (e.g., that'd be a circular dependency). - // Upstream rust libraries are not allowed to depend on our local native - // libraries as that would violate the structure of the DAG, in that - // scenario they are required to link to them as well in a shared fashion. - // - // Note that upstream rust libraries may contain native dependencies as - // well, but they also can't depend on what we just started to add to the - // link line. And finally upstream native libraries can't depend on anything - // in this DAG so far because they're only dylibs and dylibs can only depend - // on other dylibs (e.g., other native deps). - // - // If -Zlink-native-libraries=false is set, then the assumption is that an - // external build system already has the native dependencies defined, and it - // will provide them to the linker itself. - if sess.opts.debugging_opts.link_native_libraries.unwrap_or(true) { - add_local_native_libraries(cmd, sess, codegen_results); - } - add_upstream_rust_crates::(cmd, sess, codegen_results, crate_type, tmpdir); - if sess.opts.debugging_opts.link_native_libraries.unwrap_or(true) { - add_upstream_native_libraries(cmd, sess, codegen_results, crate_type); - } + link_local_crate_native_libs_and_dependent_crate_libs::( + cmd, + sess, + crate_type, + codegen_results, + tmpdir, + ); // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER // Tell the linker what we're doing. @@ -1508,29 +1592,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( } // OBJECT-FILES-NO, AUDIT-ORDER - // FIXME (#2397): At some point we want to rpath our guesses as to - // where extern libraries might live, based on the - // addl_lib_search_paths - if sess.opts.cg.rpath { - let target_triple = sess.opts.target_triple.triple(); - let mut get_install_prefix_lib_path = || { - let install_prefix = option_env!("CFG_PREFIX").expect("CFG_PREFIX"); - let tlib = filesearch::relative_target_lib_path(&sess.sysroot, target_triple); - let mut path = PathBuf::from(install_prefix); - path.push(&tlib); - - path - }; - let mut rpath_config = RPathConfig { - used_crates: &codegen_results.crate_info.used_crates_dynamic, - out_filename: out_filename.to_path_buf(), - has_rpath: sess.target.target.options.has_rpath, - is_like_osx: sess.target.target.options.is_like_osx, - linker_is_gnu: sess.target.target.options.linker_is_gnu, - get_install_prefix_lib_path: &mut get_install_prefix_lib_path, - }; - cmd.args(&rpath::get_rpath_flags(&mut rpath_config)); - } + add_rpath_args(cmd, sess, codegen_results, out_filename); // OBJECT-FILES-MAYBE, CUSTOMIZATION-POINT add_user_defined_link_args(cmd, sess, codegen_results); From b30d906a983f24e4c03a64b032485ff9950d83b2 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 7 Apr 2020 01:10:49 +0200 Subject: [PATCH 15/25] Add some more comments --- src/librustc_mir_build/lints.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/librustc_mir_build/lints.rs b/src/librustc_mir_build/lints.rs index 3e0028a66d452..74ab602541596 100644 --- a/src/librustc_mir_build/lints.rs +++ b/src/librustc_mir_build/lints.rs @@ -28,7 +28,12 @@ fn check_fn_for_unconditional_recursion<'tcx>( def_id: DefId, ) { let self_calls = find_blocks_calling_self(tcx, &body, def_id); + + // Stores a list of `Span`s for every basic block. Those are the spans of `Call` terminators + // where we know that one of them will definitely be reached. let mut results = IndexVec::from_elem_n(vec![], body.basic_blocks().len()); + + // We start the analysis at the self calls and work backwards. let mut queue: VecDeque<_> = self_calls.iter().collect(); while let Some(bb) = queue.pop_front() { @@ -39,6 +44,8 @@ fn check_fn_for_unconditional_recursion<'tcx>( let locations = if self_calls.contains(bb) { // `bb` *is* a self-call. + // We don't look at successors here because they are irrelevant here and we don't want + // to lint them (eg. `f(); f()` should only lint the first call). vec![bb] } else { // If *all* successors of `bb` lead to a self-call, emit notes at all of their @@ -69,12 +76,16 @@ fn check_fn_for_unconditional_recursion<'tcx>( } }; + // If all our successors are known to lead to self-calls, then we do too. let all_are_self_calls = relevant_successors.clone().all(|&succ| !results[succ].is_empty()); if all_are_self_calls { + // We'll definitely lead to a self-call. Merge all call locations of the successors + // for linting them later. relevant_successors.flat_map(|&succ| results[succ].iter().copied()).collect() } else { + // At least 1 successor does not always lead to a self-call, so we also don't. vec![] } }; From 5a4fa4554f26b1d34c4b336f4609edc65704643e Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 7 Apr 2020 01:48:30 +0300 Subject: [PATCH 16/25] linker: Some minor code cleanup --- src/librustc_codegen_ssa/back/link.rs | 96 +++++++++++---------------- 1 file changed, 38 insertions(+), 58 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 9a434b39b9a14..20e64f0c48851 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -154,7 +154,7 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>( // The third parameter is for env vars, used on windows to set up the // path for MSVC to find its DLLs, and gcc to find its bundled // toolchain -pub fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> Command { +fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> Command { let msvc_tool = windows_registry::find_tool(&sess.opts.target_triple.triple(), "link.exe"); // If our linker looks like a batch script on Windows then to execute this @@ -285,11 +285,7 @@ pub fn each_linked_rlib( /// building an `.rlib` (stomping over one another), or writing an `.rmeta` into a /// directory being searched for `extern crate` (observing an incomplete file). /// The returned path is the temporary file containing the complete metadata. -pub fn emit_metadata<'a>( - sess: &'a Session, - metadata: &EncodedMetadata, - tmpdir: &TempDir, -) -> PathBuf { +pub fn emit_metadata(sess: &Session, metadata: &EncodedMetadata, tmpdir: &TempDir) -> PathBuf { let out_filename = tmpdir.path().join(METADATA_FILENAME); let result = fs::write(&out_filename, &metadata.raw_data); @@ -744,7 +740,7 @@ pub fn ignored_for_lto(sess: &Session, info: &CrateInfo, cnum: CrateNum) -> bool && (info.compiler_builtins == Some(cnum) || info.is_no_builtins.contains(&cnum)) } -pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) { +fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) { fn infer_from( sess: &Session, linker: Option, @@ -832,7 +828,7 @@ pub fn linker_and_flavor(sess: &Session) -> (PathBuf, LinkerFlavor) { /// Returns a boolean indicating whether we should preserve the object files on /// the filesystem for their debug information. This is often useful with /// split-dwarf like schemes. -pub fn preserve_objects_for_their_debuginfo(sess: &Session) -> bool { +fn preserve_objects_for_their_debuginfo(sess: &Session) -> bool { // If the objects don't have debuginfo there's nothing to preserve. if sess.opts.debuginfo == config::DebugInfo::None { return false; @@ -886,7 +882,7 @@ enum RlibFlavor { StaticlibBase, } -pub fn print_native_static_libs(sess: &Session, all_native_libs: &[NativeLibrary]) { +fn print_native_static_libs(sess: &Session, all_native_libs: &[NativeLibrary]) { let lib_args: Vec<_> = all_native_libs .iter() .filter(|l| relevant_lib(sess, l)) @@ -1012,7 +1008,7 @@ fn get_object_file_path(sess: &Session, name: &str) -> PathBuf { PathBuf::from(name) } -pub fn exec_linker( +fn exec_linker( sess: &Session, cmd: &Command, out_filename: &Path, @@ -1161,7 +1157,7 @@ pub fn exec_linker( } /// Add begin object files defined by the target spec. -fn add_pre_link_objects(cmd: &mut dyn Linker, sess: &'a Session, crate_type: config::CrateType) { +fn add_pre_link_objects(cmd: &mut dyn Linker, sess: &Session, crate_type: config::CrateType) { let pre_link_objects = if crate_type == config::CrateType::Executable { &sess.target.target.options.pre_link_objects_exe } else { @@ -1179,7 +1175,7 @@ fn add_pre_link_objects(cmd: &mut dyn Linker, sess: &'a Session, crate_type: con } /// Add end object files defined by the target spec. -fn add_post_link_objects(cmd: &mut dyn Linker, sess: &'a Session, crate_type: config::CrateType) { +fn add_post_link_objects(cmd: &mut dyn Linker, sess: &Session, crate_type: config::CrateType) { for obj in &sess.target.target.options.post_link_objects { cmd.add_object(&get_object_file_path(sess, obj)); } @@ -1194,7 +1190,7 @@ fn add_post_link_objects(cmd: &mut dyn Linker, sess: &'a Session, crate_type: co /// FIXME: Determine where exactly these args need to be inserted. fn add_pre_link_args( cmd: &mut dyn Linker, - sess: &'a Session, + sess: &Session, flavor: LinkerFlavor, crate_type: config::CrateType, ) { @@ -1213,7 +1209,7 @@ fn add_pre_link_args( /// FIXME: Determine where exactly these args need to be inserted. fn add_user_defined_link_args( cmd: &mut dyn Linker, - sess: &'a Session, + sess: &Session, codegen_results: &CodegenResults, ) { cmd.args(&sess.opts.cg.link_args); @@ -1224,7 +1220,7 @@ fn add_user_defined_link_args( /// FIXME: Determine where exactly these args need to be inserted. fn add_late_link_args( cmd: &mut dyn Linker, - sess: &'a Session, + sess: &Session, flavor: LinkerFlavor, crate_type: config::CrateType, codegen_results: &CodegenResults, @@ -1249,7 +1245,7 @@ fn add_late_link_args( /// Add arbitrary "post-link" args defined by the target spec. /// FIXME: Determine where exactly these args need to be inserted. -fn add_post_link_args(cmd: &mut dyn Linker, sess: &'a Session, flavor: LinkerFlavor) { +fn add_post_link_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) { if let Some(args) = sess.target.target.options.post_link_args.get(&flavor) { cmd.args(args); } @@ -1264,8 +1260,7 @@ fn add_local_crate_regular_objects(cmd: &mut dyn Linker, codegen_results: &Codeg /// Add object files for allocator code linked once for the whole crate tree. fn add_local_crate_allocator_objects(cmd: &mut dyn Linker, codegen_results: &CodegenResults) { - let obj = codegen_results.allocator_module.as_ref().and_then(|m| m.object.as_ref()); - if let Some(obj) = obj { + if let Some(obj) = codegen_results.allocator_module.as_ref().and_then(|m| m.object.as_ref()) { cmd.add_object(obj); } } @@ -1280,8 +1275,8 @@ fn add_local_crate_metadata_objects( // executable. This metadata is in a separate object file from the main // object file, so we link that in here. if crate_type == config::CrateType::Dylib || crate_type == config::CrateType::ProcMacro { - let obj = codegen_results.metadata_module.as_ref().and_then(|m| m.object.as_ref()); - if let Some(obj) = obj { + if let Some(obj) = codegen_results.metadata_module.as_ref().and_then(|m| m.object.as_ref()) + { cmd.add_object(obj); } } @@ -1337,25 +1332,24 @@ fn link_local_crate_native_libs_and_dependent_crate_libs<'a, B: ArchiveBuilder<' } /// Add sysroot and other globally set directories to the directory search list. -fn add_library_search_dirs(cmd: &mut dyn Linker, sess: &'a Session) { - // The default library location, we need this to find the runtime. - // The location of crates will be determined as needed. - let lib_path = sess.target_filesearch(PathKind::All).get_lib_path(); - - // prefer system mingw-w64 libs, see get_crt_libs_path comment for more details +fn add_library_search_dirs(cmd: &mut dyn Linker, sess: &Session) { + // Prefer system mingw-w64 libs, see get_crt_libs_path comment for more details. if cfg!(windows) && sess.target.target.llvm_target.contains("windows-gnu") { if let Some(compiler_libs_path) = get_crt_libs_path(sess) { cmd.include_path(&compiler_libs_path); } } + // The default library location, we need this to find the runtime. + // The location of crates will be determined as needed. + let lib_path = sess.target_filesearch(PathKind::All).get_lib_path(); cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); } /// Add options requesting executables to be position-independent or not position-independent. fn add_position_independent_executable_args( cmd: &mut dyn Linker, - sess: &'a Session, + sess: &Session, flavor: LinkerFlavor, crate_type: config::CrateType, codegen_results: &CodegenResults, @@ -1364,7 +1358,6 @@ fn add_position_independent_executable_args( return; } - let mut position_independent_executable = false; if sess.target.target.options.position_independent_executables { let attr_link_args = &*codegen_results.crate_info.link_args; let mut user_defined_link_args = sess.opts.cg.link_args.iter().chain(attr_link_args); @@ -1372,39 +1365,26 @@ fn add_position_independent_executable_args( && !sess.crt_static(Some(crate_type)) && !user_defined_link_args.any(|x| x == "-static") { - position_independent_executable = true; + cmd.position_independent_executable(); + return; } } - if position_independent_executable { - cmd.position_independent_executable(); - } else { - // recent versions of gcc can be configured to generate position - // independent executables by default. We have to pass -no-pie to - // explicitly turn that off. Not applicable to ld. - if sess.target.target.options.linker_is_gnu && flavor != LinkerFlavor::Ld { - cmd.no_position_independent_executable(); - } + // Recent versions of gcc can be configured to generate position + // independent executables by default. We have to pass -no-pie to + // explicitly turn that off. Not applicable to ld. + if sess.target.target.options.linker_is_gnu && flavor != LinkerFlavor::Ld { + cmd.no_position_independent_executable(); } } /// Add options making relocation sections in the produced ELF files read-only /// and suppressing lazy binding. -fn add_relro_args(cmd: &mut dyn Linker, sess: &'a Session) { - let relro_level = match sess.opts.debugging_opts.relro_level { - Some(level) => level, - None => sess.target.target.options.relro_level, - }; - match relro_level { - RelroLevel::Full => { - cmd.full_relro(); - } - RelroLevel::Partial => { - cmd.partial_relro(); - } - RelroLevel::Off => { - cmd.no_relro(); - } +fn add_relro_args(cmd: &mut dyn Linker, sess: &Session) { + match sess.opts.debugging_opts.relro_level.unwrap_or(sess.target.target.options.relro_level) { + RelroLevel::Full => cmd.full_relro(), + RelroLevel::Partial => cmd.partial_relro(), + RelroLevel::Off => cmd.no_relro(), RelroLevel::None => {} } } @@ -1412,7 +1392,7 @@ fn add_relro_args(cmd: &mut dyn Linker, sess: &'a Session) { /// Add library search paths used at runtime by dynamic linkers. fn add_rpath_args( cmd: &mut dyn Linker, - sess: &'a Session, + sess: &Session, codegen_results: &CodegenResults, out_filename: &Path, ) { @@ -1623,7 +1603,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( // Also note that the native libraries linked here are only the ones located // in the current crate. Upstream crates with native library dependencies // may have their native library pulled in above. -pub fn add_local_native_libraries( +fn add_local_native_libraries( cmd: &mut dyn Linker, sess: &Session, codegen_results: &CodegenResults, @@ -1953,7 +1933,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>( // generic function calls a native function, then the generic function must // be instantiated in the target crate, meaning that the native symbol must // also be resolved in the target crate. -pub fn add_upstream_native_libraries( +fn add_upstream_native_libraries( cmd: &mut dyn Linker, sess: &Session, codegen_results: &CodegenResults, @@ -2010,14 +1990,14 @@ pub fn add_upstream_native_libraries( } } -pub fn relevant_lib(sess: &Session, lib: &NativeLibrary) -> bool { +fn relevant_lib(sess: &Session, lib: &NativeLibrary) -> bool { match lib.cfg { Some(ref cfg) => rustc_attr::cfg_matches(cfg, &sess.parse_sess, None), None => true, } } -pub fn are_upstream_rust_objects_already_included(sess: &Session) -> bool { +fn are_upstream_rust_objects_already_included(sess: &Session) -> bool { match sess.lto() { config::Lto::Fat => true, config::Lto::Thin => { From b8f416d67ff77d6eb71902895b59abbfb50737db Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 7 Apr 2020 19:30:16 +0200 Subject: [PATCH 17/25] Further improve comments --- src/librustc_mir_build/lints.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir_build/lints.rs b/src/librustc_mir_build/lints.rs index 74ab602541596..a7370c36f0b9f 100644 --- a/src/librustc_mir_build/lints.rs +++ b/src/librustc_mir_build/lints.rs @@ -29,8 +29,9 @@ fn check_fn_for_unconditional_recursion<'tcx>( ) { let self_calls = find_blocks_calling_self(tcx, &body, def_id); - // Stores a list of `Span`s for every basic block. Those are the spans of `Call` terminators - // where we know that one of them will definitely be reached. + // Stores a list of `Span`s for every basic block. Those are the spans of self-calls where we + // know that one of them will definitely be reached. If the list is empty, the block either + // wasn't processed yet or will not always go to a self-call. let mut results = IndexVec::from_elem_n(vec![], body.basic_blocks().len()); // We start the analysis at the self calls and work backwards. @@ -51,7 +52,7 @@ fn check_fn_for_unconditional_recursion<'tcx>( // If *all* successors of `bb` lead to a self-call, emit notes at all of their // locations. - // Converging successors without unwind paths. + // Determine all "relevant" successors. We ignore successors only reached via unwinding. let terminator = body[bb].terminator(); let relevant_successors = match &terminator.kind { TerminatorKind::Call { destination: None, .. } From 859b8da21fc82c08fde8b3aafefc12881a0c3f09 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 7 Apr 2020 10:55:00 -0700 Subject: [PATCH 18/25] Implement Chain with Option fuses The iterators are now "fused" with `Option` so we don't need separate state to track which part is already exhausted, and we may also get niche layout for `None`. We don't use the real `Fuse` adapter because its specialization for `FusedIterator` unconditionally descends into the iterator, and that could be expensive to keep revisiting stuff like nested chains. It also hurts compiler performance to add more iterator layers to `Chain`. --- src/libcore/iter/adapters/chain.rs | 255 ++++++++++++----------------- 1 file changed, 106 insertions(+), 149 deletions(-) diff --git a/src/libcore/iter/adapters/chain.rs b/src/libcore/iter/adapters/chain.rs index 3611a1aadaddb..6c97c43df407a 100644 --- a/src/libcore/iter/adapters/chain.rs +++ b/src/libcore/iter/adapters/chain.rs @@ -1,8 +1,7 @@ +use crate::iter::{DoubleEndedIterator, FusedIterator, Iterator, TrustedLen}; use crate::ops::Try; use crate::usize; -use super::super::{DoubleEndedIterator, FusedIterator, Iterator, TrustedLen}; - /// An iterator that links two iterators together, in a chain. /// /// This `struct` is created by the [`chain`] method on [`Iterator`]. See its @@ -14,37 +13,34 @@ use super::super::{DoubleEndedIterator, FusedIterator, Iterator, TrustedLen}; #[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct Chain { - a: A, - b: B, - state: ChainState, + // These are "fused" with `Option` so we don't need separate state to track which part is + // already exhausted, and we may also get niche layout for `None`. We don't use the real `Fuse` + // adapter because its specialization for `FusedIterator` unconditionally descends into the + // iterator, and that could be expensive to keep revisiting stuff like nested chains. It also + // hurts compiler performance to add more iterator layers to `Chain`. + a: Option, + b: Option, } impl Chain { pub(in super::super) fn new(a: A, b: B) -> Chain { - Chain { a, b, state: ChainState::Both } + Chain { a: Some(a), b: Some(b) } } } -// The iterator protocol specifies that iteration ends with the return value -// `None` from `.next()` (or `.next_back()`) and it is unspecified what -// further calls return. The chain adaptor must account for this since it uses -// two subiterators. -// -// It uses three states: -// -// - Both: `a` and `b` are remaining -// - Front: `a` remaining -// - Back: `b` remaining -// -// The fourth state (neither iterator is remaining) only occurs after Chain has -// returned None once, so we don't need to store this state. -#[derive(Clone, Debug)] -enum ChainState { - // both front and back iterator are remaining - Both, - // only front is remaining - Front, - // only back is remaining - Back, +/// Fuse the iterator if the expression is `None`. +macro_rules! fuse { + ($self:ident . $iter:ident . $($call:tt)+) => { + match $self.$iter { + Some(ref mut iter) => match iter.$($call)+ { + None => { + $self.$iter = None; + None + } + item => item, + }, + None => None, + } + }; } #[stable(feature = "rust1", since = "1.0.0")] @@ -57,88 +53,65 @@ where #[inline] fn next(&mut self) -> Option { - match self.state { - ChainState::Both => match self.a.next() { - elt @ Some(..) => elt, - None => { - self.state = ChainState::Back; - self.b.next() - } - }, - ChainState::Front => self.a.next(), - ChainState::Back => self.b.next(), + match fuse!(self.a.next()) { + None => fuse!(self.b.next()), + item => item, } } #[inline] #[rustc_inherit_overflow_checks] fn count(self) -> usize { - match self.state { - ChainState::Both => self.a.count() + self.b.count(), - ChainState::Front => self.a.count(), - ChainState::Back => self.b.count(), + match self { + Chain { a: Some(a), b: Some(b) } => a.count() + b.count(), + Chain { a: Some(a), b: None } => a.count(), + Chain { a: None, b: Some(b) } => b.count(), + Chain { a: None, b: None } => 0, } } - fn try_fold(&mut self, init: Acc, mut f: F) -> R + fn try_fold(&mut self, mut acc: Acc, mut f: F) -> R where Self: Sized, F: FnMut(Acc, Self::Item) -> R, R: Try, { - let mut accum = init; - match self.state { - ChainState::Both | ChainState::Front => { - accum = self.a.try_fold(accum, &mut f)?; - if let ChainState::Both = self.state { - self.state = ChainState::Back; - } - } - _ => {} + if let Some(ref mut a) = self.a { + acc = a.try_fold(acc, &mut f)?; + self.a = None; } - if let ChainState::Back = self.state { - accum = self.b.try_fold(accum, &mut f)?; + if let Some(ref mut b) = self.b { + acc = b.try_fold(acc, &mut f)?; + self.b = None; } - Try::from_ok(accum) + Try::from_ok(acc) } - fn fold(self, init: Acc, mut f: F) -> Acc + fn fold(self, mut acc: Acc, mut f: F) -> Acc where F: FnMut(Acc, Self::Item) -> Acc, { - let mut accum = init; - match self.state { - ChainState::Both | ChainState::Front => { - accum = self.a.fold(accum, &mut f); - } - _ => {} + if let Some(a) = self.a { + acc = a.fold(acc, &mut f); } - match self.state { - ChainState::Both | ChainState::Back => { - accum = self.b.fold(accum, &mut f); - } - _ => {} + if let Some(b) = self.b { + acc = b.fold(acc, &mut f); } - accum + acc } #[inline] fn nth(&mut self, mut n: usize) -> Option { - match self.state { - ChainState::Both | ChainState::Front => { - for x in self.a.by_ref() { - if n == 0 { - return Some(x); - } - n -= 1; - } - if let ChainState::Both = self.state { - self.state = ChainState::Back; + if let Some(ref mut a) = self.a { + while let Some(x) = a.next() { + if n == 0 { + return Some(x); } + n -= 1; } - ChainState::Back => {} + self.a = None; } - if let ChainState::Back = self.state { self.b.nth(n) } else { None } + fuse!(self.b.nth(n)) } #[inline] @@ -146,39 +119,33 @@ where where P: FnMut(&Self::Item) -> bool, { - match self.state { - ChainState::Both => match self.a.find(&mut predicate) { - None => { - self.state = ChainState::Back; - self.b.find(predicate) - } - v => v, - }, - ChainState::Front => self.a.find(predicate), - ChainState::Back => self.b.find(predicate), + match fuse!(self.a.find(&mut predicate)) { + None => fuse!(self.b.find(predicate)), + item => item, } } #[inline] fn last(self) -> Option { - match self.state { - ChainState::Both => { + match self { + Chain { a: Some(a), b: Some(b) } => { // Must exhaust a before b. - let a_last = self.a.last(); - let b_last = self.b.last(); + let a_last = a.last(); + let b_last = b.last(); b_last.or(a_last) } - ChainState::Front => self.a.last(), - ChainState::Back => self.b.last(), + Chain { a: Some(a), b: None } => a.last(), + Chain { a: None, b: Some(b) } => b.last(), + Chain { a: None, b: None } => None, } } #[inline] fn size_hint(&self) -> (usize, Option) { - match self.state { - ChainState::Both => { - let (a_lower, a_upper) = self.a.size_hint(); - let (b_lower, b_upper) = self.b.size_hint(); + match self { + Chain { a: Some(a), b: Some(b) } => { + let (a_lower, a_upper) = a.size_hint(); + let (b_lower, b_upper) = b.size_hint(); let lower = a_lower.saturating_add(b_lower); @@ -189,8 +156,9 @@ where (lower, upper) } - ChainState::Front => self.a.size_hint(), - ChainState::Back => self.b.size_hint(), + Chain { a: Some(a), b: None } => a.size_hint(), + Chain { a: None, b: Some(b) } => b.size_hint(), + Chain { a: None, b: None } => (0, Some(0)), } } } @@ -203,82 +171,71 @@ where { #[inline] fn next_back(&mut self) -> Option { - match self.state { - ChainState::Both => match self.b.next_back() { - elt @ Some(..) => elt, - None => { - self.state = ChainState::Front; - self.a.next_back() - } - }, - ChainState::Front => self.a.next_back(), - ChainState::Back => self.b.next_back(), + match fuse!(self.b.next_back()) { + None => fuse!(self.a.next_back()), + item => item, } } #[inline] fn nth_back(&mut self, mut n: usize) -> Option { - match self.state { - ChainState::Both | ChainState::Back => { - for x in self.b.by_ref().rev() { - if n == 0 { - return Some(x); - } - n -= 1; - } - if let ChainState::Both = self.state { - self.state = ChainState::Front; + if let Some(ref mut b) = self.b { + while let Some(x) = b.next_back() { + if n == 0 { + return Some(x); } + n -= 1; } - ChainState::Front => {} + self.b = None; + } + fuse!(self.a.nth_back(n)) + } + + #[inline] + fn rfind

(&mut self, mut predicate: P) -> Option + where + P: FnMut(&Self::Item) -> bool, + { + match fuse!(self.b.rfind(&mut predicate)) { + None => fuse!(self.a.rfind(predicate)), + item => item, } - if let ChainState::Front = self.state { self.a.nth_back(n) } else { None } } - fn try_rfold(&mut self, init: Acc, mut f: F) -> R + fn try_rfold(&mut self, mut acc: Acc, mut f: F) -> R where Self: Sized, F: FnMut(Acc, Self::Item) -> R, R: Try, { - let mut accum = init; - match self.state { - ChainState::Both | ChainState::Back => { - accum = self.b.try_rfold(accum, &mut f)?; - if let ChainState::Both = self.state { - self.state = ChainState::Front; - } - } - _ => {} + if let Some(ref mut b) = self.b { + acc = b.try_rfold(acc, &mut f)?; + self.b = None; } - if let ChainState::Front = self.state { - accum = self.a.try_rfold(accum, &mut f)?; + if let Some(ref mut a) = self.a { + acc = a.try_rfold(acc, f)?; + self.a = None; } - Try::from_ok(accum) + Try::from_ok(acc) } - fn rfold(self, init: Acc, mut f: F) -> Acc + fn rfold(self, mut acc: Acc, mut f: F) -> Acc where F: FnMut(Acc, Self::Item) -> Acc, { - let mut accum = init; - match self.state { - ChainState::Both | ChainState::Back => { - accum = self.b.rfold(accum, &mut f); - } - _ => {} + if let Some(b) = self.b { + acc = b.rfold(acc, &mut f); } - match self.state { - ChainState::Both | ChainState::Front => { - accum = self.a.rfold(accum, &mut f); - } - _ => {} + if let Some(a) = self.a { + acc = a.rfold(acc, f); } - accum + acc } } // Note: *both* must be fused to handle double-ended iterators. +// Now that we "fuse" both sides, we *could* implement this unconditionally, +// but we should be cautious about committing to that in the public API. #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Chain where From 8aac1077ed495ef8d1241ce76d4b64d7eb13a856 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 7 Apr 2020 16:50:16 -0700 Subject: [PATCH 19/25] Reduce callsites in Chain::count() --- src/libcore/iter/adapters/chain.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libcore/iter/adapters/chain.rs b/src/libcore/iter/adapters/chain.rs index 6c97c43df407a..0100e62fae655 100644 --- a/src/libcore/iter/adapters/chain.rs +++ b/src/libcore/iter/adapters/chain.rs @@ -62,12 +62,15 @@ where #[inline] #[rustc_inherit_overflow_checks] fn count(self) -> usize { - match self { - Chain { a: Some(a), b: Some(b) } => a.count() + b.count(), - Chain { a: Some(a), b: None } => a.count(), - Chain { a: None, b: Some(b) } => b.count(), - Chain { a: None, b: None } => 0, - } + let a_count = match self.a { + Some(a) => a.count(), + None => 0, + }; + let b_count = match self.b { + Some(b) => b.count(), + None => 0, + }; + a_count + b_count } fn try_fold(&mut self, mut acc: Acc, mut f: F) -> R From 2c4cffde3bdc36d9634ebf24e512f19aa6fe1ccb Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 7 Apr 2020 16:50:26 -0700 Subject: [PATCH 20/25] Reduce callsites in Chain::last() --- src/libcore/iter/adapters/chain.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/libcore/iter/adapters/chain.rs b/src/libcore/iter/adapters/chain.rs index 0100e62fae655..aadecdfbb6589 100644 --- a/src/libcore/iter/adapters/chain.rs +++ b/src/libcore/iter/adapters/chain.rs @@ -130,17 +130,16 @@ where #[inline] fn last(self) -> Option { - match self { - Chain { a: Some(a), b: Some(b) } => { - // Must exhaust a before b. - let a_last = a.last(); - let b_last = b.last(); - b_last.or(a_last) - } - Chain { a: Some(a), b: None } => a.last(), - Chain { a: None, b: Some(b) } => b.last(), - Chain { a: None, b: None } => None, - } + // Must exhaust a before b. + let a_last = match self.a { + Some(a) => a.last(), + None => None, + }; + let b_last = match self.b { + Some(b) => b.last(), + None => None, + }; + b_last.or(a_last) } #[inline] From ce8abc63a7e1fcac4b69574e00a70352e583cec8 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 7 Apr 2020 16:58:52 -0700 Subject: [PATCH 21/25] Avoid extra &mut in Chain::fold and try_fold --- src/libcore/iter/adapters/chain.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/iter/adapters/chain.rs b/src/libcore/iter/adapters/chain.rs index aadecdfbb6589..2dd405ced20e1 100644 --- a/src/libcore/iter/adapters/chain.rs +++ b/src/libcore/iter/adapters/chain.rs @@ -84,7 +84,7 @@ where self.a = None; } if let Some(ref mut b) = self.b { - acc = b.try_fold(acc, &mut f)?; + acc = b.try_fold(acc, f)?; self.b = None; } Try::from_ok(acc) @@ -98,7 +98,7 @@ where acc = a.fold(acc, &mut f); } if let Some(b) = self.b { - acc = b.fold(acc, &mut f); + acc = b.fold(acc, f); } acc } From f6c729d4a09552ae04205a79a3cecb670ed587a0 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 8 Apr 2020 03:49:53 +0200 Subject: [PATCH 22/25] track_caller: harden naked interactions --- src/librustc_passes/check_attr.rs | 2 +- .../rfc-2091-track-caller/error-with-naked.rs | 17 +++++++++++++++-- .../error-with-naked.stderr | 14 +++++++++++++- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/librustc_passes/check_attr.rs b/src/librustc_passes/check_attr.rs index 619a8c6f229b3..376ff1108d638 100644 --- a/src/librustc_passes/check_attr.rs +++ b/src/librustc_passes/check_attr.rs @@ -141,7 +141,7 @@ impl CheckAttrVisitor<'tcx> { target: Target, ) -> bool { match target { - Target::Fn if attr::contains_name(attrs, sym::naked) => { + _ if attr::contains_name(attrs, sym::naked) => { struct_span_err!( self.tcx.sess, *attr_span, diff --git a/src/test/ui/rfc-2091-track-caller/error-with-naked.rs b/src/test/ui/rfc-2091-track-caller/error-with-naked.rs index dd9e5d0413585..f457384833335 100644 --- a/src/test/ui/rfc-2091-track-caller/error-with-naked.rs +++ b/src/test/ui/rfc-2091-track-caller/error-with-naked.rs @@ -1,8 +1,21 @@ #![feature(naked_functions, track_caller)] -#[track_caller] +#[track_caller] //~ ERROR cannot use `#[track_caller]` with `#[naked]` #[naked] fn f() {} -//~^^^ ERROR cannot use `#[track_caller]` with `#[naked]` + +struct S; + +impl S { + #[track_caller] //~ ERROR cannot use `#[track_caller]` with `#[naked]` + #[naked] + fn g() {} +} + +extern "Rust" { + #[track_caller] //~ ERROR cannot use `#[track_caller]` with `#[naked]` + #[naked] + fn h(); +} fn main() {} diff --git a/src/test/ui/rfc-2091-track-caller/error-with-naked.stderr b/src/test/ui/rfc-2091-track-caller/error-with-naked.stderr index 2f5003cfdb7a5..1249d1df07179 100644 --- a/src/test/ui/rfc-2091-track-caller/error-with-naked.stderr +++ b/src/test/ui/rfc-2091-track-caller/error-with-naked.stderr @@ -4,6 +4,18 @@ error[E0736]: cannot use `#[track_caller]` with `#[naked]` LL | #[track_caller] | ^^^^^^^^^^^^^^^ -error: aborting due to previous error +error[E0736]: cannot use `#[track_caller]` with `#[naked]` + --> $DIR/error-with-naked.rs:16:5 + | +LL | #[track_caller] + | ^^^^^^^^^^^^^^^ + +error[E0736]: cannot use `#[track_caller]` with `#[naked]` + --> $DIR/error-with-naked.rs:10:5 + | +LL | #[track_caller] + | ^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0736`. From f03db79eaa8ce5c5b53100baa63816c41631c625 Mon Sep 17 00:00:00 2001 From: Tobias Thiel Date: Tue, 7 Apr 2020 21:41:54 -0700 Subject: [PATCH 23/25] rustc_session: forbid lints override regardless of position --- src/doc/rustc/src/lints/levels.md | 2 +- src/librustc_session/config.rs | 8 +++++++- .../ui-fulldeps/lint-group-forbid-always-trumps-cli.rs | 7 +++++++ .../lint-group-forbid-always-trumps-cli.stderr | 10 ++++++++++ 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 src/test/ui-fulldeps/lint-group-forbid-always-trumps-cli.rs create mode 100644 src/test/ui-fulldeps/lint-group-forbid-always-trumps-cli.stderr diff --git a/src/doc/rustc/src/lints/levels.md b/src/doc/rustc/src/lints/levels.md index 3cfe2f698f3e0..64cbbbb003585 100644 --- a/src/doc/rustc/src/lints/levels.md +++ b/src/doc/rustc/src/lints/levels.md @@ -170,7 +170,7 @@ The order of these command line arguments is taken into account. The following a $ rustc lib.rs --crate-type=lib -D unused-variables -A unused-variables ``` -You can make use of this behavior by overriding the level of one specific lint out of a group of lints. The following example denies all the lints in the `unused` group, but explicitly allows the `unused-variables` lint in that group: +You can make use of this behavior by overriding the level of one specific lint out of a group of lints. The following example denies all the lints in the `unused` group, but explicitly allows the `unused-variables` lint in that group (forbid still trumps everything regardless of ordering): ```bash $ rustc lib.rs --crate-type=lib -D unused -A unused-variables diff --git a/src/librustc_session/config.rs b/src/librustc_session/config.rs index 58a03dbe388e6..4e2423bc3b114 100644 --- a/src/librustc_session/config.rs +++ b/src/librustc_session/config.rs @@ -1017,7 +1017,13 @@ pub fn get_cmd_lint_options( let mut describe_lints = false; for &level in &[lint::Allow, lint::Warn, lint::Deny, lint::Forbid] { - for (arg_pos, lint_name) in matches.opt_strs_pos(level.as_str()) { + for (passed_arg_pos, lint_name) in matches.opt_strs_pos(level.as_str()) { + let arg_pos = if let lint::Forbid = level { + // forbid is always specified last, so it can't be overridden + usize::max_value() + } else { + passed_arg_pos + }; if lint_name == "help" { describe_lints = true; } else { diff --git a/src/test/ui-fulldeps/lint-group-forbid-always-trumps-cli.rs b/src/test/ui-fulldeps/lint-group-forbid-always-trumps-cli.rs new file mode 100644 index 0000000000000..fc19bc039063a --- /dev/null +++ b/src/test/ui-fulldeps/lint-group-forbid-always-trumps-cli.rs @@ -0,0 +1,7 @@ +// aux-build:lint-group-plugin-test.rs +// compile-flags: -F unused -A unused + +fn main() { + let x = 1; + //~^ ERROR unused variable: `x` +} diff --git a/src/test/ui-fulldeps/lint-group-forbid-always-trumps-cli.stderr b/src/test/ui-fulldeps/lint-group-forbid-always-trumps-cli.stderr new file mode 100644 index 0000000000000..6bab367b0d175 --- /dev/null +++ b/src/test/ui-fulldeps/lint-group-forbid-always-trumps-cli.stderr @@ -0,0 +1,10 @@ +error: unused variable: `x` + --> $DIR/lint-group-forbid-always-trumps-cli.rs:5:9 + | +LL | let x = 1; + | ^ help: if this is intentional, prefix it with an underscore: `_x` + | + = note: `-F unused-variables` implied by `-F unused` + +error: aborting due to previous error + From 563152d8830737b65d39d280214b1a64aa006f98 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 8 Apr 2020 14:49:57 +0000 Subject: [PATCH 24/25] comment pessimistic yield and saving/restoring state --- src/librustc_passes/region.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/librustc_passes/region.rs b/src/librustc_passes/region.rs index 1807756fe524b..32e7e8843a07c 100644 --- a/src/librustc_passes/region.rs +++ b/src/librustc_passes/region.rs @@ -717,9 +717,16 @@ impl<'tcx> Visitor<'tcx> for RegionResolutionVisitor<'tcx> { self.cx.parent ); + // Save all state that is specific to the outer function + // body. These will be restored once down below, once we've + // visited the body. let outer_ec = mem::replace(&mut self.expr_and_pat_count, 0); let outer_cx = self.cx; let outer_ts = mem::take(&mut self.terminating_scopes); + // The 'pessimistic yield' flag is set to true when we are + // processing a `+=` statement and have to make pessimistic + // control flow assumptions. This doesn't apply to nested + // bodies within the `+=` statements. See #69307. let outer_pessimistic_yield = mem::replace(&mut self.pessimistic_yield, false); self.terminating_scopes.insert(body.value.hir_id.local_id); From 45589b52fe215b7090bf88683927716c25197203 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 8 Apr 2020 04:35:51 +0200 Subject: [PATCH 25/25] track_caller: support on FFI imports --- src/librustc_error_codes/error_codes.rs | 2 +- src/librustc_error_codes/error_codes/E0738.md | 11 ---- src/librustc_passes/check_attr.rs | 12 +---- .../rfc-2091-track-caller/error-extern-fn.rs | 9 ---- .../error-extern-fn.stderr | 9 ---- .../rfc-2091-track-caller/track-caller-ffi.rs | 50 +++++++++++++++++++ 6 files changed, 52 insertions(+), 41 deletions(-) delete mode 100644 src/librustc_error_codes/error_codes/E0738.md delete mode 100644 src/test/ui/rfc-2091-track-caller/error-extern-fn.rs delete mode 100644 src/test/ui/rfc-2091-track-caller/error-extern-fn.stderr create mode 100644 src/test/ui/rfc-2091-track-caller/track-caller-ffi.rs diff --git a/src/librustc_error_codes/error_codes.rs b/src/librustc_error_codes/error_codes.rs index 6e690655f60c5..8d9982131c33e 100644 --- a/src/librustc_error_codes/error_codes.rs +++ b/src/librustc_error_codes/error_codes.rs @@ -416,7 +416,6 @@ E0734: include_str!("./error_codes/E0734.md"), E0735: include_str!("./error_codes/E0735.md"), E0736: include_str!("./error_codes/E0736.md"), E0737: include_str!("./error_codes/E0737.md"), -E0738: include_str!("./error_codes/E0738.md"), E0739: include_str!("./error_codes/E0739.md"), E0740: include_str!("./error_codes/E0740.md"), E0741: include_str!("./error_codes/E0741.md"), @@ -614,4 +613,5 @@ E0751: include_str!("./error_codes/E0751.md"), E0722, // Malformed `#[optimize]` attribute E0724, // `#[ffi_returns_twice]` is only allowed in foreign functions E0726, // non-explicit (not `'_`) elided lifetime in unsupported position +// E0738, // Removed; errored on `#[track_caller] fn`s in `extern "Rust" { ... }`. } diff --git a/src/librustc_error_codes/error_codes/E0738.md b/src/librustc_error_codes/error_codes/E0738.md deleted file mode 100644 index 8f31b701e495e..0000000000000 --- a/src/librustc_error_codes/error_codes/E0738.md +++ /dev/null @@ -1,11 +0,0 @@ -`#[track_caller]` cannot be used to annotate foreign functions. - -Erroneous example: - -```compile_fail,E0738 -#![feature(track_caller)] -extern "Rust" { - #[track_caller] - fn bar(); -} -``` diff --git a/src/librustc_passes/check_attr.rs b/src/librustc_passes/check_attr.rs index 376ff1108d638..3f2c02f6c461b 100644 --- a/src/librustc_passes/check_attr.rs +++ b/src/librustc_passes/check_attr.rs @@ -151,17 +151,7 @@ impl CheckAttrVisitor<'tcx> { .emit(); false } - Target::ForeignFn => { - struct_span_err!( - self.tcx.sess, - *attr_span, - E0738, - "`#[track_caller]` is not supported on foreign functions", - ) - .emit(); - false - } - Target::Fn | Target::Method(..) => true, + Target::Fn | Target::Method(..) | Target::ForeignFn => true, _ => { struct_span_err!( self.tcx.sess, diff --git a/src/test/ui/rfc-2091-track-caller/error-extern-fn.rs b/src/test/ui/rfc-2091-track-caller/error-extern-fn.rs deleted file mode 100644 index 9f6a69a51c0ce..0000000000000 --- a/src/test/ui/rfc-2091-track-caller/error-extern-fn.rs +++ /dev/null @@ -1,9 +0,0 @@ -#![feature(track_caller)] -#![allow(dead_code)] - -extern "Rust" { - #[track_caller] //~ ERROR: `#[track_caller]` is not supported on foreign functions - fn bar(); -} - -fn main() {} diff --git a/src/test/ui/rfc-2091-track-caller/error-extern-fn.stderr b/src/test/ui/rfc-2091-track-caller/error-extern-fn.stderr deleted file mode 100644 index b03f5fbbdb20e..0000000000000 --- a/src/test/ui/rfc-2091-track-caller/error-extern-fn.stderr +++ /dev/null @@ -1,9 +0,0 @@ -error[E0738]: `#[track_caller]` is not supported on foreign functions - --> $DIR/error-extern-fn.rs:5:5 - | -LL | #[track_caller] - | ^^^^^^^^^^^^^^^ - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0738`. diff --git a/src/test/ui/rfc-2091-track-caller/track-caller-ffi.rs b/src/test/ui/rfc-2091-track-caller/track-caller-ffi.rs new file mode 100644 index 0000000000000..23c17d743c43c --- /dev/null +++ b/src/test/ui/rfc-2091-track-caller/track-caller-ffi.rs @@ -0,0 +1,50 @@ +// run-pass + +#![feature(track_caller)] + +use std::panic::Location; + +extern "Rust" { + #[track_caller] + fn rust_track_caller_ffi_test_tracked() -> &'static Location<'static>; + fn rust_track_caller_ffi_test_untracked() -> &'static Location<'static>; +} + +fn rust_track_caller_ffi_test_nested_tracked() -> &'static Location<'static> { + unsafe { rust_track_caller_ffi_test_tracked() } +} + +mod provides { + use std::panic::Location; + #[track_caller] // UB if we did not have this! + #[no_mangle] + fn rust_track_caller_ffi_test_tracked() -> &'static Location<'static> { + Location::caller() + } + #[no_mangle] + fn rust_track_caller_ffi_test_untracked() -> &'static Location<'static> { + Location::caller() + } +} + +fn main() { + let location = Location::caller(); + assert_eq!(location.file(), file!()); + assert_eq!(location.line(), 31); + assert_eq!(location.column(), 20); + + let tracked = unsafe { rust_track_caller_ffi_test_tracked() }; + assert_eq!(tracked.file(), file!()); + assert_eq!(tracked.line(), 36); + assert_eq!(tracked.column(), 28); + + let untracked = unsafe { rust_track_caller_ffi_test_untracked() }; + assert_eq!(untracked.file(), file!()); + assert_eq!(untracked.line(), 26); + assert_eq!(untracked.column(), 9); + + let contained = rust_track_caller_ffi_test_nested_tracked(); + assert_eq!(contained.file(), file!()); + assert_eq!(contained.line(), 14); + assert_eq!(contained.column(), 14); +}