From 63c471e10227944268ddf67973a584d68e314a8d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 27 Sep 2018 15:50:17 -0700 Subject: [PATCH 01/30] rustc: Tweak filenames encoded into metadata This commit is a fix for #54408 where on nightly right now whenever generics are inlined the path name listed for the inlined function's debuginfo is a relative path to the cwd, which surely doesn't exist! Previously on beta/stable the debuginfo mentioned an absolute path which still didn't exist, but more predictably didn't exist. The change between stable/nightly is that nightly is now compiled with `--remap-path-prefix` to give a deterministic prefix to all rustc-generated paths in debuginfo. By using `--remap-path-prefix` the previous logic would recognize that the cwd was remapped, causing the original relative path name of the standard library to get emitted. If `--remap-path-prefix` *wasn't* passed in then the logic would create an absolute path name and then create a new source file entry. The fix in this commit is to apply the "recreate the source file entry with an absolute path" logic a bit more aggresively. If the source file's name was remapped then we don't touch it, but otherwise we always take the working dir (which may have been remapped) and then join it to the file to ensure that we process all relative file names as well. The end result is that the standard library should have an absolute path for all file names in debuginfo (using our `--remap-path-prefix` argument) as it does on stable after this patch. Closes #54408 --- src/librustc_metadata/encoder.rs | 42 ++++++++----------- .../auxiliary/xcrate-generic.rs | 16 +++++++ .../remap_path_prefix/xcrate-generic.rs | 25 +++++++++++ 3 files changed, 59 insertions(+), 24 deletions(-) create mode 100644 src/test/codegen/remap_path_prefix/auxiliary/xcrate-generic.rs create mode 100644 src/test/codegen/remap_path_prefix/xcrate-generic.rs diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index c36ae02ab54e0..292bf1daee22f 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -340,7 +340,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let source_map = self.tcx.sess.source_map(); let all_source_files = source_map.files(); - let (working_dir, working_dir_was_remapped) = self.tcx.sess.working_dir.clone(); + let (working_dir, _cwd_remapped) = self.tcx.sess.working_dir.clone(); let adapted = all_source_files.iter() .filter(|source_file| { @@ -349,32 +349,26 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { !source_file.is_imported() }) .map(|source_file| { - // When exporting SourceFiles, we expand all paths to absolute - // paths because any relative paths are potentially relative to - // a wrong directory. - // However, if a path has been modified via - // `--remap-path-prefix` we assume the user has already set - // things up the way they want and don't touch the path values - // anymore. match source_file.name { + // This path of this SourceFile has been modified by + // path-remapping, so we use it verbatim (and avoid + // cloning the whole map in the process). + _ if source_file.name_was_remapped => source_file.clone(), + + // Otherwise expand all paths to absolute paths because + // any relative paths are potentially relative to a + // wrong directory. FileName::Real(ref name) => { - if source_file.name_was_remapped || - (name.is_relative() && working_dir_was_remapped) { - // This path of this SourceFile has been modified by - // path-remapping, so we use it verbatim (and avoid cloning - // the whole map in the process). - source_file.clone() - } else { - let mut adapted = (**source_file).clone(); - adapted.name = Path::new(&working_dir).join(name).into(); - adapted.name_hash = { - let mut hasher: StableHasher = StableHasher::new(); - adapted.name.hash(&mut hasher); - hasher.finish() - }; - Lrc::new(adapted) - } + let mut adapted = (**source_file).clone(); + adapted.name = Path::new(&working_dir).join(name).into(); + adapted.name_hash = { + let mut hasher: StableHasher = StableHasher::new(); + adapted.name.hash(&mut hasher); + hasher.finish() + }; + Lrc::new(adapted) }, + // expanded code, not from a file _ => source_file.clone(), } diff --git a/src/test/codegen/remap_path_prefix/auxiliary/xcrate-generic.rs b/src/test/codegen/remap_path_prefix/auxiliary/xcrate-generic.rs new file mode 100644 index 0000000000000..6c477a407812a --- /dev/null +++ b/src/test/codegen/remap_path_prefix/auxiliary/xcrate-generic.rs @@ -0,0 +1,16 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// ignore-tidy-linelength +// compile-flags: -g --remap-path-prefix={{cwd}}=/the/aux-cwd --remap-path-prefix={{src-base}}/remap_path_prefix/auxiliary=/the/aux-src + +#![crate_type = "lib"] + +pub fn foo() {} diff --git a/src/test/codegen/remap_path_prefix/xcrate-generic.rs b/src/test/codegen/remap_path_prefix/xcrate-generic.rs new file mode 100644 index 0000000000000..f206df9813165 --- /dev/null +++ b/src/test/codegen/remap_path_prefix/xcrate-generic.rs @@ -0,0 +1,25 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// ignore-windows +// ignore-tidy-linelength +// compile-flags: -g -C metadata=foo -C no-prepopulate-passes +// aux-build:xcrate-generic.rs + +#![crate_type = "lib"] + +extern crate xcrate_generic; + +pub fn foo() { + xcrate_generic::foo::(); +} + +// Here we check that local debuginfo is mapped correctly. +// CHECK: !DIFile(filename: "/the/aux-src/xcrate-generic.rs", directory: "") From f81d1dd29401cd586a13b77db729f42076f07902 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Tue, 11 Sep 2018 21:35:08 -0400 Subject: [PATCH 02/30] Rewrite the `UnconditionalRecursion` lint to use MIR Part of #51002 --- src/librustc/lint/builtin.rs | 7 + src/librustc_lint/builtin.rs | 277 ------------------ src/librustc_lint/lib.rs | 1 - src/librustc_mir/build/mod.rs | 4 + src/librustc_mir/lib.rs | 1 + src/librustc_mir/lints.rs | 156 ++++++++++ .../ui/did_you_mean/issue-31424.nll.stderr | 14 +- src/test/ui/did_you_mean/issue-31424.rs | 1 + src/test/ui/did_you_mean/issue-31424.stderr | 14 +- src/test/ui/nll/issue-51191.rs | 1 + src/test/ui/nll/issue-51191.stderr | 22 +- ...-bound-on-closure-outlives-call.nll.stderr | 14 +- .../region-bound-on-closure-outlives-call.rs | 1 + ...gion-bound-on-closure-outlives-call.stderr | 14 +- 14 files changed, 240 insertions(+), 287 deletions(-) create mode 100644 src/librustc_mir/lints.rs diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 64056ece98770..cc493884f8488 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -233,6 +233,12 @@ declare_lint! { "detect mut variables which don't need to be mutable" } +declare_lint! { + pub UNCONDITIONAL_RECURSION, + Warn, + "functions that cannot return without calling themselves" +} + declare_lint! { pub SINGLE_USE_LIFETIMES, Allow, @@ -396,6 +402,7 @@ impl LintPass for HardwiredLints { DEPRECATED, UNUSED_UNSAFE, UNUSED_MUT, + UNCONDITIONAL_RECURSION, SINGLE_USE_LIFETIMES, UNUSED_LIFETIMES, UNUSED_LABELS, diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 7eda6e94dd071..4ca51e8ce1764 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -30,10 +30,7 @@ use rustc::hir::def::Def; use rustc::hir::def_id::DefId; -use rustc::cfg; -use rustc::ty::subst::Substs; use rustc::ty::{self, Ty}; -use rustc::traits; use hir::Node; use util::nodemap::NodeSet; use lint::{LateContext, LintContext, LintArray}; @@ -844,279 +841,6 @@ impl EarlyLintPass for UnusedDocComment { } } -declare_lint! { - pub UNCONDITIONAL_RECURSION, - Warn, - "functions that cannot return without calling themselves" -} - -#[derive(Copy, Clone)] -pub struct UnconditionalRecursion; - - -impl LintPass for UnconditionalRecursion { - fn get_lints(&self) -> LintArray { - lint_array![UNCONDITIONAL_RECURSION] - } -} - -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnconditionalRecursion { - fn check_fn(&mut self, - cx: &LateContext, - fn_kind: FnKind, - _: &hir::FnDecl, - body: &hir::Body, - sp: Span, - id: ast::NodeId) { - let method = match fn_kind { - FnKind::ItemFn(..) => None, - FnKind::Method(..) => { - Some(cx.tcx.associated_item(cx.tcx.hir.local_def_id(id))) - } - // closures can't recur, so they don't matter. - FnKind::Closure(_) => return, - }; - - // 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 cfg = cfg::CFG::new(cx.tcx, &body); - - let mut work_queue = vec![cfg.entry]; - let mut reached_exit_without_self_call = false; - let mut self_call_spans = vec![]; - let mut visited = FxHashSet::default(); - - while let Some(idx) = work_queue.pop() { - if idx == cfg.exit { - // found a path! - reached_exit_without_self_call = true; - break; - } - - let cfg_id = idx.node_id(); - if visited.contains(&cfg_id) { - // already done - continue; - } - visited.insert(cfg_id); - - // is this a recursive call? - let local_id = cfg.graph.node_data(idx).id(); - if local_id != hir::DUMMY_ITEM_LOCAL_ID { - let node_id = cx.tcx.hir.hir_to_node_id(hir::HirId { - owner: body.value.hir_id.owner, - local_id - }); - let self_recursive = match method { - Some(ref method) => expr_refers_to_this_method(cx, method, node_id), - None => expr_refers_to_this_fn(cx, id, node_id), - }; - if self_recursive { - self_call_spans.push(cx.tcx.hir.span(node_id)); - // this is a self call, so we shouldn't explore past - // this node in the CFG. - continue; - } - } - - // add the successors of this node to explore the graph further. - for (_, edge) in cfg.graph.outgoing_edges(idx) { - let target_idx = edge.target(); - let target_cfg_id = target_idx.node_id(); - if !visited.contains(&target_cfg_id) { - work_queue.push(target_idx) - } - } - } - - // 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_spans.is_empty() { - let sp = cx.tcx.sess.source_map().def_span(sp); - let mut db = cx.struct_span_lint(UNCONDITIONAL_RECURSION, - sp, - "function cannot return without recursing"); - db.span_label(sp, "cannot return without recursing"); - // offer some help to the programmer. - for call in &self_call_spans { - db.span_label(*call, "recursive call site"); - } - db.help("a `loop` may express intention better if this is on purpose"); - db.emit(); - } - - // all done - return; - - // Functions for identifying if the given Expr NodeId `id` - // represents a call to the function `fn_id`/method `method`. - - fn expr_refers_to_this_fn(cx: &LateContext, fn_id: ast::NodeId, id: ast::NodeId) -> bool { - match cx.tcx.hir.get(id) { - Node::Expr(&hir::Expr { node: hir::ExprKind::Call(ref callee, _), .. }) => { - let def = if let hir::ExprKind::Path(ref qpath) = callee.node { - cx.tables.qpath_def(qpath, callee.hir_id) - } else { - return false; - }; - match def { - Def::Local(..) | Def::Upvar(..) => false, - _ => def.def_id() == cx.tcx.hir.local_def_id(fn_id) - } - } - _ => false, - } - } - - // Check if the expression `id` performs a call to `method`. - fn expr_refers_to_this_method(cx: &LateContext, - method: &ty::AssociatedItem, - id: ast::NodeId) - -> bool { - use rustc::ty::adjustment::*; - - // Ignore non-expressions. - let expr = if let Node::Expr(e) = cx.tcx.hir.get(id) { - e - } else { - return false; - }; - - // Check for overloaded autoderef method calls. - let mut source = cx.tables.expr_ty(expr); - for adjustment in cx.tables.expr_adjustments(expr) { - if let Adjust::Deref(Some(deref)) = adjustment.kind { - let (def_id, substs) = deref.method_call(cx.tcx, source); - if method_call_refers_to_method(cx, method, def_id, substs, id) { - return true; - } - } - source = adjustment.target; - } - - // Check for method calls and overloaded operators. - if cx.tables.is_method_call(expr) { - let hir_id = cx.tcx.hir.definitions().node_to_hir_id(id); - if let Some(def) = cx.tables.type_dependent_defs().get(hir_id) { - let def_id = def.def_id(); - let substs = cx.tables.node_substs(hir_id); - if method_call_refers_to_method(cx, method, def_id, substs, id) { - return true; - } - } else { - cx.tcx.sess.delay_span_bug(expr.span, - "no type-dependent def for method call"); - } - } - - // Check for calls to methods via explicit paths (e.g. `T::method()`). - match expr.node { - hir::ExprKind::Call(ref callee, _) => { - let def = if let hir::ExprKind::Path(ref qpath) = callee.node { - cx.tables.qpath_def(qpath, callee.hir_id) - } else { - return false; - }; - match def { - Def::Method(def_id) => { - let substs = cx.tables.node_substs(callee.hir_id); - method_call_refers_to_method(cx, method, def_id, substs, id) - } - _ => false, - } - } - _ => false, - } - } - - // Check if the method call to the method with the ID `callee_id` - // and instantiated with `callee_substs` refers to method `method`. - fn method_call_refers_to_method<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, - method: &ty::AssociatedItem, - callee_id: DefId, - callee_substs: &Substs<'tcx>, - expr_id: ast::NodeId) - -> bool { - let tcx = cx.tcx; - let callee_item = tcx.associated_item(callee_id); - - match callee_item.container { - // This is an inherent method, so the `def_id` refers - // directly to the method definition. - ty::ImplContainer(_) => callee_id == method.def_id, - - // A trait method, from any number of possible sources. - // Attempt to select a concrete impl before checking. - ty::TraitContainer(trait_def_id) => { - let trait_ref = ty::TraitRef::from_method(tcx, trait_def_id, callee_substs); - let trait_ref = ty::Binder::bind(trait_ref); - let span = tcx.hir.span(expr_id); - let obligation = - traits::Obligation::new(traits::ObligationCause::misc(span, expr_id), - cx.param_env, - trait_ref.to_poly_trait_predicate()); - - tcx.infer_ctxt().enter(|infcx| { - let mut selcx = traits::SelectionContext::new(&infcx); - match selcx.select(&obligation) { - // The method comes from a `T: Trait` bound. - // If `T` is `Self`, then this call is inside - // a default method definition. - Ok(Some(traits::VtableParam(_))) => { - let on_self = trait_ref.self_ty().is_self(); - // We can only be recursing in a default - // method if we're being called literally - // on the `Self` type. - on_self && callee_id == method.def_id - } - - // The `impl` is known, so we check that with a - // special case: - Ok(Some(traits::VtableImpl(vtable_impl))) => { - let container = ty::ImplContainer(vtable_impl.impl_def_id); - // It matches if it comes from the same impl, - // and has the same method name. - container == method.container && - callee_item.ident.name == method.ident.name - } - - // There's no way to know if this call is - // recursive, so we assume it's not. - _ => false, - } - }) - } - } - } - } -} - declare_lint! { PLUGIN_AS_LIBRARY, Warn, @@ -1783,7 +1507,6 @@ impl LintPass for SoftLints { MISSING_DEBUG_IMPLEMENTATIONS, ANONYMOUS_PARAMETERS, UNUSED_DOC_COMMENTS, - UNCONDITIONAL_RECURSION, PLUGIN_AS_LIBRARY, PRIVATE_NO_MANGLE_FNS, PRIVATE_NO_MANGLE_STATICS, diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 98d4c87dc3b5e..63ecb052924c4 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -144,7 +144,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { UnusedAllocation: UnusedAllocation, MissingCopyImplementations: MissingCopyImplementations, UnstableFeatures: UnstableFeatures, - UnconditionalRecursion: UnconditionalRecursion, InvalidNoMangleItems: InvalidNoMangleItems, PluginAsLibrary: PluginAsLibrary, MutableTransmutes: MutableTransmutes, diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 3dbd3bbb41573..b899918d2949e 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -35,6 +35,8 @@ use syntax_pos::Span; use transform::MirSource; use util as mir_util; +use super::lints; + /// Construct the MIR for a given def-id. pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Mir<'tcx> { let id = tcx.hir.as_local_node_id(def_id).unwrap(); @@ -176,6 +178,8 @@ pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Mir<'t mir_util::dump_mir(tcx, None, "mir_map", &0, MirSource::item(def_id), &mir, |_, _| Ok(()) ); + lints::check(tcx, &mir, def_id); + mir }) } diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index b3ef9eab8017d..257e4d1db80c3 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -79,6 +79,7 @@ mod borrow_check; mod build; mod dataflow; mod hair; +mod lints; mod shim; pub mod transform; pub mod util; diff --git a/src/librustc_mir/lints.rs b/src/librustc_mir/lints.rs new file mode 100644 index 0000000000000..4c7938504c10f --- /dev/null +++ b/src/librustc_mir/lints.rs @@ -0,0 +1,156 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use rustc_data_structures::bit_set::BitSet; +use rustc::hir::def_id::DefId; +use rustc::hir::intravisit::FnKind; +use rustc::hir::map::blocks::FnLikeNode; +use rustc::lint::builtin::UNCONDITIONAL_RECURSION; +use rustc::mir::{self, Mir, TerminatorKind}; +use rustc::ty::{AssociatedItem, AssociatedItemContainer, Instance, TyCtxt, TyKind}; +use rustc::ty::subst::Substs; + +pub fn check(tcx: TyCtxt<'a, 'tcx, 'tcx>, + mir: &Mir<'tcx>, + def_id: DefId) { + let node_id = tcx.hir.as_local_node_id(def_id).unwrap(); + + if let Some(fn_like_node) = FnLikeNode::from_node(tcx.hir.get(node_id)) { + check_fn_for_unconditional_recursion(tcx, fn_like_node.kind(), mir, def_id); + } +} + +fn check_fn_for_unconditional_recursion(tcx: TyCtxt<'a, 'tcx, 'tcx>, + fn_kind: FnKind, + mir: &Mir<'tcx>, + def_id: DefId) { + if let FnKind::Closure(_) = fn_kind { + // closures can't recur, so they don't matter. + 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 = mir.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 param_env = tcx.param_env(def_id); + let trait_substs_count = + match tcx.opt_associated_item(def_id) { + Some(AssociatedItem { + container: AssociatedItemContainer::TraitContainer(trait_def_id), + .. + }) => tcx.generics_of(trait_def_id).count(), + _ => 0 + }; + let caller_substs = &Substs::identity_for_item(tcx, def_id)[..trait_substs_count]; + + while let Some(bb) = reachable_without_self_call_queue.pop() { + if visited.contains(bb) { + //already done + continue; + } + + visited.insert(bb); + + let block = &basic_blocks[bb]; + + if let Some(ref terminator) = block.terminator { + match terminator.kind { + TerminatorKind::Call { ref func, .. } => { + let func_ty = func.ty(mir, tcx); + + if let TyKind::FnDef(fn_def_id, substs) = func_ty.sty { + 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; + } + } + }, + TerminatorKind::Abort | TerminatorKind::Return => { + //found a path! + reached_exit_without_self_call = true; + break; + } + _ => {} + } + + for successor in terminator.successors() { + reachable_without_self_call_queue.push(*successor); + } + } + } + + // 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() { + let node_id = tcx.hir.as_local_node_id(def_id).unwrap(); + let sp = tcx.sess.source_map().def_span(tcx.hir.span(node_id)); + let mut db = tcx.struct_span_lint_node(UNCONDITIONAL_RECURSION, + node_id, + sp, + "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"); + } + db.help("a `loop` may express intention better if this is on purpose"); + db.emit(); + } +} diff --git a/src/test/ui/did_you_mean/issue-31424.nll.stderr b/src/test/ui/did_you_mean/issue-31424.nll.stderr index 15139e4e8ae36..fca29c9a9f644 100644 --- a/src/test/ui/did_you_mean/issue-31424.nll.stderr +++ b/src/test/ui/did_you_mean/issue-31424.nll.stderr @@ -7,8 +7,20 @@ LL | (&mut self).bar(); //~ ERROR cannot borrow | cannot borrow as mutable | try removing `&mut` here +warning: function cannot return without recursing + --> $DIR/issue-31424.rs:22:5 + | +LL | fn bar(self: &mut Self) { + | ^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing +LL | //~^ WARN function cannot return without recursing +LL | (&mut self).bar(); //~ ERROR cannot borrow + | ----------------- recursive call site + | + = note: #[warn(unconditional_recursion)] on by default + = help: a `loop` may express intention better if this is on purpose + error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable - --> $DIR/issue-31424.rs:23:9 + --> $DIR/issue-31424.rs:24:9 | LL | (&mut self).bar(); //~ ERROR cannot borrow | ^^^^^^^^^^^ diff --git a/src/test/ui/did_you_mean/issue-31424.rs b/src/test/ui/did_you_mean/issue-31424.rs index 1b31e064337e2..903a76a824338 100644 --- a/src/test/ui/did_you_mean/issue-31424.rs +++ b/src/test/ui/did_you_mean/issue-31424.rs @@ -20,6 +20,7 @@ impl Struct { // In this case we could keep the suggestion, but to distinguish the // two cases is pretty hard. It's an obscure case anyway. fn bar(self: &mut Self) { + //~^ WARN function cannot return without recursing (&mut self).bar(); //~ ERROR cannot borrow } } diff --git a/src/test/ui/did_you_mean/issue-31424.stderr b/src/test/ui/did_you_mean/issue-31424.stderr index 9d0ab21ffaf0e..2e4bcc7f95947 100644 --- a/src/test/ui/did_you_mean/issue-31424.stderr +++ b/src/test/ui/did_you_mean/issue-31424.stderr @@ -7,8 +7,20 @@ LL | (&mut self).bar(); //~ ERROR cannot borrow | cannot reborrow mutably | try removing `&mut` here +warning: function cannot return without recursing + --> $DIR/issue-31424.rs:22:5 + | +LL | fn bar(self: &mut Self) { + | ^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing +LL | //~^ WARN function cannot return without recursing +LL | (&mut self).bar(); //~ ERROR cannot borrow + | ----------------- recursive call site + | + = note: #[warn(unconditional_recursion)] on by default + = help: a `loop` may express intention better if this is on purpose + error[E0596]: cannot borrow immutable argument `self` as mutable - --> $DIR/issue-31424.rs:23:15 + --> $DIR/issue-31424.rs:24:15 | LL | (&mut self).bar(); //~ ERROR cannot borrow | ^^^^ cannot borrow mutably diff --git a/src/test/ui/nll/issue-51191.rs b/src/test/ui/nll/issue-51191.rs index 87ec3e5df0b81..0f8372e094d6a 100644 --- a/src/test/ui/nll/issue-51191.rs +++ b/src/test/ui/nll/issue-51191.rs @@ -14,6 +14,7 @@ struct Struct; impl Struct { fn bar(self: &mut Self) { + //~^ WARN function cannot return without recursing (&mut self).bar(); //~^ ERROR cannot borrow `self` as mutable, as it is not declared as mutable [E0596] } diff --git a/src/test/ui/nll/issue-51191.stderr b/src/test/ui/nll/issue-51191.stderr index c5b5218f173ac..88c653effb405 100644 --- a/src/test/ui/nll/issue-51191.stderr +++ b/src/test/ui/nll/issue-51191.stderr @@ -1,5 +1,17 @@ +warning: function cannot return without recursing + --> $DIR/issue-51191.rs:16:5 + | +LL | fn bar(self: &mut Self) { + | ^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing +LL | //~^ WARN function cannot return without recursing +LL | (&mut self).bar(); + | ----------------- recursive call site + | + = note: #[warn(unconditional_recursion)] on by default + = help: a `loop` may express intention better if this is on purpose + error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable - --> $DIR/issue-51191.rs:17:9 + --> $DIR/issue-51191.rs:18:9 | LL | (&mut self).bar(); | ^^^^^^^^^^^ @@ -8,7 +20,7 @@ LL | (&mut self).bar(); | try removing `&mut` here error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable - --> $DIR/issue-51191.rs:22:9 + --> $DIR/issue-51191.rs:23:9 | LL | fn imm(self) { | ---- help: consider changing this to be mutable: `mut self` @@ -16,19 +28,19 @@ LL | (&mut self).bar(); | ^^^^^^^^^^^ cannot borrow as mutable error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable - --> $DIR/issue-51191.rs:31:9 + --> $DIR/issue-51191.rs:32:9 | LL | (&mut self).bar(); | ^^^^^^^^^^^ cannot borrow as mutable error[E0596]: cannot borrow data in a `&` reference as mutable - --> $DIR/issue-51191.rs:31:9 + --> $DIR/issue-51191.rs:32:9 | LL | (&mut self).bar(); | ^^^^^^^^^^^ cannot borrow as mutable error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable - --> $DIR/issue-51191.rs:37:9 + --> $DIR/issue-51191.rs:38:9 | LL | (&mut self).bar(); | ^^^^^^^^^^^ diff --git a/src/test/ui/regions/region-bound-on-closure-outlives-call.nll.stderr b/src/test/ui/regions/region-bound-on-closure-outlives-call.nll.stderr index da6ebaaefadf1..f0fb57039c465 100644 --- a/src/test/ui/regions/region-bound-on-closure-outlives-call.nll.stderr +++ b/src/test/ui/regions/region-bound-on-closure-outlives-call.nll.stderr @@ -1,5 +1,17 @@ +warning: function cannot return without recursing + --> $DIR/region-bound-on-closure-outlives-call.rs:11:1 + | +LL | fn call_rec(mut f: F) -> usize where F: FnMut(usize) -> usize { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing +LL | //~^ WARN function cannot return without recursing +LL | (|x| f(x))(call_rec(f)) //~ ERROR cannot move out of `f` + | ----------- recursive call site + | + = note: #[warn(unconditional_recursion)] on by default + = help: a `loop` may express intention better if this is on purpose + error[E0505]: cannot move out of `f` because it is borrowed - --> $DIR/region-bound-on-closure-outlives-call.rs:12:25 + --> $DIR/region-bound-on-closure-outlives-call.rs:13:25 | LL | (|x| f(x))(call_rec(f)) //~ ERROR cannot move out of `f` | --------------------^-- diff --git a/src/test/ui/regions/region-bound-on-closure-outlives-call.rs b/src/test/ui/regions/region-bound-on-closure-outlives-call.rs index b73c283fa515e..f931e281c83fc 100644 --- a/src/test/ui/regions/region-bound-on-closure-outlives-call.rs +++ b/src/test/ui/regions/region-bound-on-closure-outlives-call.rs @@ -9,6 +9,7 @@ // except according to those terms. fn call_rec(mut f: F) -> usize where F: FnMut(usize) -> usize { + //~^ WARN function cannot return without recursing (|x| f(x))(call_rec(f)) //~ ERROR cannot move out of `f` } diff --git a/src/test/ui/regions/region-bound-on-closure-outlives-call.stderr b/src/test/ui/regions/region-bound-on-closure-outlives-call.stderr index 7adf68ee9b6d1..6465f1ccf3345 100644 --- a/src/test/ui/regions/region-bound-on-closure-outlives-call.stderr +++ b/src/test/ui/regions/region-bound-on-closure-outlives-call.stderr @@ -1,5 +1,17 @@ +warning: function cannot return without recursing + --> $DIR/region-bound-on-closure-outlives-call.rs:11:1 + | +LL | fn call_rec(mut f: F) -> usize where F: FnMut(usize) -> usize { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing +LL | //~^ WARN function cannot return without recursing +LL | (|x| f(x))(call_rec(f)) //~ ERROR cannot move out of `f` + | ----------- recursive call site + | + = note: #[warn(unconditional_recursion)] on by default + = help: a `loop` may express intention better if this is on purpose + error[E0505]: cannot move out of `f` because it is borrowed - --> $DIR/region-bound-on-closure-outlives-call.rs:12:25 + --> $DIR/region-bound-on-closure-outlives-call.rs:13:25 | LL | (|x| f(x))(call_rec(f)) //~ ERROR cannot move out of `f` | --- ^ move out of `f` occurs here From 7d3d835c58e789127744076c97110664ea9c6087 Mon Sep 17 00:00:00 2001 From: Philip Munksgaard Date: Thu, 4 Oct 2018 22:17:01 +0200 Subject: [PATCH 03/30] replace escape-rust-expr test with dont-show-const-contents The old test was supposed to check for proper html escaping when showing the contents of constants. This was changed as part of #53409. The revised test asserts that the contents of the constant is not shown as part of the generated documentation. --- .../{escape-rust-expr.rs => dont-show-const-contents.rs} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename src/test/rustdoc/{escape-rust-expr.rs => dont-show-const-contents.rs} (64%) diff --git a/src/test/rustdoc/escape-rust-expr.rs b/src/test/rustdoc/dont-show-const-contents.rs similarity index 64% rename from src/test/rustdoc/escape-rust-expr.rs rename to src/test/rustdoc/dont-show-const-contents.rs index 4594eb95ea18e..1392c62b4abb1 100644 --- a/src/test/rustdoc/escape-rust-expr.rs +++ b/src/test/rustdoc/dont-show-const-contents.rs @@ -8,8 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// Test that we HTML-escape Rust expressions, where HTML special chars -// can occur, and we know it's definitely not markup. +// Test that the contents of constants are not displayed as part of the +// documentation. -// @!has escape_rust_expr/constant.CONST_S.html '//pre[@class="rust const"]' '"