From f7d35cc5a8ffcf74937e8c00586d97609b1095ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Romanowski?= Date: Mon, 1 Oct 2018 23:23:57 +0100 Subject: [PATCH] Fix range borrowing suggestions logic --- src/librustc_typeck/check/demand.rs | 54 +++++-- .../ui/range/issue-54505-no-literals.fixed | 85 ++++++++++ .../ui/range/issue-54505-no-literals.stderr | 147 ++++++++++++++++++ src/test/ui/range/issue-54505-no-std.rs | 5 +- src/test/ui/range/issue-54505-no-std.stderr | 79 ++++++++++ 5 files changed, 354 insertions(+), 16 deletions(-) create mode 100644 src/test/ui/range/issue-54505-no-literals.fixed create mode 100644 src/test/ui/range/issue-54505-no-literals.stderr create mode 100644 src/test/ui/range/issue-54505-no-std.stderr diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index f53a8168548..e0577add61c 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -309,12 +309,16 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }; if self.can_coerce(ref_ty, expected) { if let Ok(src) = cm.span_to_snippet(sp) { - let sugg_expr = match expr.node { // parenthesize if needed (Issue #46756) - hir::ExprKind::Cast(_, _) | - hir::ExprKind::Binary(_, _, _) | - _ if self.is_range_literal(expr) => format!("({})", src), - _ => src, + let needs_parens = match expr.node { + // parenthesize if needed (Issue #46756) + hir::ExprKind::Cast(_, _) | + hir::ExprKind::Binary(_, _, _) => true, + // parenthesize borrows of range literals (Issue #54505) + _ if self.is_range_literal(expr) => true, + _ => false, }; + let sugg_expr = if needs_parens { format!("({})", src) } else { src }; + if let Some(sugg) = self.can_use_as_ref(expr) { return Some(sugg); } @@ -380,8 +384,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { fn is_range_literal(&self, expr: &hir::Expr) -> bool { use hir::{Path, QPath, ExprKind, TyKind}; - // TODO how to work out std vs core here? - let ops_path = ["{{root}}", "std", "ops"]; + // we support `::std::ops::Range` and `::std::core::Range` prefixes + // (via split on "|") + let ops_path = ["{{root}}", "std|core", "ops"]; let is_range_path = |path: &Path| { let ident_names: Vec<_> = path.segments @@ -394,24 +399,47 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { preceding.len() == 3 && preceding.iter() .zip(ops_path.iter()) - .all(|(a, b)| a == b) + .all(|(seg, match_seg)| { + match_seg.split("|") + .into_iter() + .any(|ref spl_seg| seg == spl_seg) + }) } else { false } }; - match expr.node { - ExprKind::Struct(QPath::Resolved(None, ref path), _, _) | - ExprKind::Path(QPath::Resolved(None, ref path)) => { - return is_range_path(&path); + let is_range_struct_snippet = |span: &Span| { + // Tell if expression span snippet looks like an explicit + // Range struct or new() call. This is to allow rejecting + // Ranges constructed with non-literals. + let source_map = self.tcx.sess.source_map(); + let end_point = source_map.end_point(*span); + + if let Ok(end_string) = source_map.span_to_snippet(end_point) { + end_string.ends_with("}") || end_string.ends_with(")") + } else { + false } + }; + + match expr.node { + // all built-in range literals but `..=` and `..` + // desugar to Structs, `..` desugars to its struct path + ExprKind::Struct(QPath::Resolved(None, ref path), _, _) | + ExprKind::Path(QPath::Resolved(None, ref path)) => { + return is_range_path(&path) && !is_range_struct_snippet(&expr.span); + } + + // `..=` desugars into RangeInclusive::new(...) ExprKind::Call(ref func, _) => { if let ExprKind::Path(QPath::TypeRelative(ref ty, ref segment)) = func.node { if let TyKind::Path(QPath::Resolved(None, ref path)) = ty.node { let calls_new = segment.ident.as_str() == "new"; - return is_range_path(&path) && calls_new; + return is_range_path(&path) && calls_new && + !is_range_struct_snippet(&expr.span); } } } diff --git a/src/test/ui/range/issue-54505-no-literals.fixed b/src/test/ui/range/issue-54505-no-literals.fixed new file mode 100644 index 00000000000..5be024dff53 --- /dev/null +++ b/src/test/ui/range/issue-54505-no-literals.fixed @@ -0,0 +1,85 @@ +// 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. + +// run-rustfix + +// Regression test for changes introduced while fixing #54505 + +// This test uses non-literals for Ranges +// (expecting no parens with borrow suggestion) + +use std::ops::RangeBounds; + + +// take a reference to any built-in range +fn take_range(_r: &impl RangeBounds) {} + + +fn main() { + take_range(&std::ops::Range { start: 0, end: 1 }); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &std::ops::Range { start: 0, end: 1 } + + take_range(&::std::ops::Range { start: 0, end: 1 }); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &::std::ops::Range { start: 0, end: 1 } + + take_range(&std::ops::RangeFrom { start: 1 }); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &std::ops::RangeFrom { start: 1 } + + take_range(&::std::ops::RangeFrom { start: 1 }); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &::std::ops::RangeFrom { start: 1 } + + take_range(&std::ops::RangeFull {}); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &std::ops::RangeFull {} + + take_range(&::std::ops::RangeFull {}); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &::std::ops::RangeFull {} + + take_range(&std::ops::RangeInclusive::new(0, 1)); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &std::ops::RangeInclusive::new(0, 1) + + take_range(&::std::ops::RangeInclusive::new(0, 1)); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &::std::ops::RangeInclusive::new(0, 1) + + take_range(&std::ops::RangeTo { end: 5 }); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &std::ops::RangeTo { end: 5 } + + take_range(&::std::ops::RangeTo { end: 5 }); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &::std::ops::RangeTo { end: 5 } + + take_range(&std::ops::RangeToInclusive { end: 5 }); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &std::ops::RangeToInclusive { end: 5 } + + take_range(&::std::ops::RangeToInclusive { end: 5 }); + //~^ ERROR mismatched types [E0308] + //~| HELP consider borrowing here + //~| SUGGESTION &::std::ops::RangeToInclusive { end: 5 } +} diff --git a/src/test/ui/range/issue-54505-no-literals.stderr b/src/test/ui/range/issue-54505-no-literals.stderr new file mode 100644 index 00000000000..a8be4de5f0e --- /dev/null +++ b/src/test/ui/range/issue-54505-no-literals.stderr @@ -0,0 +1,147 @@ +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:26:16 + | +LL | take_range(std::ops::Range { start: 0, end: 1 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::Range` + | help: consider borrowing here: `&std::ops::Range { start: 0, end: 1 }` + | + = note: expected type `&_` + found type `std::ops::Range<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:31:16 + | +LL | take_range(::std::ops::Range { start: 0, end: 1 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::Range` + | help: consider borrowing here: `&::std::ops::Range { start: 0, end: 1 }` + | + = note: expected type `&_` + found type `std::ops::Range<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:36:16 + | +LL | take_range(std::ops::RangeFrom { start: 1 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::RangeFrom` + | help: consider borrowing here: `&std::ops::RangeFrom { start: 1 }` + | + = note: expected type `&_` + found type `std::ops::RangeFrom<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:41:16 + | +LL | take_range(::std::ops::RangeFrom { start: 1 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::RangeFrom` + | help: consider borrowing here: `&::std::ops::RangeFrom { start: 1 }` + | + = note: expected type `&_` + found type `std::ops::RangeFrom<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:46:16 + | +LL | take_range(std::ops::RangeFull {}); + | ^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::RangeFull` + | help: consider borrowing here: `&std::ops::RangeFull {}` + | + = note: expected type `&_` + found type `std::ops::RangeFull` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:51:16 + | +LL | take_range(::std::ops::RangeFull {}); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::RangeFull` + | help: consider borrowing here: `&::std::ops::RangeFull {}` + | + = note: expected type `&_` + found type `std::ops::RangeFull` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:56:16 + | +LL | take_range(std::ops::RangeInclusive::new(0, 1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::RangeInclusive` + | help: consider borrowing here: `&std::ops::RangeInclusive::new(0, 1)` + | + = note: expected type `&_` + found type `std::ops::RangeInclusive<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:61:16 + | +LL | take_range(::std::ops::RangeInclusive::new(0, 1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::RangeInclusive` + | help: consider borrowing here: `&::std::ops::RangeInclusive::new(0, 1)` + | + = note: expected type `&_` + found type `std::ops::RangeInclusive<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:66:16 + | +LL | take_range(std::ops::RangeTo { end: 5 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::RangeTo` + | help: consider borrowing here: `&std::ops::RangeTo { end: 5 }` + | + = note: expected type `&_` + found type `std::ops::RangeTo<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:71:16 + | +LL | take_range(::std::ops::RangeTo { end: 5 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::RangeTo` + | help: consider borrowing here: `&::std::ops::RangeTo { end: 5 }` + | + = note: expected type `&_` + found type `std::ops::RangeTo<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:76:16 + | +LL | take_range(std::ops::RangeToInclusive { end: 5 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::RangeToInclusive` + | help: consider borrowing here: `&std::ops::RangeToInclusive { end: 5 }` + | + = note: expected type `&_` + found type `std::ops::RangeToInclusive<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-literals.rs:81:16 + | +LL | take_range(::std::ops::RangeToInclusive { end: 5 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | expected reference, found struct `std::ops::RangeToInclusive` + | help: consider borrowing here: `&::std::ops::RangeToInclusive { end: 5 }` + | + = note: expected type `&_` + found type `std::ops::RangeToInclusive<{integer}>` + +error: aborting due to 12 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/range/issue-54505-no-std.rs b/src/test/ui/range/issue-54505-no-std.rs index 8e8793c1736..0dadaecd7d8 100644 --- a/src/test/ui/range/issue-54505-no-std.rs +++ b/src/test/ui/range/issue-54505-no-std.rs @@ -8,9 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// run-rustfix -// error-pattern: error: `#[panic_handler]` function required, but not found -// error-pattern: language item required, but not found: `panic_info` +// error-pattern: `#[panic_handler]` function required, but not found +// error-pattern: language item required, but not found: `eh_personality` // Regression test for #54505 - range borrowing suggestion had diff --git a/src/test/ui/range/issue-54505-no-std.stderr b/src/test/ui/range/issue-54505-no-std.stderr new file mode 100644 index 00000000000..035bc640658 --- /dev/null +++ b/src/test/ui/range/issue-54505-no-std.stderr @@ -0,0 +1,79 @@ +error: `#[panic_handler]` function required, but not found + +error: language item required, but not found: `eh_personality` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-std.rs:31:16 + | +LL | take_range(0..1); + | ^^^^ + | | + | expected reference, found struct `core::ops::Range` + | help: consider borrowing here: `&(0..1)` + | + = note: expected type `&_` + found type `core::ops::Range<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-std.rs:36:16 + | +LL | take_range(1..); + | ^^^ + | | + | expected reference, found struct `core::ops::RangeFrom` + | help: consider borrowing here: `&(1..)` + | + = note: expected type `&_` + found type `core::ops::RangeFrom<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-std.rs:41:16 + | +LL | take_range(..); + | ^^ + | | + | expected reference, found struct `core::ops::RangeFull` + | help: consider borrowing here: `&(..)` + | + = note: expected type `&_` + found type `core::ops::RangeFull` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-std.rs:46:16 + | +LL | take_range(0..=1); + | ^^^^^ + | | + | expected reference, found struct `core::ops::RangeInclusive` + | help: consider borrowing here: `&(0..=1)` + | + = note: expected type `&_` + found type `core::ops::RangeInclusive<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-std.rs:51:16 + | +LL | take_range(..5); + | ^^^ + | | + | expected reference, found struct `core::ops::RangeTo` + | help: consider borrowing here: `&(..5)` + | + = note: expected type `&_` + found type `core::ops::RangeTo<{integer}>` + +error[E0308]: mismatched types + --> $DIR/issue-54505-no-std.rs:56:16 + | +LL | take_range(..=42); + | ^^^^^ + | | + | expected reference, found struct `core::ops::RangeToInclusive` + | help: consider borrowing here: `&(..=42)` + | + = note: expected type `&_` + found type `core::ops::RangeToInclusive<{integer}>` + +error: aborting due to 8 previous errors + +For more information about this error, try `rustc --explain E0308`.