Skip to content
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

Rollup of 5 pull requests #69325

Merged
merged 26 commits into from
Feb 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
683ebc2
On mismatched argument count point at arguments
estebank Feb 6, 2020
ad4b3f3
const-prop: use one helper method for all lints; consistently lint ov…
RalfJung Feb 14, 2020
bd48522
fix tests, and use variants to test debug and release builds together
RalfJung Feb 15, 2020
9c76394
more revisions and use them for another test
RalfJung Feb 15, 2020
4b8c784
add test for issue 69020
RalfJung Feb 15, 2020
2107e73
fix exceeding_bitshift lint and test
RalfJung Feb 15, 2020
415218f
expand assoc-const test a bit, just to be sure
RalfJung Feb 15, 2020
25870a0
fix another test
RalfJung Feb 15, 2020
c4a6f84
fix compile-fail
RalfJung Feb 15, 2020
0c8c800
Tighter type bounds for messages
RalfJung Feb 15, 2020
3134df2
adjust run-fail tests
RalfJung Feb 15, 2020
94047f1
remove no-longer-needed test
RalfJung Feb 15, 2020
b6aaacd
fix codegen tests
RalfJung Feb 15, 2020
97cc3a2
fix incremental tests
RalfJung Feb 15, 2020
5e19350
better lint names
RalfJung Feb 18, 2020
58bb47e
avoid excessive number of revisions
RalfJung Feb 19, 2020
88d14bf
fix 32bit-only test
RalfJung Feb 19, 2020
c816430
Tweak binding lifetime suggestion text
estebank Feb 20, 2020
a6b5f87
clean up E0321 explanation
GuillaumeGomez Feb 20, 2020
90ebf93
Greatly improve E0322 explanation
GuillaumeGomez Feb 20, 2020
8e793ed
Fix broken link to the rustc guide
LeSeulArtichaut Feb 20, 2020
b680a5e
Rollup merge of #68877 - estebank:point-at-params, r=petrochenkov
Centril Feb 20, 2020
d237e0f
Rollup merge of #69185 - RalfJung:const-prop-lints, r=oli-obk
Centril Feb 20, 2020
1facbb8
Rollup merge of #69305 - estebank:consider-lt, r=Dylan-DPC
Centril Feb 20, 2020
362f650
Rollup merge of #69311 - GuillaumeGomez:clean-up-e0321-e0322, r=Dylan…
Centril Feb 20, 2020
c1165ce
Rollup merge of #69317 - LeSeulArtichaut:patch-1, r=Dylan-DPC
Centril Feb 20, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/librustc_error_codes/error_codes/E0321.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
A cross-crate opt-out trait was implemented on something which wasn't a struct
or enum type. Erroneous code example:
or enum type.

Erroneous code example:

```compile_fail,E0321
#![feature(optin_builtin_traits)]
Expand Down
10 changes: 10 additions & 0 deletions src/librustc_error_codes/error_codes/E0322.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
The `Sized` trait was implemented explicitly.

Erroneous code example:

```compile_fail,E0322
struct Foo;

