Skip to content

Commit

Permalink
Auto merge of #60920 - Manishearth:rollup-p4xp4gk, r=Manishearth
Browse files Browse the repository at this point in the history
Rollup of 4 pull requests

Successful merges:

 - #60791 (Update books)
 - #60891 (Allow claiming issues with triagebot)
 - #60901 (Handle more string addition cases with appropriate suggestions)
 - #60902 (Prevent Error::type_id overrides)

Failed merges:

r? @ghost
  • Loading branch information
bors committed May 17, 2019
2 parents 823a75d + f48f37b commit 73a3a90
Show file tree
Hide file tree
Showing 17 changed files with 253 additions and 46 deletions.
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

0 comments on commit 73a3a90

Please sign in to comment.