Skip to content

Rollup of 4 pull requests #60920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 17, 2019
4 changes: 3 additions & 1 deletion src/ci/docker/x86_64-gnu-tools/checktools.sh
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ status_check() {
check_dispatch $1 beta nomicon src/doc/nomicon
check_dispatch $1 beta reference src/doc/reference
check_dispatch $1 beta rust-by-example src/doc/rust-by-example
check_dispatch $1 beta edition-guide src/doc/edition-guide
# Temporarily disabled until
# https://github.com/rust-lang/rust/issues/60459 is fixed.
# check_dispatch $1 beta edition-guide src/doc/edition-guide
check_dispatch $1 beta rls src/tools/rls
check_dispatch $1 beta rustfmt src/tools/rustfmt
check_dispatch $1 beta clippy-driver src/tools/clippy
Expand Down
2 changes: 1 addition & 1 deletion src/doc/edition-guide
2 changes: 1 addition & 1 deletion src/doc/embedded-book
2 changes: 1 addition & 1 deletion src/doc/nomicon
2 changes: 1 addition & 1 deletion src/doc/reference
2 changes: 1 addition & 1 deletion src/doc/rustc-guide
83 changes: 60 additions & 23 deletions src/librustc_typeck/check/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
};
if let Some(missing_trait) = missing_trait {
if op.node == hir::BinOpKind::Add &&
self.check_str_addition(expr, lhs_expr, rhs_expr, lhs_ty,
rhs_ty, &mut err, true, op) {
self.check_str_addition(
lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, true, op) {
// This has nothing here because it means we did string
// concatenation (e.g., "Hello " += "World!"). This means
// we don't want the note in the else clause to be emitted
Expand Down Expand Up @@ -400,8 +400,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
};
if let Some(missing_trait) = missing_trait {
if op.node == hir::BinOpKind::Add &&
self.check_str_addition(expr, lhs_expr, rhs_expr, lhs_ty,
rhs_ty, &mut err, false, op) {
self.check_str_addition(
lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, false, op) {
// This has nothing here because it means we did string
// concatenation (e.g., "Hello " + "World!"). This means
// we don't want the note in the else clause to be emitted
Expand Down Expand Up @@ -502,9 +502,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
false
}

/// Provide actionable suggestions when trying to add two strings with incorrect types,
/// like `&str + &str`, `String + String` and `&str + &String`.
///
/// If this function returns `true` it means a note was printed, so we don't need
/// to print the normal "implementation of `std::ops::Add` might be missing" note
fn check_str_addition(
&self,
expr: &'gcx hir::Expr,
lhs_expr: &'gcx hir::Expr,
rhs_expr: &'gcx hir::Expr,
lhs_ty: Ty<'tcx>,
Expand All @@ -514,45 +518,78 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
op: hir::BinOp,
) -> bool {
let source_map = self.tcx.sess.source_map();
let remove_borrow_msg = "String concatenation appends the string on the right to the \
string on the left and may require reallocation. This \
requires ownership of the string on the left";

let msg = "`to_owned()` can be used to create an owned `String` \
from a string reference. String concatenation \
appends the string on the right to the string \
on the left and may require reallocation. This \
requires ownership of the string on the left";
// If this function returns true it means a note was printed, so we don't need
// to print the normal "implementation of `std::ops::Add` might be missing" note

let is_std_string = |ty| &format!("{:?}", ty) == "std::string::String";

match (&lhs_ty.sty, &rhs_ty.sty) {
(&Ref(_, l_ty, _), &Ref(_, r_ty, _))
if l_ty.sty == Str && r_ty.sty == Str => {
if !is_assign {
err.span_label(op.span,
"`+` can't be used to concatenate two `&str` strings");
(&Ref(_, l_ty, _), &Ref(_, r_ty, _)) // &str or &String + &str, &String or &&str
if (l_ty.sty == Str || is_std_string(l_ty)) && (
r_ty.sty == Str || is_std_string(r_ty) ||
&format!("{:?}", rhs_ty) == "&&str"
) =>
{
if !is_assign { // Do not supply this message if `&str += &str`
err.span_label(
op.span,
"`+` cannot be used to concatenate two `&str` strings",
);
match source_map.span_to_snippet(lhs_expr.span) {
Ok(lstring) => err.span_suggestion(
lhs_expr.span,
msg,
format!("{}.to_owned()", lstring),
Applicability::MachineApplicable,
),
Ok(lstring) => {
err.span_suggestion(
lhs_expr.span,
if lstring.starts_with("&") {
remove_borrow_msg
} else {
msg
},
if lstring.starts_with("&") {
// let a = String::new();
// let _ = &a + "bar";
format!("{}", &lstring[1..])
} else {
format!("{}.to_owned()", lstring)
},
Applicability::MachineApplicable,
)
}
_ => err.help(msg),
};
}
true
}
(&Ref(_, l_ty, _), &Adt(..))
if l_ty.sty == Str && &format!("{:?}", rhs_ty) == "std::string::String" => {
err.span_label(expr.span,
"`+` can't be used to concatenate a `&str` with a `String`");
(&Ref(_, l_ty, _), &Adt(..)) // Handle `&str` & `&String` + `String`
if (l_ty.sty == Str || is_std_string(l_ty)) && is_std_string(rhs_ty) =>
{
err.span_label(
op.span,
"`+` cannot be used to concatenate a `&str` with a `String`",
);
match (
source_map.span_to_snippet(lhs_expr.span),
source_map.span_to_snippet(rhs_expr.span),
is_assign,
) {
(Ok(l), Ok(r), false) => {
let to_string = if l.starts_with("&") {
// let a = String::new(); let b = String::new();
// let _ = &a + b;
format!("{}", &l[1..])
} else {
format!("{}.to_owned()", l)
};
err.multipart_suggestion(
msg,
vec![
(lhs_expr.span, format!("{}.to_owned()", l)),
(lhs_expr.span, to_string),
(rhs_expr.span, format!("&{}", r)),
],
Applicability::MachineApplicable,
Expand Down
12 changes: 10 additions & 2 deletions src/libstd/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,19 @@ pub trait Error: Debug + Display {
#[unstable(feature = "error_type_id",
reason = "this is memory unsafe to override in user code",
issue = "60784")]
fn type_id(&self) -> TypeId where Self: 'static {
fn type_id(&self, _: private::Internal) -> TypeId where Self: 'static {
TypeId::of::<Self>()
}
}

mod private {
// This is a hack to prevent `type_id` from being overridden by `Error`
// implementations, since that can enable unsound downcasting.
#[unstable(feature = "error_type_id", issue = "60784")]
#[derive(Debug)]
pub struct Internal;
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<'a, E: Error + 'a> From<E> for Box<dyn Error + 'a> {
/// Converts a type of [`Error`] into a box of dyn [`Error`].
Expand Down Expand Up @@ -575,7 +583,7 @@ impl dyn Error + 'static {
let t = TypeId::of::<T>();

// Get TypeId of the type in the trait object
let boxed = self.type_id();
let boxed = self.type_id(private::Internal);

// Compare both TypeIds on equality
t == boxed
Expand Down
3 changes: 1 addition & 2 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3497,8 +3497,7 @@ impl<'a> Parser<'a> {
let binary = self.mk_binary(source_map::respan(cur_op_span, ast_op), lhs, rhs);
self.mk_expr(span, binary, ThinVec::new())
}
AssocOp::Assign =>
self.mk_expr(span, ExprKind::Assign(lhs, rhs), ThinVec::new()),
AssocOp::Assign => self.mk_expr(span, ExprKind::Assign(lhs, rhs), ThinVec::new()),
AssocOp::ObsoleteInPlace =>
self.mk_expr(span, ExprKind::ObsoleteInPlace(lhs, rhs), ThinVec::new()),
AssocOp::AssignOp(k) => {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-47377.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0369]: binary operation `+` cannot be applied to type `&str`
LL | let _a = b + ", World!";
| - ^ ---------- &str
| | |
| | `+` can't be used to concatenate two `&str` strings
| | `+` cannot be used to concatenate two `&str` strings
| &str
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
|
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-47380.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0369]: binary operation `+` cannot be applied to type `&str`
LL | println!("🦀🦀🦀🦀🦀"); let _a = b + ", World!";
| - ^ ---------- &str
| | |
| | `+` can't be used to concatenate two `&str` strings
| | `+` cannot be used to concatenate two `&str` strings
| &str
help: `to_owned()` can be used to create an owned `String` from a string reference. String concatenation appends the string on the right to the string on the left and may require reallocation. This requires ownership of the string on the left
|
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/span/issue-39018.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,23 @@ enum World {
Hello,
Goodbye,
}

fn foo() {
let a = String::new();
let b = String::new();
let c = "";
let d = "";
let e = &a;
let _ = &a + &b; //~ ERROR binary operation
let _ = &a + b; //~ ERROR binary operation
let _ = a + &b; // ok
let _ = a + b; //~ ERROR mismatched types
let _ = e + b; //~ ERROR binary operation
let _ = e + &b; //~ ERROR binary operation
let _ = e + d; //~ ERROR binary operation
let _ = e + &d; //~ ERROR binary operation
let _ = &c + &d; //~ ERROR binary operation
let _ = &c + d; //~ ERROR binary operation
let _ = c + &d; //~ ERROR binary operation
let _ = c + d; //~ ERROR binary operation
}
Loading