Skip to content

Rollup of 6 pull requests #64616

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 24 commits into from
Sep 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8c0f601
fix Miri discriminant load/store when overflows are involved
RalfJung Aug 17, 2019
b73f1a5
better and more consistent variable names
RalfJung Sep 16, 2019
75fe84f
factor getting the discriminant layout to a new method
RalfJung Sep 16, 2019
61a28d5
do the variant idx computations on the host (non-overflowing)
RalfJung Sep 16, 2019
b21622c
add test
RalfJung Sep 16, 2019
822393d
Point at original span when emitting unreachable lint
Aaron1011 Sep 18, 2019
cd4b468
Make note better when all arms in a `match` diverge
Aaron1011 Sep 18, 2019
9e777eb
Some formatting cleanup
Aaron1011 Sep 18, 2019
6edcfbe
Apply formatting fixes
Aaron1011 Sep 18, 2019
a8ce93e
Introduce Diverges::always constructor
Aaron1011 Sep 19, 2019
41e1128
Use 'if let' instead of 'match'
Aaron1011 Sep 19, 2019
034a8fd
Another formatting fix
Aaron1011 Sep 19, 2019
d67528f
Merge inherent impl blocks for `Diverges
Aaron1011 Sep 19, 2019
37bafea
Fix backticks in documentation
grovesNL Sep 19, 2019
1de533a
first determine if the variant is a niche-variant, then compute absol…
RalfJung Sep 19, 2019
c23e78a
Remove unnecessary `mut` in doc example
adrianheine Sep 19, 2019
2fff780
rustbuild: Don't package libstd twice
alexcrichton Sep 19, 2019
d7f6474
rustbuild: Copy crate doc files fewer times
alexcrichton Sep 19, 2019
bda52e5
Rollup merge of #63448 - RalfJung:miri-discriminant, r=eddyb
Centril Sep 19, 2019
3c28682
Rollup merge of #64592 - Aaron1011:feature/unreachable-span, r=Centril
Centril Sep 19, 2019
908636f
Rollup merge of #64601 - grovesNL:two-backticks, r=jonas-schievink
Centril Sep 19, 2019
ea58ed9
Rollup merge of #64606 - adrianheine:patch-1, r=sfackler
Centril Sep 19, 2019
ff7a9cf
Rollup merge of #64611 - alexcrichton:no-libstd-twice, r=Mark-Simulacrum
Centril Sep 19, 2019
99cbffb
Rollup merge of #64613 - alexcrichton:less-doc-copies, r=Mark-Simulacrum
Centril Sep 19, 2019
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
2 changes: 1 addition & 1 deletion src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ impl Step for Analysis {
return distdir(builder).join(format!("{}-{}.tar.gz", name, target));
}

builder.ensure(Std { compiler, target });
builder.ensure(compile::Std { compiler, target });

let image = tmpdir(builder).join(format!("{}-{}-image", name, target));

Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,11 +476,11 @@ impl Step for Std {
.arg("--index-page").arg(&builder.src.join("src/doc/index.md"));

builder.run(&mut cargo);
builder.cp_r(&my_out, &out);
};
for krate in &["alloc", "core", "std", "proc_macro", "test"] {
run_cargo_rustdoc_for(krate);
}
builder.cp_r(&my_out, &out);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1136,7 +1136,7 @@ fn resolve_local<'tcx>(
// Rule A. `let (ref x, ref y) = (foo().x, 44)`. The rvalue `(22, 44)`
// would have an extended lifetime, but not `foo()`.
//
// Rule B. `let x = &foo().x`. The rvalue ``foo()` would have extended
// Rule B. `let x = &foo().x`. The rvalue `foo()` would have extended
// lifetime.
//
// In some cases, multiple rules may apply (though not to the same
Expand Down
11 changes: 11 additions & 0 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ impl IntegerExt for Integer {

pub trait PrimitiveExt {
fn to_ty<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx>;
fn to_int_ty<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx>;
}

impl PrimitiveExt for Primitive {
Expand All @@ -138,6 +139,16 @@ impl PrimitiveExt for Primitive {
Pointer => tcx.mk_mut_ptr(tcx.mk_unit()),
}
}

/// Return an *integer* type matching this primitive.
/// Useful in particular when dealing with enum discriminants.
fn to_int_ty(&self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> {
match *self {
Int(i, signed) => i.to_ty(tcx, signed),
Pointer => tcx.types.usize,
Float(..) => bug!("floats do not have an int type"),
}
}
}

/// The first half of a fat pointer.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/query/on_disk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ impl<'a, 'tcx> SpecializedDecoder<DefIndex> for CacheDecoder<'a, 'tcx> {

// Both the `CrateNum` and the `DefIndex` of a `DefId` can change in between two
// compilation sessions. We use the `DefPathHash`, which is stable across
// sessions, to map the old DefId`` to the new one.
// sessions, to map the old `DefId` to the new one.
impl<'a, 'tcx> SpecializedDecoder<DefId> for CacheDecoder<'a, 'tcx> {
#[inline]
fn specialized_decode(&mut self) -> Result<DefId, Self::Error> {
Expand Down
53 changes: 37 additions & 16 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
//! Functions concerning immediate values and operands, and reading from operands.
//! All high-level functions to read from memory work on operands as sources.

use std::convert::TryInto;
use std::convert::{TryInto, TryFrom};

use rustc::{mir, ty};
use rustc::ty::layout::{
self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, VariantIdx,
self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, PrimitiveExt, VariantIdx,
};

use rustc::mir::interpret::{
Expand Down Expand Up @@ -609,15 +609,20 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx, (u128, VariantIdx)> {
trace!("read_discriminant_value {:#?}", rval.layout);

let (discr_kind, discr_index) = match rval.layout.variants {
let (discr_layout, discr_kind, discr_index) = match rval.layout.variants {
layout::Variants::Single { index } => {
let discr_val = rval.layout.ty.discriminant_for_variant(*self.tcx, index).map_or(
index.as_u32() as u128,
|discr| discr.val);
return Ok((discr_val, index));
}
layout::Variants::Multiple { ref discr_kind, discr_index, .. } =>
(discr_kind, discr_index),
layout::Variants::Multiple {
discr: ref discr_layout,
ref discr_kind,
discr_index,
..
} =>
(discr_layout, discr_kind, discr_index),
};

// read raw discriminant value
Expand All @@ -634,7 +639,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
.map_err(|_| err_unsup!(InvalidDiscriminant(raw_discr.erase_tag())))?;
let real_discr = if discr_val.layout.ty.is_signed() {
// going from layout tag type to typeck discriminant type
// requires first sign extending with the layout discriminant
// requires first sign extending with the discriminant layout
let sexted = sign_extend(bits_discr, discr_val.layout.size) as i128;
// and then zeroing with the typeck discriminant type
let discr_ty = rval.layout.ty
Expand Down Expand Up @@ -666,8 +671,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
ref niche_variants,
niche_start,
} => {
let variants_start = niche_variants.start().as_u32() as u128;
let variants_end = niche_variants.end().as_u32() as u128;
let variants_start = niche_variants.start().as_u32();
let variants_end = niche_variants.end().as_u32();
let raw_discr = raw_discr.not_undef().map_err(|_| {
err_unsup!(InvalidDiscriminant(ScalarMaybeUndef::Undef))
})?;
Expand All @@ -682,18 +687,34 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
(dataful_variant.as_u32() as u128, dataful_variant)
},
Ok(raw_discr) => {
let adjusted_discr = raw_discr.wrapping_sub(niche_start)
.wrapping_add(variants_start);
if variants_start <= adjusted_discr && adjusted_discr <= variants_end {
let index = adjusted_discr as usize;
assert_eq!(index as u128, adjusted_discr);
assert!(index < rval.layout.ty
// We need to use machine arithmetic to get the relative variant idx:
// variant_index_relative = discr_val - niche_start_val
let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?;
let discr_val = ImmTy::from_uint(raw_discr, discr_layout);
let niche_start_val = ImmTy::from_uint(niche_start, discr_layout);
let variant_index_relative_val = self.binary_op(
mir::BinOp::Sub,
discr_val,
niche_start_val,
)?;
let variant_index_relative = variant_index_relative_val
.to_scalar()?
.assert_bits(discr_val.layout.size);
// Check if this is in the range that indicates an actual discriminant.
if variant_index_relative <= u128::from(variants_end - variants_start) {
let variant_index_relative = u32::try_from(variant_index_relative)
.expect("we checked that this fits into a u32");
// Then computing the absolute variant idx should not overflow any more.
let variant_index = variants_start
.checked_add(variant_index_relative)
.expect("oveflow computing absolute variant idx");
assert!((variant_index as usize) < rval.layout.ty
.ty_adt_def()
.expect("tagged layout for non adt")
.variants.len());
(adjusted_discr, VariantIdx::from_usize(index))
(u128::from(variant_index), VariantIdx::from_u32(variant_index))
} else {
(dataful_variant.as_u32() as u128, dataful_variant)
(u128::from(dataful_variant.as_u32()), dataful_variant)
}
},
}
Expand Down
34 changes: 23 additions & 11 deletions src/librustc_mir/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ use std::hash::Hash;
use rustc::mir;
use rustc::mir::interpret::truncate;
use rustc::ty::{self, Ty};
use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx};
use rustc::ty::layout::{
self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, PrimitiveExt
};
use rustc::ty::TypeFoldable;

use super::{
Expand Down Expand Up @@ -1027,7 +1029,7 @@ where
}
layout::Variants::Multiple {
discr_kind: layout::DiscriminantKind::Tag,
ref discr,
discr: ref discr_layout,
discr_index,
..
} => {
Expand All @@ -1038,7 +1040,7 @@ where
// raw discriminants for enums are isize or bigger during
// their computation, but the in-memory tag is the smallest possible
// representation
let size = discr.value.size(self);
let size = discr_layout.value.size(self);
let discr_val = truncate(discr_val, size);

let discr_dest = self.place_field(dest, discr_index as u64)?;
Expand All @@ -1050,22 +1052,32 @@ where
ref niche_variants,
niche_start,
},
discr: ref discr_layout,
discr_index,
..
} => {
assert!(
variant_index.as_usize() < dest.layout.ty.ty_adt_def().unwrap().variants.len(),
);
if variant_index != dataful_variant {
let niche_dest =
self.place_field(dest, discr_index as u64)?;
let niche_value = variant_index.as_u32() - niche_variants.start().as_u32();
let niche_value = (niche_value as u128)
.wrapping_add(niche_start);
self.write_scalar(
Scalar::from_uint(niche_value, niche_dest.layout.size),
niche_dest
let variants_start = niche_variants.start().as_u32();
let variant_index_relative = variant_index.as_u32()
.checked_sub(variants_start)
.expect("overflow computing relative variant idx");
// We need to use machine arithmetic when taking into account `niche_start`:
// discr_val = variant_index_relative + niche_start_val
let discr_layout = self.layout_of(discr_layout.value.to_int_ty(*self.tcx))?;
let niche_start_val = ImmTy::from_uint(niche_start, discr_layout);
let variant_index_relative_val =
ImmTy::from_uint(variant_index_relative, discr_layout);
let discr_val = self.binary_op(
mir::BinOp::Add,
variant_index_relative_val,
niche_start_val,
)?;
// Write result.
let niche_dest = self.place_field(dest, discr_index as u64)?;
self.write_immediate(*discr_val, niche_dest)?;
}
}
}
Expand Down
21 changes: 18 additions & 3 deletions src/librustc_typeck/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// If there are no arms, that is a diverging match; a special case.
if arms.is_empty() {
self.diverges.set(self.diverges.get() | Diverges::Always);
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
return tcx.types.never;
}

Expand All @@ -69,7 +69,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// warnings).
match all_pats_diverge {
Diverges::Maybe => Diverges::Maybe,
Diverges::Always | Diverges::WarnedAlways => Diverges::WarnedAlways,
Diverges::Always { .. } | Diverges::WarnedAlways => Diverges::WarnedAlways,
}
}).collect();

Expand Down Expand Up @@ -167,6 +167,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
prior_arm_ty = Some(arm_ty);
}

// If all of the arms in the `match` diverge,
// and we're dealing with an actual `match` block
// (as opposed to a `match` desugared from something else'),
// we can emit a better note. Rather than pointing
// at a diverging expression in an arbitrary arm,
// we can point at the entire `match` expression
if let (Diverges::Always { .. }, hir::MatchSource::Normal) = (all_arms_diverge, match_src) {
all_arms_diverge = Diverges::Always {
span: expr.span,
custom_note: Some(
"any code following this `match` expression is unreachable, as all arms diverge"
)
};
}

// We won't diverge unless the discriminant or all arms diverge.
self.diverges.set(discrim_diverges | all_arms_diverge);

Expand All @@ -176,7 +191,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// When the previously checked expression (the scrutinee) diverges,
/// warn the user about the match arms being unreachable.
fn warn_arms_when_scrutinee_diverges(&self, arms: &'tcx [hir::Arm], source: hir::MatchSource) {
if self.diverges.get().always() {
if self.diverges.get().is_always() {
use hir::MatchSource::*;
let msg = match source {
IfDesugar { .. } | IfLetDesugar { .. } => "block in `if` expression",
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// Any expression that produces a value of type `!` must have diverged
if ty.is_never() {
self.diverges.set(self.diverges.get() | Diverges::Always);
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
}

// Record the type, which applies it effects.
Expand Down
55 changes: 45 additions & 10 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,20 @@ pub enum Diverges {

/// Definitely known to diverge and therefore
/// not reach the next sibling or its parent.
Always,
Always {
/// The `Span` points to the expression
/// that caused us to diverge
/// (e.g. `return`, `break`, etc).
span: Span,
/// In some cases (e.g. a `match` expression
/// where all arms diverge), we may be
/// able to provide a more informative
/// message to the user.
/// If this is `None`, a default messsage
/// will be generated, which is suitable
/// for most cases.
custom_note: Option<&'static str>
},

/// Same as `Always` but with a reachability
/// warning already emitted.
Expand Down Expand Up @@ -486,8 +499,22 @@ impl ops::BitOrAssign for Diverges {
}

impl Diverges {
fn always(self) -> bool {
self >= Diverges::Always
/// Creates a `Diverges::Always` with the provided `span` and the default note message.
fn always(span: Span) -> Diverges {
Diverges::Always {
span,
custom_note: None
}
}

fn is_always(self) -> bool {
// Enum comparison ignores the
// contents of fields, so we just
// fill them in with garbage here.
self >= Diverges::Always {
span: DUMMY_SP,
custom_note: None
}
}
}

Expand Down Expand Up @@ -2307,17 +2334,25 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Produces warning on the given node, if the current point in the
/// function is unreachable, and there hasn't been another warning.
fn warn_if_unreachable(&self, id: hir::HirId, span: Span, kind: &str) {
if self.diverges.get() == Diverges::Always &&
// FIXME: Combine these two 'if' expressions into one once
// let chains are implemented
if let Diverges::Always { span: orig_span, custom_note } = self.diverges.get() {
// If span arose from a desugaring of `if` or `while`, then it is the condition itself,
// which diverges, that we are about to lint on. This gives suboptimal diagnostics.
// Instead, stop here so that the `if`- or `while`-expression's block is linted instead.
!span.is_desugaring(DesugaringKind::CondTemporary) {
self.diverges.set(Diverges::WarnedAlways);
if !span.is_desugaring(DesugaringKind::CondTemporary) {
self.diverges.set(Diverges::WarnedAlways);

debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind);
debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind);

let msg = format!("unreachable {}", kind);
self.tcx().lint_hir(lint::builtin::UNREACHABLE_CODE, id, span, &msg);
let msg = format!("unreachable {}", kind);
self.tcx().struct_span_lint_hir(lint::builtin::UNREACHABLE_CODE, id, span, &msg)
.span_note(
orig_span,
custom_note.unwrap_or("any code following this expression is unreachable")
)
.emit();
}
}
}

Expand Down Expand Up @@ -3825,7 +3860,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
//
// #41425 -- label the implicit `()` as being the
// "found type" here, rather than the "expected type".
if !self.diverges.get().always() {
if !self.diverges.get().is_always() {
// #50009 -- Do not point at the entire fn block span, point at the return type
// span, as it is the cause of the requirement, and
// `consider_hint_about_removing_semicolon` will point at the last expression
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ impl Stdio {
/// .expect("Failed to spawn child process");
///
/// {
/// let mut stdin = child.stdin.as_mut().expect("Failed to open stdin");
/// let stdin = child.stdin.as_mut().expect("Failed to open stdin");
/// stdin.write_all("Hello, world!".as_bytes()).expect("Failed to write to stdin");
/// }
///
Expand Down
Loading