impl Sized for Foo {} // error!
```

The `Sized` trait is a special trait built-in to the compiler for types with a
constant size known at compile-time. This trait is automatically implemented
for types as needed by the compiler, and it is currently disallowed to
Expand Down
22 changes: 12 additions & 10 deletions src/librustc_infer/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1781,28 +1781,30 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
bound_kind: GenericKind<'tcx>,
sub: S,
) {
let consider = format!(
"consider adding an explicit lifetime bound {}",
if type_param_span.map(|(_, _, is_impl_trait)| is_impl_trait).unwrap_or(false) {
format!(" `{}` to `{}`...", sub, bound_kind)
} else {
format!("`{}: {}`...", bound_kind, sub)
},
);
let msg = "consider adding an explicit lifetime bound";
if let Some((sp, has_lifetimes, is_impl_trait)) = type_param_span {
let suggestion = if is_impl_trait {
format!("{} + {}", bound_kind, sub)
} else {
let tail = if has_lifetimes { " + " } else { "" };
format!("{}: {}{}", bound_kind, sub, tail)
};
err.span_suggestion_short(
err.span_suggestion(
sp,
&consider,
&format!("{}...", msg),
suggestion,
Applicability::MaybeIncorrect, // Issue #41966
);
} else {
let consider = format!(
"{} {}...",
msg,
if type_param_span.map(|(_, _, is_impl_trait)| is_impl_trait).unwrap_or(false) {
format!(" `{}` to `{}`", sub, bound_kind)
} else {
format!("`{}: {}`", bound_kind, sub)
},
);
err.help(&consider);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
store.register_renamed("unstable_name_collision", "unstable_name_collisions");
store.register_renamed("unused_doc_comment", "unused_doc_comments");
store.register_renamed("async_idents", "keyword_idents");
store.register_renamed("exceeding_bitshifts", "arithmetic_overflow");
store.register_removed("unknown_features", "replaced by an error");
store.register_removed("unsigned_negation", "replaced by negate_unsigned feature gate");
store.register_removed("negate_unsigned", "cast a signed value instead");
Expand Down
177 changes: 86 additions & 91 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
use std::borrow::Cow;
use std::cell::Cell;

use rustc::mir::interpret::{InterpError, InterpResult, Scalar};
use rustc::lint;
use rustc::mir::interpret::{InterpResult, Scalar};
use rustc::mir::visit::{
MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor,
};
Expand Down Expand Up @@ -292,7 +293,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine {
struct ConstPropagator<'mir, 'tcx> {
ecx: InterpCx<'mir, 'tcx, ConstPropMachine>,
tcx: TyCtxt<'tcx>,
source: MirSource<'tcx>,
can_const_prop: IndexVec<Local, ConstPropMode>,
param_env: ParamEnv<'tcx>,
// FIXME(eddyb) avoid cloning these two fields more than once,
Expand Down Expand Up @@ -372,7 +372,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
ConstPropagator {
ecx,
tcx,
source,
param_env,
can_const_prop,
// FIXME(eddyb) avoid cloning these two fields more than once,
Expand Down Expand Up @@ -501,19 +500,20 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}
}

fn report_panic_as_lint(&self, source_info: SourceInfo, panic: AssertKind<u64>) -> Option<()> {
// Somewhat convoluted way to re-use the CTFE error reporting code.
fn report_assert_as_lint(
&self,
lint: &'static lint::Lint,
source_info: SourceInfo,
message: &'static str,
panic: AssertKind<u64>,
) -> Option<()> {
let lint_root = self.lint_root(source_info)?;
let error = InterpError::MachineStop(Box::new(format!("{:?}", panic)));
let mut diagnostic = error_to_const_error(&self.ecx, error.into());
diagnostic.span = source_info.span; // fix the span
diagnostic.report_as_lint(
self.tcx.at(source_info.span),
"this expression will panic at runtime",
lint_root,
None,
);
None
self.tcx.struct_span_lint_hir(lint, lint_root, source_info.span, |lint| {
let mut err = lint.build(message);
err.span_label(source_info.span, format!("{:?}", panic));
err.emit()
});
return None;
}

fn check_unary_op(
Expand All @@ -530,7 +530,12 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
// `AssertKind` only has an `OverflowNeg` variant, so make sure that is
// appropriate to use.
assert_eq!(op, UnOp::Neg, "Neg is the only UnOp that can overflow");
self.report_panic_as_lint(source_info, AssertKind::OverflowNeg)?;
self.report_assert_as_lint(
lint::builtin::ARITHMETIC_OVERFLOW,
source_info,
"this arithmetic operation will overflow",
AssertKind::OverflowNeg,
)?;
}

Some(())
Expand All @@ -542,27 +547,24 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
left: &Operand<'tcx>,
right: &Operand<'tcx>,
source_info: SourceInfo,
place_layout: TyLayout<'tcx>,
) -> Option<()> {
let r =
self.use_ecx(|this| this.ecx.read_immediate(this.ecx.eval_operand(right, None)?))?;
// Check for exceeding shifts *even if* we cannot evaluate the LHS.
if op == BinOp::Shr || op == BinOp::Shl {
let left_bits = place_layout.size.bits();
// We need the type of the LHS. We cannot use `place_layout` as that is the type
// of the result, which for checked binops is not the same!
let left_ty = left.ty(&self.local_decls, self.tcx);
let left_size_bits = self.ecx.layout_of(left_ty).ok()?.size.bits();
let right_size = r.layout.size;
let r_bits = r.to_scalar().and_then(|r| r.to_bits(right_size));
if r_bits.map_or(false, |b| b >= left_bits as u128) {
let lint_root = self.lint_root(source_info)?;
self.tcx.struct_span_lint_hir(
::rustc::lint::builtin::EXCEEDING_BITSHIFTS,
lint_root,
source_info.span,
|lint| {
let dir = if op == BinOp::Shr { "right" } else { "left" };
lint.build(&format!("attempt to shift {} with overflow", dir)).emit()
},
);
return None;
if r_bits.map_or(false, |b| b >= left_size_bits as u128) {
self.report_assert_as_lint(
lint::builtin::ARITHMETIC_OVERFLOW,
source_info,
"this arithmetic operation will overflow",
AssertKind::Overflow(op),
)?;
}
}

Expand All @@ -572,7 +574,12 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;
Ok(overflow)
})? {
self.report_panic_as_lint(source_info, AssertKind::Overflow(op))?;
self.report_assert_as_lint(
lint::builtin::ARITHMETIC_OVERFLOW,
source_info,
"this arithmetic operation will overflow",
AssertKind::Overflow(op),
)?;
}

Some(())
Expand All @@ -595,8 +602,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
return None;
}

let overflow_check = self.tcx.sess.overflow_checks();

// Perform any special handling for specific Rvalue types.
// Generally, checks here fall into one of two categories:
// 1. Additional checking to provide useful lints to the user
Expand All @@ -606,20 +611,25 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
// - In this case, we'll return `None` from this function to stop evaluation.
match rvalue {
// Additional checking: give lints to the user if an overflow would occur.
// If `overflow_check` is set, running const-prop on the `Assert` terminators
// will already generate the appropriate messages.
Rvalue::UnaryOp(op, arg) if !overflow_check => {
// We do this here and not in the `Assert` terminator as that terminator is
// only sometimes emitted (overflow checks can be disabled), but we want to always
// lint.
Rvalue::UnaryOp(op, arg) => {
trace!("checking UnaryOp(op = {:?}, arg = {:?})", op, arg);
self.check_unary_op(*op, arg, source_info)?;
}

// Additional checking: check for overflows on integer binary operations and report
// them to the user as lints.
// If `overflow_check` is set, running const-prop on the `Assert` terminators
// will already generate the appropriate messages.
Rvalue::BinaryOp(op, left, right) if !overflow_check => {
Rvalue::BinaryOp(op, left, right) => {
trace!("checking BinaryOp(op = {:?}, left = {:?}, right = {:?})", op, left, right);
self.check_binary_op(*op, left, right, source_info, place_layout)?;
self.check_binary_op(*op, left, right, source_info)?;
}
Rvalue::CheckedBinaryOp(op, left, right) => {
trace!(
"checking CheckedBinaryOp(op = {:?}, left = {:?}, right = {:?})",
op,
left,
right
);
self.check_binary_op(*op, left, right, source_info)?;
}

// Do not try creating references (#67862)
Expand Down Expand Up @@ -898,54 +908,39 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
}
Operand::Constant(_) => {}
}
let span = terminator.source_info.span;
let hir_id = self
.tcx
.hir()
.as_local_hir_id(self.source.def_id())
.expect("some part of a failing const eval must be local");
self.tcx.struct_span_lint_hir(
::rustc::lint::builtin::CONST_ERR,
hir_id,
span,
|lint| {
let msg = match msg {
AssertKind::Overflow(_)
| AssertKind::OverflowNeg
| AssertKind::DivisionByZero
| AssertKind::RemainderByZero => msg.description().to_owned(),
AssertKind::BoundsCheck { ref len, ref index } => {
let len = self
.eval_operand(len, source_info)
.expect("len must be const");
let len = match self.ecx.read_scalar(len) {
Ok(ScalarMaybeUndef::Scalar(Scalar::Raw {
data,
..
})) => data,
other => bug!("const len not primitive: {:?}", other),
};
let index = self
.eval_operand(index, source_info)
.expect("index must be const");
let index = match self.ecx.read_scalar(index) {
Ok(ScalarMaybeUndef::Scalar(Scalar::Raw {
data,
..
})) => data,
other => bug!("const index not primitive: {:?}", other),
};
format!(
"index out of bounds: \
the len is {} but the index is {}",
len, index,
)
}
// Need proper const propagator for these
_ => return,
};
lint.build(&msg).emit()
},
let msg = match msg {
AssertKind::DivisionByZero => AssertKind::DivisionByZero,
AssertKind::RemainderByZero => AssertKind::RemainderByZero,
AssertKind::BoundsCheck { ref len, ref index } => {
let len =
self.eval_operand(len, source_info).expect("len must be const");
let len = self
.ecx
.read_scalar(len)
.unwrap()
.to_machine_usize(&self.tcx)
.unwrap();
let index = self
.eval_operand(index, source_info)
.expect("index must be const");
let index = self
.ecx
.read_scalar(index)
.unwrap()
.to_machine_usize(&self.tcx)
.unwrap();
AssertKind::BoundsCheck { len, index }
}
// Overflow is are already covered by checks on the binary operators.
AssertKind::Overflow(_) | AssertKind::OverflowNeg => return,
// Need proper const propagator for these.
_ => return,
};
self.report_assert_as_lint(
lint::builtin::UNCONDITIONAL_PANIC,
source_info,
"this operation will panic at runtime",
msg,
);
} else {
if self.should_const_prop(value) {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_passes/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! For more information about how MIR-based region-checking works,
//! see the [rustc guide].
//!
//! [rustc guide]: https://rust-lang.github.io/rustc-guide/mir/borrowck.html
//! [rustc guide]: https://rust-lang.github.io/rustc-guide/borrow_check.html

use rustc::hir::map::Map;
use rustc::middle::region::*;
Expand Down
13 changes: 10 additions & 3 deletions src/librustc_session/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,15 @@ declare_lint! {
}

declare_lint! {
pub EXCEEDING_BITSHIFTS,
pub ARITHMETIC_OVERFLOW,
Deny,
"shift exceeds the type's number of bits"
"arithmetic operation overflows"
}

declare_lint! {
pub UNCONDITIONAL_PANIC,
Deny,
"operation will cause a panic at runtime"
}

declare_lint! {
Expand Down Expand Up @@ -495,7 +501,8 @@ declare_lint_pass! {
/// that are used by other parts of the compiler.
HardwiredLints => [
ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
EXCEEDING_BITSHIFTS,
ARITHMETIC_OVERFLOW,
UNCONDITIONAL_PANIC,
UNUSED_IMPORTS,
UNUSED_EXTERN_CRATES,
UNUSED_QUALIFICATIONS,
Expand Down
Loading