From ed10a3faae1fd1176b2edf4a61438e0542c103b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 10 Oct 2018 17:30:10 -0700 Subject: [PATCH 01/18] Custom E0277 diagnostic for `Path` --- src/libcore/marker.rs | 1 + src/librustc/traits/error_reporting.rs | 3 +++ src/test/ui/error-codes/E0277.stderr | 2 +- src/test/ui/suggestions/path-by-value.rs | 6 ++++++ src/test/ui/suggestions/path-by-value.stderr | 15 +++++++++++++++ 5 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/suggestions/path-by-value.rs create mode 100644 src/test/ui/suggestions/path-by-value.stderr diff --git a/src/libcore/marker.rs b/src/libcore/marker.rs index 266c6913747f1..662a8ddd96862 100644 --- a/src/libcore/marker.rs +++ b/src/libcore/marker.rs @@ -92,6 +92,7 @@ impl !Send for *mut T { } #[stable(feature = "rust1", since = "1.0.0")] #[lang = "sized"] #[rustc_on_unimplemented( + on(parent_trait="std::path::Path", label="borrow the `Path` instead"), message="the size for values of type `{Self}` cannot be known at compilation time", label="doesn't have a size known at compile-time", note="to learn more, visit InferCtxt<'a, 'gcx, 'tcx> { flags.push(("from_method".to_owned(), Some(method.to_string()))); } } + if let Some(t) = self.get_parent_trait_ref(&obligation.cause.code) { + flags.push(("parent_trait".to_owned(), Some(t.to_string()))); + } if let Some(k) = obligation.cause.span.compiler_desugaring_kind() { flags.push(("from_desugaring".to_owned(), None)); diff --git a/src/test/ui/error-codes/E0277.stderr b/src/test/ui/error-codes/E0277.stderr index ab9020222ea2b..d0c089fa0f374 100644 --- a/src/test/ui/error-codes/E0277.stderr +++ b/src/test/ui/error-codes/E0277.stderr @@ -2,7 +2,7 @@ error[E0277]: the size for values of type `[u8]` cannot be known at compilation --> $DIR/E0277.rs:23:6 | LL | fn f(p: Path) { } - | ^ doesn't have a size known at compile-time + | ^ borrow the `Path` instead | = help: within `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]` = note: to learn more, visit diff --git a/src/test/ui/suggestions/path-by-value.rs b/src/test/ui/suggestions/path-by-value.rs new file mode 100644 index 0000000000000..c875ca674aeb0 --- /dev/null +++ b/src/test/ui/suggestions/path-by-value.rs @@ -0,0 +1,6 @@ +use std::path::Path; + +fn f(p: Path) { } +//~^ ERROR E0277 + +fn main() {} diff --git a/src/test/ui/suggestions/path-by-value.stderr b/src/test/ui/suggestions/path-by-value.stderr new file mode 100644 index 0000000000000..338cfc990dcda --- /dev/null +++ b/src/test/ui/suggestions/path-by-value.stderr @@ -0,0 +1,15 @@ +error[E0277]: the size for values of type `[u8]` cannot be known at compilation time + --> $DIR/path-by-value.rs:3:6 + | +LL | fn f(p: Path) { } + | ^ borrow the `Path` instead + | + = help: within `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]` + = note: to learn more, visit + = note: required because it appears within the type `std::path::Path` + = note: all local variables must have a statically known size + = help: unsized locals are gated as an unstable feature + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. From e95472bc90a48793add23f27faf5bea51bbb6b14 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 12 Oct 2018 13:47:37 +0200 Subject: [PATCH 02/18] Only suggest paths that exist. In order to output a path that could actually be imported (valid and visible), we need to handle re-exports correctly. For example, take `std::os::unix::process::CommandExt`, this trait is actually defined at `std::sys::unix::ext::process::CommandExt` (at time of writing). `std::os::unix` rexports the contents of `std::sys::unix::ext`. `std::sys` is private so the "true" path to `CommandExt` isn't accessible. In this case, the visible parent map will look something like this: (child) -> (parent) `std::sys::unix::ext::process::CommandExt` -> `std::sys::unix::ext::process` `std::sys::unix::ext::process` -> `std::sys::unix::ext` `std::sys::unix::ext` -> `std::os` This is correct, as the visible parent of `std::sys::unix::ext` is in fact `std::os`. When printing the path to `CommandExt` and looking at the current segment that corresponds to `std::sys::unix::ext`, we would normally print `ext` and then go to the parent - resulting in a mangled path like `std::os::ext::process::CommandExt`. Instead, we must detect that there was a re-export and instead print `unix` (which is the name `std::sys::unix::ext` was re-exported as in `std::os`). --- src/librustc/ty/item_path.rs | 74 +++++++++++++++++++++++---- src/test/ui/issues/issue-39175.rs | 16 ++++++ src/test/ui/issues/issue-39175.stderr | 15 ++++++ 3 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 src/test/ui/issues/issue-39175.rs create mode 100644 src/test/ui/issues/issue-39175.stderr diff --git a/src/librustc/ty/item_path.rs b/src/librustc/ty/item_path.rs index ab0813240364c..74d4ad9851cfb 100644 --- a/src/librustc/ty/item_path.rs +++ b/src/librustc/ty/item_path.rs @@ -10,7 +10,7 @@ use hir::map::DefPathData; use hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; -use ty::{self, Ty, TyCtxt}; +use ty::{self, DefIdTree, Ty, TyCtxt}; use middle::cstore::{ExternCrate, ExternCrateSource}; use syntax::ast; use syntax::symbol::{keywords, LocalInternedString, Symbol}; @@ -219,19 +219,73 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { cur_def_key = self.def_key(parent); } + let visible_parent = visible_parent_map.get(&cur_def).map(|did| *did); + let actual_parent = self.parent(cur_def); + debug!( + "try_push_visible_item_path: visible_parent={:?} actual_parent={:?}", + visible_parent, actual_parent, + ); + let data = cur_def_key.disambiguated_data.data; - let symbol = data.get_opt_name().map(|n| n.as_str()).unwrap_or_else(|| { - if let DefPathData::CrateRoot = data { // reexported `extern crate` (#43189) - self.original_crate_name(cur_def.krate).as_str() - } else { - Symbol::intern("").as_str() - } - }); + let symbol = match data { + // In order to output a path that could actually be imported (valid and visible), + // we need to handle re-exports correctly. + // + // For example, take `std::os::unix::process::CommandExt`, this trait is actually + // defined at `std::sys::unix::ext::process::CommandExt` (at time of writing). + // + // `std::os::unix` rexports the contents of `std::sys::unix::ext`. `std::sys` is + // private so the "true" path to `CommandExt` isn't accessible. + // + // In this case, the `visible_parent_map` will look something like this: + // + // (child) -> (parent) + // `std::sys::unix::ext::process::CommandExt` -> `std::sys::unix::ext::process` + // `std::sys::unix::ext::process` -> `std::sys::unix::ext` + // `std::sys::unix::ext` -> `std::os` + // + // This is correct, as the visible parent of `std::sys::unix::ext` is in fact + // `std::os`. + // + // When printing the path to `CommandExt` and looking at the `cur_def_key` that + // corresponds to `std::sys::unix::ext`, we would normally print `ext` and then go + // to the parent - resulting in a mangled path like + // `std::os::ext::process::CommandExt`. + // + // Instead, we must detect that there was a re-export and instead print `unix` + // (which is the name `std::sys::unix::ext` was re-exported as in `std::os`). To + // do this, we compare the parent of `std::sys::unix::ext` (`std::sys::unix`) with + // the visible parent (`std::os`). If these do not match, then we iterate over + // the children of the visible parent (as was done when computing + // `visible_parent_map`), looking for the specific child we currently have and then + // have access to the re-exported name. + DefPathData::Module(module_name) if visible_parent != actual_parent => { + let mut name: Option = None; + if let Some(visible_parent) = visible_parent { + for child in self.item_children(visible_parent).iter() { + if child.def.def_id() == cur_def { + name = Some(child.ident); + } + } + } + name.map(|n| n.as_str()).unwrap_or(module_name.as_str()) + }, + _ => { + data.get_opt_name().map(|n| n.as_str()).unwrap_or_else(|| { + // Re-exported `extern crate` (#43189). + if let DefPathData::CrateRoot = data { + self.original_crate_name(cur_def.krate).as_str() + } else { + Symbol::intern("").as_str() + } + }) + }, + }; debug!("try_push_visible_item_path: symbol={:?}", symbol); cur_path.push(symbol); - match visible_parent_map.get(&cur_def) { - Some(&def) => cur_def = def, + match visible_parent { + Some(def) => cur_def = def, None => return false, }; } diff --git a/src/test/ui/issues/issue-39175.rs b/src/test/ui/issues/issue-39175.rs new file mode 100644 index 0000000000000..2245f184d55f5 --- /dev/null +++ b/src/test/ui/issues/issue-39175.rs @@ -0,0 +1,16 @@ +// Copyright 2014 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. + +use std::process::Command; +// use std::os::unix::process::CommandExt; + +fn main() { + Command::new("echo").arg("hello").exec(); +} diff --git a/src/test/ui/issues/issue-39175.stderr b/src/test/ui/issues/issue-39175.stderr new file mode 100644 index 0000000000000..108c969cdadde --- /dev/null +++ b/src/test/ui/issues/issue-39175.stderr @@ -0,0 +1,15 @@ +error[E0599]: no method named `exec` found for type `&mut std::process::Command` in the current scope + --> $DIR/issue-39175.rs:15:39 + | +LL | Command::new("echo").arg("hello").exec(); + | ^^^^ + | + = help: items from traits can only be used if the trait is in scope +help: the following trait is implemented but not in scope, perhaps add a `use` for it: + | +LL | use std::os::unix::process::CommandExt; + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. From 507381ed0367f27e8804a1da33f8565c96a260ce Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 13 Oct 2018 23:38:31 +0200 Subject: [PATCH 03/18] Fix ICE and report a human readable error --- src/librustc/ty/cast.rs | 2 ++ src/librustc_mir/transform/qualify_min_const_fn.rs | 13 ++++++++++--- .../ui/consts/min_const_fn/bad_const_fn_body_ice.rs | 5 +++++ .../min_const_fn/bad_const_fn_body_ice.stderr | 10 ++++++++++ 4 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/consts/min_const_fn/bad_const_fn_body_ice.rs create mode 100644 src/test/ui/consts/min_const_fn/bad_const_fn_body_ice.stderr diff --git a/src/librustc/ty/cast.rs b/src/librustc/ty/cast.rs index c0861abb774de..ab82f28c8bff4 100644 --- a/src/librustc/ty/cast.rs +++ b/src/librustc/ty/cast.rs @@ -58,6 +58,8 @@ pub enum CastKind { } impl<'tcx> CastTy<'tcx> { + /// Returns `Some` for integral/pointer casts. + /// casts like unsizing casts will return `None` pub fn from_ty(t: Ty<'tcx>) -> Option> { match t.sty { ty::Bool => Some(CastTy::Int(IntTy::Bool)), diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index aa559c96ec668..edcae4648b72c 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -148,7 +148,7 @@ fn check_rvalue( Rvalue::Len(place) | Rvalue::Discriminant(place) | Rvalue::Ref(_, _, place) => { check_place(tcx, mir, place, span, PlaceMode::Read) } - Rvalue::Cast(_, operand, cast_ty) => { + Rvalue::Cast(CastKind::Misc, operand, cast_ty) => { use rustc::ty::cast::CastTy; let cast_in = CastTy::from_ty(operand.ty(mir, tcx)).expect("bad input type for cast"); let cast_out = CastTy::from_ty(cast_ty).expect("bad output type for cast"); @@ -163,6 +163,10 @@ fn check_rvalue( _ => check_operand(tcx, mir, operand, span), } } + Rvalue::Cast(_, _, _) => Err(( + span, + "only int casts are allowed in const fn".into(), + )), // binops are fine on integers Rvalue::BinaryOp(_, lhs, rhs) | Rvalue::CheckedBinaryOp(_, lhs, rhs) => { check_operand(tcx, mir, lhs, span)?; @@ -177,8 +181,11 @@ fn check_rvalue( )) } } - // checked by regular const fn checks - Rvalue::NullaryOp(..) => Ok(()), + Rvalue::NullaryOp(NullOp::SizeOf, _) => Ok(()), + Rvalue::NullaryOp(NullOp::Box, _) => Err(( + span, + "heap allocations are not allowed in const fn".into(), + )), Rvalue::UnaryOp(_, operand) => { let ty = operand.ty(mir, tcx); if ty.is_integral() || ty.is_bool() { diff --git a/src/test/ui/consts/min_const_fn/bad_const_fn_body_ice.rs b/src/test/ui/consts/min_const_fn/bad_const_fn_body_ice.rs new file mode 100644 index 0000000000000..3e42cb8c1b074 --- /dev/null +++ b/src/test/ui/consts/min_const_fn/bad_const_fn_body_ice.rs @@ -0,0 +1,5 @@ +const fn foo(a: i32) -> Vec { + vec![1, 2, 3] //~ ERROR heap allocations are not allowed in const fn +} + +fn main() {} diff --git a/src/test/ui/consts/min_const_fn/bad_const_fn_body_ice.stderr b/src/test/ui/consts/min_const_fn/bad_const_fn_body_ice.stderr new file mode 100644 index 0000000000000..f6b704370b6f2 --- /dev/null +++ b/src/test/ui/consts/min_const_fn/bad_const_fn_body_ice.stderr @@ -0,0 +1,10 @@ +error: heap allocations are not allowed in const fn + --> $DIR/bad_const_fn_body_ice.rs:2:5 + | +LL | vec![1, 2, 3] //~ ERROR heap allocations are not allowed in const fn + | ^^^^^^^^^^^^^ + | + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: aborting due to previous error + From 34052047cf7cc30ccc8fbd947db56822d3d71423 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 15 Oct 2018 19:58:21 +0200 Subject: [PATCH 04/18] Explain all casts in detail --- src/librustc_mir/transform/qualify_min_const_fn.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index edcae4648b72c..c9d48f5f6adb2 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -163,9 +163,18 @@ fn check_rvalue( _ => check_operand(tcx, mir, operand, span), } } - Rvalue::Cast(_, _, _) => Err(( + Rvalue::Cast(CastKind::UnsafeFnPointer, _, _) | + Rvalue::Cast(CastKind::ReifyFnPointer, _, _) => Err(( span, - "only int casts are allowed in const fn".into(), + "function pointer casts are not allowed in const fn".into(), + )), + Rvalue::Cast(CastKind::ClosureFnPointer, _, _) => Err(( + span, + "closures are not allowed in const fn".into(), + )), + Rvalue::Cast(CastKind::Unsize, _, _) => Err(( + span, + "unsizing casts are not allowed in const fn".into(), )), // binops are fine on integers Rvalue::BinaryOp(_, lhs, rhs) | Rvalue::CheckedBinaryOp(_, lhs, rhs) => { From 2456f330d508b223d8cfcada8652ad37b647996a Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 15 Oct 2018 20:10:16 +0200 Subject: [PATCH 05/18] Try to trigger some error cases --- .../ui/consts/min_const_fn/cast_errors.rs | 14 ++++++++ .../ui/consts/min_const_fn/cast_errors.stderr | 32 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 src/test/ui/consts/min_const_fn/cast_errors.rs create mode 100644 src/test/ui/consts/min_const_fn/cast_errors.stderr diff --git a/src/test/ui/consts/min_const_fn/cast_errors.rs b/src/test/ui/consts/min_const_fn/cast_errors.rs new file mode 100644 index 0000000000000..36827b1b6b0e2 --- /dev/null +++ b/src/test/ui/consts/min_const_fn/cast_errors.rs @@ -0,0 +1,14 @@ +fn main() {} + +const fn unsize(x: &[u8; 3]) -> &[u8] { x } +//~^ ERROR unsizing casts are not allowed in const fn +const fn closure() -> fn() { || {} } +//~^ ERROR function pointers in const fn are unstable +const fn closure2() { + (|| {}) as fn(); +//~^ ERROR function pointers in const fn are unstable +} +const fn reify(f: fn()) -> unsafe fn() { f } +//~^ ERROR function pointers in const fn are unstable +const fn reify2() { main as unsafe fn(); } +//~^ ERROR function pointers in const fn are unstable \ No newline at end of file diff --git a/src/test/ui/consts/min_const_fn/cast_errors.stderr b/src/test/ui/consts/min_const_fn/cast_errors.stderr new file mode 100644 index 0000000000000..ba980b7aacb6c --- /dev/null +++ b/src/test/ui/consts/min_const_fn/cast_errors.stderr @@ -0,0 +1,32 @@ +error: unsizing casts are not allowed in const fn + --> $DIR/cast_errors.rs:3:41 + | +LL | const fn unsize(x: &[u8; 3]) -> &[u8] { x } + | ^ + +error: function pointers in const fn are unstable + --> $DIR/cast_errors.rs:5:23 + | +LL | const fn closure() -> fn() { || {} } + | ^^^^ + +error: function pointers in const fn are unstable + --> $DIR/cast_errors.rs:8:5 + | +LL | (|| {}) as fn(); + | ^^^^^^^^^^^^^^^ + +error: function pointers in const fn are unstable + --> $DIR/cast_errors.rs:11:28 + | +LL | const fn reify(f: fn()) -> unsafe fn() { f } + | ^^^^^^^^^^^ + +error: function pointers in const fn are unstable + --> $DIR/cast_errors.rs:13:21 + | +LL | const fn reify2() { main as unsafe fn(); } + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors + From 007390c21cc5e5e8705d455d20fba1df13bd7362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20S=CC=B6c=CC=B6h=CC=B6n=CC=B6e=CC=B6i=CC=B6d=CC=B6?= =?UTF-8?q?e=CC=B6r=20Scherer?= Date: Mon, 15 Oct 2018 20:30:11 +0200 Subject: [PATCH 06/18] Add trailing newline to satisfy tidy --- src/test/ui/consts/min_const_fn/cast_errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/consts/min_const_fn/cast_errors.rs b/src/test/ui/consts/min_const_fn/cast_errors.rs index 36827b1b6b0e2..8648cd35387ce 100644 --- a/src/test/ui/consts/min_const_fn/cast_errors.rs +++ b/src/test/ui/consts/min_const_fn/cast_errors.rs @@ -11,4 +11,4 @@ const fn closure2() { const fn reify(f: fn()) -> unsafe fn() { f } //~^ ERROR function pointers in const fn are unstable const fn reify2() { main as unsafe fn(); } -//~^ ERROR function pointers in const fn are unstable \ No newline at end of file +//~^ ERROR function pointers in const fn are unstable From 8180e1b54c1f7457c258701287bcbbf32170a743 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 11 Oct 2018 18:02:00 +0200 Subject: [PATCH 07/18] Check the type of statics and constants for `Sized`ness --- src/librustc_typeck/check/wfcheck.rs | 38 +++++++++++++------ src/test/ui/consts/const-array-oob.stderr | 4 +- .../const-eval/const-eval-overflow-4.rs | 2 +- .../const-eval/const-eval-overflow-4.stderr | 15 +++----- src/test/ui/consts/const-unsized.stderr | 20 ++++------ .../infinite-recursion-const-fn.stderr | 4 +- src/test/ui/issues/issue-24446.rs | 3 +- src/test/ui/issues/issue-24446.stderr | 31 +++------------ src/test/ui/issues/issue-54410.rs | 9 +++++ src/test/ui/issues/issue-54410.stderr | 12 ++++++ ...-bounds-on-structs-and-enums-static.stderr | 9 ++--- src/test/ui/wf/wf-const-type.stderr | 4 +- src/test/ui/wf/wf-static-type.stderr | 4 +- 13 files changed, 79 insertions(+), 76 deletions(-) create mode 100644 src/test/ui/issues/issue-54410.rs create mode 100644 src/test/ui/issues/issue-54410.stderr diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index cc1906d91d4c9..016ea5cba6609 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -118,12 +118,17 @@ pub fn check_item_well_formed<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: Def hir::ItemKind::Fn(..) => { check_item_fn(tcx, item); } - hir::ItemKind::Static(..) => { - check_item_type(tcx, item); + hir::ItemKind::Static(ref ty, ..) => { + check_item_type(tcx, item.id, ty.span); } - hir::ItemKind::Const(..) => { - check_item_type(tcx, item); + hir::ItemKind::Const(ref ty, ..) => { + check_item_type(tcx, item.id, ty.span); } + hir::ItemKind::ForeignMod(ref module) => for it in module.items.iter() { + if let hir::ForeignItemKind::Static(ref ty, ..) = it.node { + check_item_type(tcx, it.id, ty.span); + } + }, hir::ItemKind::Struct(ref struct_def, ref ast_generics) => { check_type_defn(tcx, item, false, |fcx| { vec![fcx.non_enum_variant(struct_def)] @@ -335,14 +340,23 @@ fn check_item_fn<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item: &hir::Item) { }) } -fn check_item_type<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item: &hir::Item) { - debug!("check_item_type: {:?}", item); - - for_item(tcx, item).with_fcx(|fcx, _this| { - let ty = fcx.tcx.type_of(fcx.tcx.hir.local_def_id(item.id)); - let item_ty = fcx.normalize_associated_types_in(item.span, &ty); - - fcx.register_wf_obligation(item_ty, item.span, ObligationCauseCode::MiscObligation); +fn check_item_type<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item_id: ast::NodeId, ty_span: Span) { + debug!("check_item_type: {:?}", item_id); + + for_id(tcx, item_id, ty_span).with_fcx(|fcx, _this| { + let ty = fcx.tcx.type_of(fcx.tcx.hir.local_def_id(item_id)); + let item_ty = fcx.normalize_associated_types_in(ty_span, &ty); + + fcx.register_wf_obligation(item_ty, ty_span, ObligationCauseCode::MiscObligation); + fcx.register_bound( + item_ty, + fcx.tcx.require_lang_item(lang_items::SizedTraitLangItem), + traits::ObligationCause::new( + ty_span, + fcx.body_id, + traits::MiscObligation, + ), + ); vec![] // no implied bounds in a const etc }); diff --git a/src/test/ui/consts/const-array-oob.stderr b/src/test/ui/consts/const-array-oob.stderr index 09e4918bf6eed..19ec9a4aa1745 100644 --- a/src/test/ui/consts/const-array-oob.stderr +++ b/src/test/ui/consts/const-array-oob.stderr @@ -7,10 +7,10 @@ LL | const BLUB: [u32; FOO[4]] = [5, 6]; = note: #[deny(const_err)] on by default error[E0080]: could not evaluate constant expression - --> $DIR/const-array-oob.rs:18:1 + --> $DIR/const-array-oob.rs:18:13 | LL | const BLUB: [u32; FOO[4]] = [5, 6]; - | ^^^^^^^^^^^^^^^^^^------^^^^^^^^^^^ + | ^^^^^^------^ | | | index out of bounds: the len is 3 but the index is 4 diff --git a/src/test/ui/consts/const-eval/const-eval-overflow-4.rs b/src/test/ui/consts/const-eval/const-eval-overflow-4.rs index 9fc31b7c72781..e9fc8ec0c374e 100644 --- a/src/test/ui/consts/const-eval/const-eval-overflow-4.rs +++ b/src/test/ui/consts/const-eval/const-eval-overflow-4.rs @@ -20,9 +20,9 @@ use std::{i8, i16, i32, i64, isize}; use std::{u8, u16, u32, u64, usize}; const A_I8_T - //~^ ERROR could not evaluate constant expression : [u32; (i8::MAX as i8 + 1i8) as usize] //~^ ERROR attempt to add with overflow + //~| ERROR could not evaluate constant expression = [0; (i8::MAX as usize) + 1]; fn main() { diff --git a/src/test/ui/consts/const-eval/const-eval-overflow-4.stderr b/src/test/ui/consts/const-eval/const-eval-overflow-4.stderr index 058c8730d7c9e..b12aa032b6070 100644 --- a/src/test/ui/consts/const-eval/const-eval-overflow-4.stderr +++ b/src/test/ui/consts/const-eval/const-eval-overflow-4.stderr @@ -1,5 +1,5 @@ error: attempt to add with overflow - --> $DIR/const-eval-overflow-4.rs:24:13 + --> $DIR/const-eval-overflow-4.rs:23:13 | LL | : [u32; (i8::MAX as i8 + 1i8) as usize] | ^^^^^^^^^^^^^^^^^^^^^ @@ -7,15 +7,12 @@ LL | : [u32; (i8::MAX as i8 + 1i8) as usize] = note: #[deny(const_err)] on by default error[E0080]: could not evaluate constant expression - --> $DIR/const-eval-overflow-4.rs:22:1 + --> $DIR/const-eval-overflow-4.rs:23:7 | -LL | / const A_I8_T -LL | | //~^ ERROR could not evaluate constant expression -LL | | : [u32; (i8::MAX as i8 + 1i8) as usize] - | | --------------------- attempt to add with overflow -LL | | //~^ ERROR attempt to add with overflow -LL | | = [0; (i8::MAX as usize) + 1]; - | |__________________________________^ +LL | : [u32; (i8::MAX as i8 + 1i8) as usize] + | ^^^^^^---------------------^^^^^^^^^^ + | | + | attempt to add with overflow error: aborting due to 2 previous errors diff --git a/src/test/ui/consts/const-unsized.stderr b/src/test/ui/consts/const-unsized.stderr index 83d23bcba0493..8671e609ac8e5 100644 --- a/src/test/ui/consts/const-unsized.stderr +++ b/src/test/ui/consts/const-unsized.stderr @@ -1,42 +1,38 @@ error[E0277]: the size for values of type `(dyn std::fmt::Debug + std::marker::Sync + 'static)` cannot be known at compilation time - --> $DIR/const-unsized.rs:13:29 + --> $DIR/const-unsized.rs:13:16 | LL | const CONST_0: Debug+Sync = *(&0 as &(Debug+Sync)); - | ^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time + | ^^^^^^^^^^ doesn't have a size known at compile-time | = help: the trait `std::marker::Sized` is not implemented for `(dyn std::fmt::Debug + std::marker::Sync + 'static)` = note: to learn more, visit - = note: constant expressions must have a statically known size error[E0277]: the size for values of type `str` cannot be known at compilation time - --> $DIR/const-unsized.rs:16:24 + --> $DIR/const-unsized.rs:16:18 | LL | const CONST_FOO: str = *"foo"; - | ^^^^^^ doesn't have a size known at compile-time + | ^^^ doesn't have a size known at compile-time | = help: the trait `std::marker::Sized` is not implemented for `str` = note: to learn more, visit - = note: constant expressions must have a statically known size error[E0277]: the size for values of type `(dyn std::fmt::Debug + std::marker::Sync + 'static)` cannot be known at compilation time - --> $DIR/const-unsized.rs:19:31 + --> $DIR/const-unsized.rs:19:18 | LL | static STATIC_1: Debug+Sync = *(&1 as &(Debug+Sync)); - | ^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time + | ^^^^^^^^^^ doesn't have a size known at compile-time | = help: the trait `std::marker::Sized` is not implemented for `(dyn std::fmt::Debug + std::marker::Sync + 'static)` = note: to learn more, visit - = note: constant expressions must have a statically known size error[E0277]: the size for values of type `str` cannot be known at compilation time - --> $DIR/const-unsized.rs:22:26 + --> $DIR/const-unsized.rs:22:20 | LL | static STATIC_BAR: str = *"bar"; - | ^^^^^^ doesn't have a size known at compile-time + | ^^^ doesn't have a size known at compile-time | = help: the trait `std::marker::Sized` is not implemented for `str` = note: to learn more, visit - = note: constant expressions must have a statically known size error: aborting due to 4 previous errors diff --git a/src/test/ui/infinite/infinite-recursion-const-fn.stderr b/src/test/ui/infinite/infinite-recursion-const-fn.stderr index 579cc6bd548f9..c77452d9abe20 100644 --- a/src/test/ui/infinite/infinite-recursion-const-fn.stderr +++ b/src/test/ui/infinite/infinite-recursion-const-fn.stderr @@ -1,5 +1,5 @@ error[E0080]: could not evaluate constant expression - --> $DIR/infinite-recursion-const-fn.rs:15:1 + --> $DIR/infinite-recursion-const-fn.rs:15:12 | LL | const fn a() -> usize { b() } | --- @@ -59,7 +59,7 @@ LL | const fn b() -> usize { a() } | inside call to `a` | inside call to `a` LL | const ARR: [i32; a()] = [5; 6]; //~ ERROR could not evaluate constant expression - | ^^^^^^^^^^^^^^^^^---^^^^^^^^^^^ + | ^^^^^^---^ | | | inside call to `a` diff --git a/src/test/ui/issues/issue-24446.rs b/src/test/ui/issues/issue-24446.rs index a9c7978642d5f..7c44319db0db2 100644 --- a/src/test/ui/issues/issue-24446.rs +++ b/src/test/ui/issues/issue-24446.rs @@ -10,8 +10,7 @@ fn main() { static foo: Fn() -> u32 = || -> u32 { - //~^ ERROR mismatched types - //~| ERROR the size for values of type + //~^ ERROR the size for values of type 0 }; } diff --git a/src/test/ui/issues/issue-24446.stderr b/src/test/ui/issues/issue-24446.stderr index 3c42462743700..d4921443cad76 100644 --- a/src/test/ui/issues/issue-24446.stderr +++ b/src/test/ui/issues/issue-24446.stderr @@ -1,33 +1,12 @@ -error[E0308]: mismatched types - --> $DIR/issue-24446.rs:12:31 - | -LL | static foo: Fn() -> u32 = || -> u32 { - | _______________________________^ -LL | | //~^ ERROR mismatched types -LL | | //~| ERROR the size for values of type -LL | | 0 -LL | | }; - | |_____^ expected trait std::ops::Fn, found closure - | - = note: expected type `(dyn std::ops::Fn() -> u32 + 'static)` - found type `[closure@$DIR/issue-24446.rs:12:31: 16:6]` - error[E0277]: the size for values of type `(dyn std::ops::Fn() -> u32 + 'static)` cannot be known at compilation time - --> $DIR/issue-24446.rs:12:31 + --> $DIR/issue-24446.rs:12:17 | -LL | static foo: Fn() -> u32 = || -> u32 { - | _______________________________^ -LL | | //~^ ERROR mismatched types -LL | | //~| ERROR the size for values of type -LL | | 0 -LL | | }; - | |_____^ doesn't have a size known at compile-time +LL | static foo: Fn() -> u32 = || -> u32 { + | ^^^^^^^^^^^ doesn't have a size known at compile-time | = help: the trait `std::marker::Sized` is not implemented for `(dyn std::ops::Fn() -> u32 + 'static)` = note: to learn more, visit - = note: constant expressions must have a statically known size -error: aborting due to 2 previous errors +error: aborting due to previous error -Some errors occurred: E0277, E0308. -For more information about an error, try `rustc --explain E0277`. +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/issues/issue-54410.rs b/src/test/ui/issues/issue-54410.rs new file mode 100644 index 0000000000000..192b97a167728 --- /dev/null +++ b/src/test/ui/issues/issue-54410.rs @@ -0,0 +1,9 @@ +use std::os::raw::c_char; +extern "C" { + pub static mut symbol: [c_char]; + //~^ ERROR the size for values of type `[i8]` cannot be known at compilation time +} + +fn main() { + println!("{:p}", unsafe { &symbol }); +} diff --git a/src/test/ui/issues/issue-54410.stderr b/src/test/ui/issues/issue-54410.stderr new file mode 100644 index 0000000000000..0c40c6384a470 --- /dev/null +++ b/src/test/ui/issues/issue-54410.stderr @@ -0,0 +1,12 @@ +error[E0277]: the size for values of type `[i8]` cannot be known at compilation time + --> $DIR/issue-54410.rs:3:28 + | +LL | pub static mut symbol: [c_char]; + | ^^^^^^^^ doesn't have a size known at compile-time + | + = help: the trait `std::marker::Sized` is not implemented for `[i8]` + = note: to learn more, visit + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/traits/trait-bounds-on-structs-and-enums-static.stderr b/src/test/ui/traits/trait-bounds-on-structs-and-enums-static.stderr index 28d4257a9fba9..5145ba1881d5b 100644 --- a/src/test/ui/traits/trait-bounds-on-structs-and-enums-static.stderr +++ b/src/test/ui/traits/trait-bounds-on-structs-and-enums-static.stderr @@ -1,11 +1,8 @@ error[E0277]: the trait bound `usize: Trait` is not satisfied - --> $DIR/trait-bounds-on-structs-and-enums-static.rs:19:1 + --> $DIR/trait-bounds-on-structs-and-enums-static.rs:19:11 | -LL | / static X: Foo = Foo { -LL | | //~^ ERROR E0277 -LL | | x: 1, -LL | | }; - | |__^ the trait `Trait` is not implemented for `usize` +LL | static X: Foo = Foo { + | ^^^^^^^^^^ the trait `Trait` is not implemented for `usize` | note: required by `Foo` --> $DIR/trait-bounds-on-structs-and-enums-static.rs:15:1 diff --git a/src/test/ui/wf/wf-const-type.stderr b/src/test/ui/wf/wf-const-type.stderr index 1c07824ef77a2..78e831ef880fc 100644 --- a/src/test/ui/wf/wf-const-type.stderr +++ b/src/test/ui/wf/wf-const-type.stderr @@ -1,8 +1,8 @@ error[E0277]: the trait bound `NotCopy: std::marker::Copy` is not satisfied - --> $DIR/wf-const-type.rs:20:1 + --> $DIR/wf-const-type.rs:20:12 | LL | const FOO: IsCopy> = IsCopy { t: None }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `NotCopy` + | ^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `NotCopy` | = note: required because of the requirements on the impl of `std::marker::Copy` for `std::option::Option` note: required by `IsCopy` diff --git a/src/test/ui/wf/wf-static-type.stderr b/src/test/ui/wf/wf-static-type.stderr index f76444cfe1fef..c9656d8654655 100644 --- a/src/test/ui/wf/wf-static-type.stderr +++ b/src/test/ui/wf/wf-static-type.stderr @@ -1,8 +1,8 @@ error[E0277]: the trait bound `NotCopy: std::marker::Copy` is not satisfied - --> $DIR/wf-static-type.rs:20:1 + --> $DIR/wf-static-type.rs:20:13 | LL | static FOO: IsCopy> = IsCopy { t: None }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `NotCopy` + | ^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `NotCopy` | = note: required because of the requirements on the impl of `std::marker::Copy` for `std::option::Option` note: required by `IsCopy` From 4dcf49121cf4b195929099cedac19a489d403eb4 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 14 Oct 2018 17:45:35 +0200 Subject: [PATCH 08/18] Use platform independent types --- src/test/ui/issues/issue-54410.rs | 3 +-- src/test/ui/issues/issue-54410.stderr | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/test/ui/issues/issue-54410.rs b/src/test/ui/issues/issue-54410.rs index 192b97a167728..e3e8ca985b970 100644 --- a/src/test/ui/issues/issue-54410.rs +++ b/src/test/ui/issues/issue-54410.rs @@ -1,6 +1,5 @@ -use std::os::raw::c_char; extern "C" { - pub static mut symbol: [c_char]; + pub static mut symbol: [i8]; //~^ ERROR the size for values of type `[i8]` cannot be known at compilation time } diff --git a/src/test/ui/issues/issue-54410.stderr b/src/test/ui/issues/issue-54410.stderr index 0c40c6384a470..ae6888f067e68 100644 --- a/src/test/ui/issues/issue-54410.stderr +++ b/src/test/ui/issues/issue-54410.stderr @@ -1,8 +1,8 @@ error[E0277]: the size for values of type `[i8]` cannot be known at compilation time - --> $DIR/issue-54410.rs:3:28 + --> $DIR/issue-54410.rs:2:28 | -LL | pub static mut symbol: [c_char]; - | ^^^^^^^^ doesn't have a size known at compile-time +LL | pub static mut symbol: [i8]; + | ^^^^ doesn't have a size known at compile-time | = help: the trait `std::marker::Sized` is not implemented for `[i8]` = note: to learn more, visit From fbbc73969b60d1ce58fe028daa8924ab56465928 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 15 Oct 2018 20:48:25 +0200 Subject: [PATCH 09/18] Add test for no_core statics --- src/test/ui/static_sized_requirement.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/test/ui/static_sized_requirement.rs diff --git a/src/test/ui/static_sized_requirement.rs b/src/test/ui/static_sized_requirement.rs new file mode 100644 index 0000000000000..0ee0637232ca4 --- /dev/null +++ b/src/test/ui/static_sized_requirement.rs @@ -0,0 +1,12 @@ +// compile-pass + +#![feature(no_core, lang_items)] +#![no_core] +#![crate_type = "lib"] + +#[lang = "sized"] +trait Sized {} + +extern { + pub static A: u32; +} From 10a01c1bcd18c33a817751858ebcc022437917ef Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 16 Oct 2018 17:00:33 +0200 Subject: [PATCH 10/18] Update cargo submodule --- src/tools/cargo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/cargo b/src/tools/cargo index 5dbac98885199..09ce4b519535c 160000 --- a/src/tools/cargo +++ b/src/tools/cargo @@ -1 +1 @@ -Subproject commit 5dbac98885199bbd7c0f189d7405b5523434d1e3 +Subproject commit 09ce4b519535c77d17b818b6dc0b72c6668642c1 From 12d79f7de4a722b84473500751434d4794d1f702 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sun, 14 Oct 2018 12:27:22 -0700 Subject: [PATCH 11/18] rustc: Fix (again) simd vectors by-val in ABI The issue of passing around SIMD types as values between functions has seen [quite a lot] of [discussion], and although we thought [we fixed it][quite a lot] it [wasn't]! This PR is a change to rustc to, again, try to fix this issue. The fundamental problem here remains the same, if a SIMD vector argument is passed by-value in LLVM's function type, then if the caller and callee disagree on target features a miscompile happens. We solve this by never passing SIMD vectors by-value, but LLVM will still thwart us with its argument promotion pass to promote by-ref SIMD arguments to by-val SIMD arguments. This commit is an attempt to thwart LLVM thwarting us. We, just before codegen, will take yet another look at the LLVM module and demote any by-value SIMD arguments we see. This is a very manual attempt by us to ensure the codegen for a module keeps working, and it unfortunately is likely producing suboptimal code, even in release mode. The saving grace for this, in theory, is that if SIMD types are passed by-value across a boundary in release mode it's pretty unlikely to be performance sensitive (as it's already doing a load/store, and otherwise perf-sensitive bits should be inlined). The implementation here is basically a big wad of C++. It was largely copied from LLVM's own argument promotion pass, only doing the reverse. In local testing this... Closes #50154 Closes #52636 Closes #54583 Closes #55059 [quite a lot]: https://github.com/rust-lang/rust/pull/47743 [discussion]: https://github.com/rust-lang/rust/issues/44367 [wasn't]: https://github.com/rust-lang/rust/issues/50154 --- src/librustc_codegen_llvm/back/lto.rs | 12 +- src/librustc_codegen_llvm/back/write.rs | 34 +++- src/librustc_codegen_llvm/llvm/ffi.rs | 2 + src/librustc_llvm/build.rs | 4 +- src/rustllvm/DemoteSimd.cpp | 181 ++++++++++++++++++ .../simd-argument-promotion-thwarted/Makefile | 13 ++ .../simd-argument-promotion-thwarted/t1.rs | 21 ++ .../simd-argument-promotion-thwarted/t2.rs | 14 ++ .../simd-argument-promotion-thwarted/t3.rs | 52 +++++ 9 files changed, 324 insertions(+), 9 deletions(-) create mode 100644 src/rustllvm/DemoteSimd.cpp create mode 100644 src/test/run-make/simd-argument-promotion-thwarted/Makefile create mode 100644 src/test/run-make/simd-argument-promotion-thwarted/t1.rs create mode 100644 src/test/run-make/simd-argument-promotion-thwarted/t2.rs create mode 100644 src/test/run-make/simd-argument-promotion-thwarted/t3.rs diff --git a/src/librustc_codegen_llvm/back/lto.rs b/src/librustc_codegen_llvm/back/lto.rs index 3ac22f4eaef65..f4afe52135876 100644 --- a/src/librustc_codegen_llvm/back/lto.rs +++ b/src/librustc_codegen_llvm/back/lto.rs @@ -80,9 +80,7 @@ impl LtoModuleCodegen { let module = module.take().unwrap(); { let config = cgcx.config(module.kind); - let llmod = module.module_llvm.llmod(); - let tm = &*module.module_llvm.tm; - run_pass_manager(cgcx, tm, llmod, config, false); + run_pass_manager(cgcx, &module, config, false); timeline.record("fat-done"); } Ok(module) @@ -557,8 +555,7 @@ fn thin_lto(cgcx: &CodegenContext, } fn run_pass_manager(cgcx: &CodegenContext, - tm: &llvm::TargetMachine, - llmod: &llvm::Module, + module: &ModuleCodegen, config: &ModuleConfig, thin: bool) { // Now we have one massive module inside of llmod. Time to run the @@ -569,7 +566,8 @@ fn run_pass_manager(cgcx: &CodegenContext, debug!("running the pass manager"); unsafe { let pm = llvm::LLVMCreatePassManager(); - llvm::LLVMRustAddAnalysisPasses(tm, pm, llmod); + let llmod = module.module_llvm.llmod(); + llvm::LLVMRustAddAnalysisPasses(module.module_llvm.tm, pm, llmod); if config.verify_llvm_ir { let pass = llvm::LLVMRustFindAndCreatePass("verify\0".as_ptr() as *const _); @@ -864,7 +862,7 @@ impl ThinModule { // little differently. info!("running thin lto passes over {}", module.name); let config = cgcx.config(module.kind); - run_pass_manager(cgcx, module.module_llvm.tm, llmod, config, true); + run_pass_manager(cgcx, &module, config, true); cgcx.save_temp_bitcode(&module, "thin-lto-after-pm"); timeline.record("thin-done"); } diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 02ef690b94233..f4a7afc1667a3 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -633,7 +633,7 @@ unsafe fn optimize(cgcx: &CodegenContext, None, &format!("llvm module passes [{}]", module_name.unwrap()), || { - llvm::LLVMRunPassManager(mpm, llmod) + llvm::LLVMRunPassManager(mpm, llmod); }); // Deallocate managers that we're now done with @@ -691,6 +691,38 @@ unsafe fn codegen(cgcx: &CodegenContext, create_msvc_imps(cgcx, llcx, llmod); } + // Ok now this one's a super interesting invocations. SIMD in rustc is + // difficult where we want some parts of the program to be able to use + // some SIMD features while other parts of the program don't. The real + // tough part is that we want this to actually work correctly! + // + // We go to great lengths to make sure this works, and one crucial + // aspect is that vector arguments (simd types) are never passed by + // value in the ABI of functions. It turns out, however, that LLVM will + // undo our "clever work" of passing vector types by reference. Its + // argument promotion pass will promote these by-ref arguments to + // by-val. That, however, introduces codegen errors! + // + // The upstream LLVM bug [1] has unfortunatey not really seen a lot of + // activity. The Rust bug [2], however, has seen quite a lot of reports + // of this in the wild. As a result, this is worked around locally here. + // We have a custom transformation, `LLVMRustDemoteSimdArguments`, which + // does the opposite of argument promotion by demoting any by-value SIMD + // arguments in function signatures to pointers intead of being + // by-value. + // + // This operates at the LLVM IR layer because LLVM is thwarting our + // codegen and this is the only chance we get to make sure it's correct + // before we hit codegen. + // + // Hopefully one day the upstream LLVM bug will be fixed and we'll no + // longer need this! + // + // [1]: https://bugs.llvm.org/show_bug.cgi?id=37358 + // [2]: https://github.com/rust-lang/rust/issues/50154 + llvm::LLVMRustDemoteSimdArguments(llmod); + cgcx.save_temp_bitcode(&module, "simd-demoted"); + // A codegen-specific pass manager is used to generate object // files for an LLVM module. // diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index c9f51efdc5095..3286377ad326f 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -1136,6 +1136,8 @@ extern "C" { /// Runs a pass manager on a module. pub fn LLVMRunPassManager(PM: &PassManager<'a>, M: &'a Module) -> Bool; + pub fn LLVMRustDemoteSimdArguments(M: &'a Module); + pub fn LLVMInitializePasses(); pub fn LLVMPassManagerBuilderCreate() -> &'static mut PassManagerBuilder; diff --git a/src/librustc_llvm/build.rs b/src/librustc_llvm/build.rs index 7d01ed556c8dd..ad5db19839ef0 100644 --- a/src/librustc_llvm/build.rs +++ b/src/librustc_llvm/build.rs @@ -162,7 +162,9 @@ fn main() { } build_helper::rerun_if_changed_anything_in_dir(Path::new("../rustllvm")); - cfg.file("../rustllvm/PassWrapper.cpp") + cfg + .file("../rustllvm/DemoteSimd.cpp") + .file("../rustllvm/PassWrapper.cpp") .file("../rustllvm/RustWrapper.cpp") .file("../rustllvm/ArchiveWrapper.cpp") .file("../rustllvm/Linker.cpp") diff --git a/src/rustllvm/DemoteSimd.cpp b/src/rustllvm/DemoteSimd.cpp new file mode 100644 index 0000000000000..3c75376077932 --- /dev/null +++ b/src/rustllvm/DemoteSimd.cpp @@ -0,0 +1,181 @@ +// 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. + +#include +#include + +#include "rustllvm.h" + +#include "llvm/IR/CallSite.h" +#include "llvm/IR/Module.h" +#include "llvm/ADT/STLExtras.h" + +using namespace llvm; + +static std::vector +GetFunctionsWithSimdArgs(Module *M) { + std::vector Ret; + + for (auto &F : M->functions()) { + // Skip all intrinsic calls as these are always tightly controlled to "work + // correctly", so no need to fixup any of these. + if (F.isIntrinsic()) + continue; + + // We're only interested in rustc-defined functions, not unstably-defined + // imported SIMD ffi functions. + if (F.isDeclaration()) + continue; + + // Argument promotion only happens on internal functions, so skip demoting + // arguments in external functions like FFI shims and such. + if (!F.hasLocalLinkage()) + continue; + + // If any argument to this function is a by-value vector type, then that's + // bad! The compiler didn't generate any functions that looked like this, + // and we try to rely on LLVM to not do this! Argument promotion may, + // however, promote arguments from behind references. In any case, figure + // out if we're interested in demoting this argument. + if (any_of(F.args(), [](Argument &arg) { return arg.getType()->isVectorTy(); })) + Ret.push_back(&F); + } + + return Ret; +} + +extern "C" void +LLVMRustDemoteSimdArguments(LLVMModuleRef Mod) { + Module *M = unwrap(Mod); + + auto Functions = GetFunctionsWithSimdArgs(M); + + for (auto F : Functions) { + // Build up our list of new parameters and new argument attributes. + // We're only changing those arguments which are vector types. + SmallVector Params; + SmallVector ArgAttrVec; + auto PAL = F->getAttributes(); + for (auto &Arg : F->args()) { + auto *Ty = Arg.getType(); + if (Ty->isVectorTy()) { + Params.push_back(PointerType::get(Ty, 0)); + ArgAttrVec.push_back(AttributeSet()); + } else { + Params.push_back(Ty); + ArgAttrVec.push_back(PAL.getParamAttributes(Arg.getArgNo())); + } + } + + // Replace `F` with a new function with our new signature. I'm... not really + // sure how this works, but this is all the steps `ArgumentPromotion` does + // to replace a signature as well. + assert(!F->isVarArg()); // ArgumentPromotion should skip these fns + FunctionType *NFTy = FunctionType::get(F->getReturnType(), Params, false); + Function *NF = Function::Create(NFTy, F->getLinkage(), F->getName()); + NF->copyAttributesFrom(F); + NF->setSubprogram(F->getSubprogram()); + F->setSubprogram(nullptr); + NF->setAttributes(AttributeList::get(F->getContext(), + PAL.getFnAttributes(), + PAL.getRetAttributes(), + ArgAttrVec)); + ArgAttrVec.clear(); + F->getParent()->getFunctionList().insert(F->getIterator(), NF); + NF->takeName(F); + + // Iterate over all invocations of `F`, updating all `call` instructions to + // store immediate vector types in a local `alloc` instead of a by-value + // vector. + // + // Like before, much of this is copied from the `ArgumentPromotion` pass in + // LLVM. + SmallVector Args; + while (!F->use_empty()) { + CallSite CS(F->user_back()); + assert(CS.getCalledFunction() == F); + Instruction *Call = CS.getInstruction(); + const AttributeList &CallPAL = CS.getAttributes(); + + // Loop over the operands, inserting an `alloca` and a store for any + // argument we're demoting to be by reference + // + // FIXME: we probably want to figure out an LLVM pass to run and clean up + // this function and instructions we're generating, we should in theory + // only generate a maximum number of `alloca` instructions rather than + // one-per-variable unconditionally. + CallSite::arg_iterator AI = CS.arg_begin(); + size_t ArgNo = 0; + for (Function::arg_iterator I = F->arg_begin(), E = F->arg_end(); I != E; + ++I, ++AI, ++ArgNo) { + if (I->getType()->isVectorTy()) { + AllocaInst *AllocA = new AllocaInst(I->getType(), 0, nullptr, "", Call); + new StoreInst(*AI, AllocA, Call); + Args.push_back(AllocA); + ArgAttrVec.push_back(AttributeSet()); + } else { + Args.push_back(*AI); + ArgAttrVec.push_back(CallPAL.getParamAttributes(ArgNo)); + } + } + assert(AI == CS.arg_end()); + + // Create a new call instructions which we'll use to replace the old call + // instruction, copying over as many attributes and such as possible. + SmallVector OpBundles; + CS.getOperandBundlesAsDefs(OpBundles); + + CallSite NewCS; + if (InvokeInst *II = dyn_cast(Call)) { + InvokeInst::Create(NF, II->getNormalDest(), II->getUnwindDest(), + Args, OpBundles, "", Call); + } else { + auto *NewCall = CallInst::Create(NF, Args, OpBundles, "", Call); + NewCall->setTailCallKind(cast(Call)->getTailCallKind()); + NewCS = NewCall; + } + NewCS.setCallingConv(CS.getCallingConv()); + NewCS.setAttributes( + AttributeList::get(F->getContext(), CallPAL.getFnAttributes(), + CallPAL.getRetAttributes(), ArgAttrVec)); + NewCS->setDebugLoc(Call->getDebugLoc()); + Args.clear(); + ArgAttrVec.clear(); + Call->replaceAllUsesWith(NewCS.getInstruction()); + NewCS->takeName(Call); + Call->eraseFromParent(); + } + + // Splice the body of the old function right into the new function. + NF->getBasicBlockList().splice(NF->begin(), F->getBasicBlockList()); + + // Update our new function to replace all uses of the by-value argument with + // loads of the pointer argument we've generated. + // + // FIXME: we probably want to only generate one load instruction per + // function? Or maybe run an LLVM pass to clean up this function? + for (Function::arg_iterator I = F->arg_begin(), + E = F->arg_end(), + I2 = NF->arg_begin(); + I != E; + ++I, ++I2) { + if (I->getType()->isVectorTy()) { + I->replaceAllUsesWith(new LoadInst(&*I2, "", &NF->begin()->front())); + } else { + I->replaceAllUsesWith(&*I2); + } + I2->takeName(&*I); + } + + // Delete all references to the old function, it should be entirely dead + // now. + M->getFunctionList().remove(F); + } +} diff --git a/src/test/run-make/simd-argument-promotion-thwarted/Makefile b/src/test/run-make/simd-argument-promotion-thwarted/Makefile new file mode 100644 index 0000000000000..3095432d0fe69 --- /dev/null +++ b/src/test/run-make/simd-argument-promotion-thwarted/Makefile @@ -0,0 +1,13 @@ +-include ../../run-make-fulldeps/tools.mk + +ifeq ($(TARGET),x86_64-unknown-linux-gnu) +all: + $(RUSTC) t1.rs -C opt-level=3 + $(TMPDIR)/t1 + $(RUSTC) t2.rs -C opt-level=3 + $(TMPDIR)/t2 + $(RUSTC) t3.rs -C opt-level=3 + $(TMPDIR)/t3 +else +all: +endif diff --git a/src/test/run-make/simd-argument-promotion-thwarted/t1.rs b/src/test/run-make/simd-argument-promotion-thwarted/t1.rs new file mode 100644 index 0000000000000..cb4a3dd7d4a7c --- /dev/null +++ b/src/test/run-make/simd-argument-promotion-thwarted/t1.rs @@ -0,0 +1,21 @@ +use std::arch::x86_64; + +fn main() { + if !is_x86_feature_detected!("avx2") { + return println!("AVX2 is not supported on this machine/build."); + } + let load_bytes: [u8; 32] = [0x0f; 32]; + let lb_ptr = load_bytes.as_ptr(); + let reg_load = unsafe { + x86_64::_mm256_loadu_si256( + lb_ptr as *const x86_64::__m256i + ) + }; + println!("{:?}", reg_load); + let mut store_bytes: [u8; 32] = [0; 32]; + let sb_ptr = store_bytes.as_mut_ptr(); + unsafe { + x86_64::_mm256_storeu_si256(sb_ptr as *mut x86_64::__m256i, reg_load); + } + assert_eq!(load_bytes, store_bytes); +} diff --git a/src/test/run-make/simd-argument-promotion-thwarted/t2.rs b/src/test/run-make/simd-argument-promotion-thwarted/t2.rs new file mode 100644 index 0000000000000..0e42b82a223d0 --- /dev/null +++ b/src/test/run-make/simd-argument-promotion-thwarted/t2.rs @@ -0,0 +1,14 @@ +use std::arch::x86_64::*; + +fn main() { + if !is_x86_feature_detected!("avx") { + return println!("AVX is not supported on this machine/build."); + } + unsafe { + let f = _mm256_set_pd(2.0, 2.0, 2.0, 2.0); + let r = _mm256_mul_pd(f, f); + + union A { a: __m256d, b: [f64; 4] } + assert_eq!(A { a: r }.b, [4.0, 4.0, 4.0, 4.0]); + } +} diff --git a/src/test/run-make/simd-argument-promotion-thwarted/t3.rs b/src/test/run-make/simd-argument-promotion-thwarted/t3.rs new file mode 100644 index 0000000000000..10062ab3e4643 --- /dev/null +++ b/src/test/run-make/simd-argument-promotion-thwarted/t3.rs @@ -0,0 +1,52 @@ +use std::arch::x86_64::*; + +#[target_feature(enable = "avx")] +unsafe fn avx_mul(a: __m256, b: __m256) -> __m256 { + _mm256_mul_ps(a, b) +} + +#[target_feature(enable = "avx")] +unsafe fn avx_store(p: *mut f32, a: __m256) { + _mm256_storeu_ps(p, a) +} + +#[target_feature(enable = "avx")] +unsafe fn avx_setr(a: f32, b: f32, c: f32, d: f32, e: f32, f: f32, g: f32, h: f32) -> __m256 { + _mm256_setr_ps(a, b, c, d, e, f, g, h) +} + +#[target_feature(enable = "avx")] +unsafe fn avx_set1(a: f32) -> __m256 { + _mm256_set1_ps(a) +} + +struct Avx(__m256); + +fn mul(a: Avx, b: Avx) -> Avx { + unsafe { Avx(avx_mul(a.0, b.0)) } +} + +fn set1(a: f32) -> Avx { + unsafe { Avx(avx_set1(a)) } +} + +fn setr(a: f32, b: f32, c: f32, d: f32, e: f32, f: f32, g: f32, h: f32) -> Avx { + unsafe { Avx(avx_setr(a, b, c, d, e, f, g, h)) } +} + +unsafe fn store(p: *mut f32, a: Avx) { + avx_store(p, a.0); +} + +fn main() { + if !is_x86_feature_detected!("avx") { + return println!("AVX is not supported on this machine/build."); + } + let mut result = [0.0f32; 8]; + let a = mul(setr(0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0), set1(0.25)); + unsafe { + store(result.as_mut_ptr(), a); + } + + assert_eq!(result, [0.0, 0.25, 0.5, 0.75, 1.0, 1.25, 1.50, 1.75]); +} From 38f3ad41c0bbf6ed7c389a070afb60a465b3afe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20S=CC=B6c=CC=B6h=CC=B6n=CC=B6e=CC=B6i=CC=B6d=CC=B6?= =?UTF-8?q?e=CC=B6r=20Scherer?= Date: Wed, 17 Oct 2018 09:04:10 +0200 Subject: [PATCH 12/18] Squash closure cast error into fn ptr cast error --- src/librustc_mir/transform/qualify_min_const_fn.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index c9d48f5f6adb2..52c557b83d591 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -164,14 +164,11 @@ fn check_rvalue( } } Rvalue::Cast(CastKind::UnsafeFnPointer, _, _) | + Rvalue::Cast(CastKind::ClosureFnPointer, _, _) | Rvalue::Cast(CastKind::ReifyFnPointer, _, _) => Err(( span, "function pointer casts are not allowed in const fn".into(), )), - Rvalue::Cast(CastKind::ClosureFnPointer, _, _) => Err(( - span, - "closures are not allowed in const fn".into(), - )), Rvalue::Cast(CastKind::Unsize, _, _) => Err(( span, "unsizing casts are not allowed in const fn".into(), From da40916bc20880785daec954ee20ee18464ecb7a Mon Sep 17 00:00:00 2001 From: ljedrz Date: Wed, 17 Oct 2018 11:13:44 +0200 Subject: [PATCH 13/18] resolve: improve common patterns --- src/librustc_resolve/build_reduced_graph.rs | 11 +- src/librustc_resolve/check_unused.rs | 6 +- src/librustc_resolve/lib.rs | 63 ++++---- src/librustc_resolve/resolve_imports.rs | 152 ++++++++++---------- 4 files changed, 108 insertions(+), 124 deletions(-) diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 25a7ff9cd3f56..e2f5829d14ff7 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -139,7 +139,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { let prefix_iter = || parent_prefix.iter().cloned() .chain(use_tree.prefix.segments.iter().map(|seg| seg.ident)); - let prefix_start = prefix_iter().nth(0); + let prefix_start = prefix_iter().next(); let starts_with_non_keyword = prefix_start.map_or(false, |ident| { !ident.is_path_segment_keyword() }); @@ -1048,13 +1048,10 @@ impl<'a, 'b, 'cl> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b, 'cl> { fn visit_token(&mut self, t: Token) { if let Token::Interpolated(nt) = t { - match nt.0 { - token::NtExpr(ref expr) => { - if let ast::ExprKind::Mac(..) = expr.node { - self.visit_invoc(expr.id); - } + if let token::NtExpr(ref expr) = nt.0 { + if let ast::ExprKind::Mac(..) = expr.node { + self.visit_invoc(expr.id); } - _ => {} } } } diff --git a/src/librustc_resolve/check_unused.rs b/src/librustc_resolve/check_unused.rs index de9481579e2f4..6f3135b37cf05 100644 --- a/src/librustc_resolve/check_unused.rs +++ b/src/librustc_resolve/check_unused.rs @@ -109,7 +109,7 @@ impl<'a, 'b, 'cl> Visitor<'a> for UnusedImportCheckVisitor<'a, 'b, 'cl> { self.item_span }; - if items.len() == 0 { + if items.is_empty() { self.unused_imports .entry(self.base_id) .or_default() @@ -170,7 +170,7 @@ pub fn check_crate(resolver: &mut Resolver, krate: &ast::Crate) { for (id, spans) in &visitor.unused_imports { let len = spans.len(); - let mut spans = spans.values().map(|s| *s).collect::>(); + let mut spans = spans.values().cloned().collect::>(); spans.sort(); let ms = MultiSpan::from_spans(spans.clone()); let mut span_snippets = spans.iter() @@ -183,7 +183,7 @@ pub fn check_crate(resolver: &mut Resolver, krate: &ast::Crate) { span_snippets.sort(); let msg = format!("unused import{}{}", if len > 1 { "s" } else { "" }, - if span_snippets.len() > 0 { + if !span_snippets.is_empty() { format!(": {}", span_snippets.join(", ")) } else { String::new() diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 86fe584dc3a40..5f821cc71c957 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1633,19 +1633,17 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> { *def = module.def().unwrap(), PathResult::NonModule(path_res) if path_res.unresolved_segments() == 0 => *def = path_res.base_def(), - PathResult::NonModule(..) => match self.resolve_path( - None, - &path, - None, - true, - span, - CrateLint::No, - ) { - PathResult::Failed(span, msg, _) => { + PathResult::NonModule(..) => + if let PathResult::Failed(span, msg, _) = self.resolve_path( + None, + &path, + None, + true, + span, + CrateLint::No, + ) { error_callback(self, span, ResolutionError::FailedToResolve(&msg)); - } - _ => {} - }, + }, PathResult::Module(ModuleOrUniformRoot::UniformRoot(_)) | PathResult::Indeterminate => unreachable!(), PathResult::Failed(span, msg, _) => { @@ -2351,7 +2349,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { span: prefix.span.to(use_tree.prefix.span), }; - if items.len() == 0 { + if items.is_empty() { // Resolve prefix of an import with empty braces (issue #28388). self.smart_resolve_path_with_crate_lint( id, @@ -2690,7 +2688,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { let map_j = self.binding_mode_map(&q); for (&key, &binding_i) in &map_i { - if map_j.len() == 0 { // Account for missing bindings when + if map_j.is_empty() { // Account for missing bindings when let binding_error = missing_vars // map_j has none. .entry(key.name) .or_insert(BindingError { @@ -2751,9 +2749,8 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { // This has to happen *after* we determine which pat_idents are variants self.check_consistent_bindings(&arm.pats); - match arm.guard { - Some(ast::Guard::If(ref expr)) => self.visit_expr(expr), - _ => {} + if let Some(ast::Guard::If(ref expr)) = arm.guard { + self.visit_expr(expr) } self.visit_expr(&arm.body); @@ -2994,14 +2991,14 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { // Make the base error. let expected = source.descr_expected(); let path_str = names_to_string(path); - let item_str = path[path.len() - 1]; + let item_str = path.last().unwrap(); let code = source.error_code(def.is_some()); let (base_msg, fallback_label, base_span) = if let Some(def) = def { (format!("expected {}, found {} `{}`", expected, def.kind_name(), path_str), format!("not a {}", expected), span) } else { - let item_span = path[path.len() - 1].span; + let item_span = path.last().unwrap().span; let (mod_prefix, mod_str) = if path.len() == 1 { (String::new(), "this scope".to_string()) } else if path.len() == 2 && path[0].name == keywords::CrateRoot.name() { @@ -3368,7 +3365,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { ); } break; - } else if snippet.trim().len() != 0 { + } else if !snippet.trim().is_empty() { debug!("tried to find type ascription `:` token, couldn't find it"); break; } @@ -3930,7 +3927,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { } _ => {} } - return def; + def } fn lookup_assoc_candidate(&mut self, @@ -4482,21 +4479,17 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { let extern_prelude_names = self.extern_prelude.clone(); for &name in extern_prelude_names.iter() { let ident = Ident::with_empty_ctxt(name); - match self.crate_loader.maybe_process_path_extern(name, ident.span) { - Some(crate_id) => { - let crate_root = self.get_module(DefId { - krate: crate_id, - index: CRATE_DEF_INDEX, - }); - self.populate_module_if_necessary(&crate_root); + if let Some(crate_id) = self.crate_loader.maybe_process_path_extern(name, + ident.span) + { + let crate_root = self.get_module(DefId { + krate: crate_id, + index: CRATE_DEF_INDEX, + }); + self.populate_module_if_necessary(&crate_root); - suggestions.extend( - self.lookup_import_candidates_from_module( - lookup_name, namespace, crate_root, ident, &filter_fn - ) - ); - } - None => {} + suggestions.extend(self.lookup_import_candidates_from_module( + lookup_name, namespace, crate_root, ident, &filter_fn)); } } } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 48f312ce9f27d..d5d772e135914 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -672,7 +672,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { }; let has_explicit_self = - import.module_path.len() > 0 && + !import.module_path.is_empty() && import.module_path[0].name == keywords::SelfValue.name(); self.per_ns(|_, ns| { @@ -703,9 +703,8 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { if let SingleImport { source, ref result, .. } = import.subclass { if source.name == "self" { // Silence `unresolved import` error if E0429 is already emitted - match result.value_ns.get() { - Err(Determined) => continue, - _ => {}, + if let Err(Determined) = result.value_ns.get() { + continue; } } } @@ -822,20 +821,19 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { fn throw_unresolved_import_error(&self, error_vec: Vec<(Span, String, String)>, span: Option) { let max_span_label_msg_count = 10; // upper limit on number of span_label message. - let (span,msg) = match error_vec.is_empty() { - true => (span.unwrap(), "unresolved import".to_string()), - false => { - let span = MultiSpan::from_spans(error_vec.clone().into_iter() - .map(|elem: (Span, String, String)| { elem.0 } - ).collect()); - let path_vec: Vec = error_vec.clone().into_iter() - .map(|elem: (Span, String, String)| { format!("`{}`", elem.1) } - ).collect(); - let path = path_vec.join(", "); - let msg = format!("unresolved import{} {}", - if path_vec.len() > 1 { "s" } else { "" }, path); - (span, msg) - } + let (span, msg) = if error_vec.is_empty() { + (span.unwrap(), "unresolved import".to_string()) + } else { + let span = MultiSpan::from_spans(error_vec.clone().into_iter() + .map(|elem: (Span, String, String)| { elem.0 }) + .collect()); + let path_vec: Vec = error_vec.clone().into_iter() + .map(|elem: (Span, String, String)| { format!("`{}`", elem.1) }) + .collect(); + let path = path_vec.join(", "); + let msg = format!("unresolved import{} {}", + if path_vec.len() > 1 { "s" } else { "" }, path); + (span, msg) }; let mut err = struct_span_err!(self.resolver.session, span, E0432, "{}", &msg); for span_error in error_vec.into_iter().take(max_span_label_msg_count) { @@ -1026,9 +1024,8 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { if all_ns_err { let mut all_ns_failed = true; self.per_ns(|this, ns| if !type_ns_only || ns == TypeNS { - match this.resolve_ident_in_module(module, ident, ns, record_used, span) { - Ok(_) => all_ns_failed = false, - _ => {} + if this.resolve_ident_in_module(module, ident, ns, record_used, span).is_ok() { + all_ns_failed = false; } }); @@ -1247,65 +1244,62 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> { } } - match binding.kind { - NameBindingKind::Import { binding: orig_binding, directive, .. } => { - if ns == TypeNS && orig_binding.is_variant() && - !orig_binding.vis.is_at_least(binding.vis, &*self) { - let msg = match directive.subclass { - ImportDirectiveSubclass::SingleImport { .. } => { - format!("variant `{}` is private and cannot be re-exported", - ident) - }, - ImportDirectiveSubclass::GlobImport { .. } => { - let msg = "enum is private and its variants \ - cannot be re-exported".to_owned(); - let error_id = (DiagnosticMessageId::ErrorId(0), // no code?! - Some(binding.span), - msg.clone()); - let fresh = self.session.one_time_diagnostics - .borrow_mut().insert(error_id); - if !fresh { - continue; - } - msg - }, - ref s @ _ => bug!("unexpected import subclass {:?}", s) - }; - let mut err = self.session.struct_span_err(binding.span, &msg); - - let imported_module = match directive.imported_module.get() { - Some(ModuleOrUniformRoot::Module(module)) => module, - _ => bug!("module should exist"), - }; - let resolutions = imported_module.parent.expect("parent should exist") - .resolutions.borrow(); - let enum_path_segment_index = directive.module_path.len() - 1; - let enum_ident = directive.module_path[enum_path_segment_index]; - - let enum_resolution = resolutions.get(&(enum_ident, TypeNS)) - .expect("resolution should exist"); - let enum_span = enum_resolution.borrow() - .binding.expect("binding should exist") - .span; - let enum_def_span = self.session.source_map().def_span(enum_span); - let enum_def_snippet = self.session.source_map() - .span_to_snippet(enum_def_span).expect("snippet should exist"); - // potentially need to strip extant `crate`/`pub(path)` for suggestion - let after_vis_index = enum_def_snippet.find("enum") - .expect("`enum` keyword should exist in snippet"); - let suggestion = format!("pub {}", - &enum_def_snippet[after_vis_index..]); - - self.session - .diag_span_suggestion_once(&mut err, - DiagnosticMessageId::ErrorId(0), - enum_def_span, - "consider making the enum public", - suggestion); - err.emit(); - } + if let NameBindingKind::Import { binding: orig_binding, directive, .. } = binding.kind { + if ns == TypeNS && orig_binding.is_variant() && + !orig_binding.vis.is_at_least(binding.vis, &*self) { + let msg = match directive.subclass { + ImportDirectiveSubclass::SingleImport { .. } => { + format!("variant `{}` is private and cannot be re-exported", + ident) + }, + ImportDirectiveSubclass::GlobImport { .. } => { + let msg = "enum is private and its variants \ + cannot be re-exported".to_owned(); + let error_id = (DiagnosticMessageId::ErrorId(0), // no code?! + Some(binding.span), + msg.clone()); + let fresh = self.session.one_time_diagnostics + .borrow_mut().insert(error_id); + if !fresh { + continue; + } + msg + }, + ref s @ _ => bug!("unexpected import subclass {:?}", s) + }; + let mut err = self.session.struct_span_err(binding.span, &msg); + + let imported_module = match directive.imported_module.get() { + Some(ModuleOrUniformRoot::Module(module)) => module, + _ => bug!("module should exist"), + }; + let resolutions = imported_module.parent.expect("parent should exist") + .resolutions.borrow(); + let enum_path_segment_index = directive.module_path.len() - 1; + let enum_ident = directive.module_path[enum_path_segment_index]; + + let enum_resolution = resolutions.get(&(enum_ident, TypeNS)) + .expect("resolution should exist"); + let enum_span = enum_resolution.borrow() + .binding.expect("binding should exist") + .span; + let enum_def_span = self.session.source_map().def_span(enum_span); + let enum_def_snippet = self.session.source_map() + .span_to_snippet(enum_def_span).expect("snippet should exist"); + // potentially need to strip extant `crate`/`pub(path)` for suggestion + let after_vis_index = enum_def_snippet.find("enum") + .expect("`enum` keyword should exist in snippet"); + let suggestion = format!("pub {}", + &enum_def_snippet[after_vis_index..]); + + self.session + .diag_span_suggestion_once(&mut err, + DiagnosticMessageId::ErrorId(0), + enum_def_span, + "consider making the enum public", + suggestion); + err.emit(); } - _ => {} } } From 89c20b78d68345040d68f0b475276687d10fdc92 Mon Sep 17 00:00:00 2001 From: ljedrz Date: Wed, 17 Oct 2018 11:36:19 +0200 Subject: [PATCH 14/18] resolve: improve/remove allocations --- src/librustc_resolve/lib.rs | 20 +++++--------------- src/librustc_resolve/macros.rs | 7 +++---- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 5f821cc71c957..131b69429c623 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3021,10 +3021,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { let mut err = this.session.struct_span_err_with_code(base_span, &base_msg, code); // Emit help message for fake-self from other languages like `this`(javascript) - let fake_self: Vec = ["this", "my"].iter().map( - |s| Ident::from_str(*s) - ).collect(); - if fake_self.contains(&item_str) + if ["this", "my"].contains(&&*item_str.as_str()) && this.self_value_is_available(path[0].span, span) { err.span_suggestion_with_applicability( span, @@ -4377,10 +4374,9 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { where FilterFn: Fn(Def) -> bool { let mut candidates = Vec::new(); - let mut worklist = Vec::new(); let mut seen_modules = FxHashSet(); let not_local_module = crate_name != keywords::Crate.ident(); - worklist.push((start_module, Vec::::new(), not_local_module)); + let mut worklist = vec![(start_module, Vec::::new(), not_local_module)]; while let Some((in_module, path_segments, @@ -4467,13 +4463,8 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { -> Vec where FilterFn: Fn(Def) -> bool { - let mut suggestions = vec![]; - - suggestions.extend( - self.lookup_import_candidates_from_module( - lookup_name, namespace, self.graph_root, keywords::Crate.ident(), &filter_fn - ) - ); + let mut suggestions = self.lookup_import_candidates_from_module( + lookup_name, namespace, self.graph_root, keywords::Crate.ident(), &filter_fn); if self.session.rust_2018() { let extern_prelude_names = self.extern_prelude.clone(); @@ -4502,9 +4493,8 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { -> Option<(Module<'a>, ImportSuggestion)> { let mut result = None; - let mut worklist = Vec::new(); let mut seen_modules = FxHashSet(); - worklist.push((self.graph_root, Vec::new())); + let mut worklist = vec![(self.graph_root, Vec::new())]; while let Some((in_module, path_segments)) = worklist.pop() { // abort if the module is already found diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 6c57e6c88abeb..28284a45bcdd5 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -203,9 +203,7 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> { self.current_module = invocation.module.get(); self.current_module.unresolved_invocations.borrow_mut().remove(&mark); self.current_module.unresolved_invocations.borrow_mut().extend(derives); - for &derive in derives { - self.invocations.insert(derive, invocation); - } + self.invocations.extend(derives.iter().map(|&derive| (derive, invocation))); let mut visitor = BuildReducedGraphVisitor { resolver: self, current_legacy_scope: invocation.parent_legacy_scope.get(), @@ -277,11 +275,12 @@ impl<'a, 'crateloader: 'a> base::Resolver for Resolver<'a, 'crateloader> { if traits.is_empty() { attrs.remove(i); } else { - let mut tokens = Vec::new(); + let mut tokens = Vec::with_capacity(traits.len() - 1); for (j, path) in traits.iter().enumerate() { if j > 0 { tokens.push(TokenTree::Token(attrs[i].span, Token::Comma).into()); } + tokens.reserve((path.segments.len() * 2).saturating_sub(1)); for (k, segment) in path.segments.iter().enumerate() { if k > 0 { tokens.push(TokenTree::Token(path.span, Token::ModSep).into()); From 0a858dc859f828af2c578bcf23418dd14c767f3c Mon Sep 17 00:00:00 2001 From: varkor Date: Wed, 17 Oct 2018 23:51:01 +0100 Subject: [PATCH 15/18] Don't warn about parentheses on `match (return)` --- src/librustc_lint/unused.rs | 13 ++++++++----- src/test/ui/lint/no-unused-parens-return-block.rs | 6 ++++++ 2 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/lint/no-unused-parens-return-block.rs diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 76717548521b7..4cf2072e792ca 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -276,10 +276,13 @@ impl UnusedParens { cx: &EarlyContext, value: &ast::Expr, msg: &str, - struct_lit_needs_parens: bool) { + followed_by_block: bool) { if let ast::ExprKind::Paren(ref inner) = value.node { - let necessary = struct_lit_needs_parens && - parser::contains_exterior_struct_lit(&inner); + let necessary = followed_by_block && if let ast::ExprKind::Ret(_) = inner.node { + true + } else { + parser::contains_exterior_struct_lit(&inner) + }; if !necessary { let pattern = pprust::expr_to_string(value); Self::remove_outer_parens(cx, value.span, &pattern, msg); @@ -343,7 +346,7 @@ impl LintPass for UnusedParens { impl EarlyLintPass for UnusedParens { fn check_expr(&mut self, cx: &EarlyContext, e: &ast::Expr) { use syntax::ast::ExprKind::*; - let (value, msg, struct_lit_needs_parens) = match e.node { + let (value, msg, followed_by_block) = match e.node { If(ref cond, ..) => (cond, "`if` condition", true), While(ref cond, ..) => (cond, "`while` condition", true), IfLet(_, ref cond, ..) => (cond, "`if let` head expression", true), @@ -380,7 +383,7 @@ impl EarlyLintPass for UnusedParens { return; } }; - self.check_unused_parens_expr(cx, &value, msg, struct_lit_needs_parens); + self.check_unused_parens_expr(cx, &value, msg, followed_by_block); } fn check_pat(&mut self, cx: &EarlyContext, p: &ast::Pat) { diff --git a/src/test/ui/lint/no-unused-parens-return-block.rs b/src/test/ui/lint/no-unused-parens-return-block.rs new file mode 100644 index 0000000000000..fc8dbb743e1f3 --- /dev/null +++ b/src/test/ui/lint/no-unused-parens-return-block.rs @@ -0,0 +1,6 @@ +// run-pass + +fn main() { + match (return) {} // ok + if (return) {} // ok +} From 40bba70823466eb1458a4fd99a4f644411b4eae4 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 18 Oct 2018 10:35:01 +0100 Subject: [PATCH 16/18] Make warnings into errors --- src/test/ui/lint/no-unused-parens-return-block.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/ui/lint/no-unused-parens-return-block.rs b/src/test/ui/lint/no-unused-parens-return-block.rs index fc8dbb743e1f3..37dc519a20401 100644 --- a/src/test/ui/lint/no-unused-parens-return-block.rs +++ b/src/test/ui/lint/no-unused-parens-return-block.rs @@ -1,5 +1,8 @@ // run-pass +#![deny(unused_parens)] +#![allow(unreachable_code)] + fn main() { match (return) {} // ok if (return) {} // ok From 518a5a4898799e4b6732ac217f85def83fc462aa Mon Sep 17 00:00:00 2001 From: Aaron Power Date: Mon, 17 Sep 2018 17:44:04 +0100 Subject: [PATCH 17/18] Updated RELEASES.md for 1.30.0 --- RELEASES.md | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/RELEASES.md b/RELEASES.md index 9bc750ddef5db..b40897a7509fa 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,3 +1,124 @@ +Version 1.30.0 (2018-10-25) +========================== + +Language +-------- +- [Procedural macros are now available.][52081] These kinds of macros allow for + more powerful code generation, there is a [new chapter available][proc-macros] + in Rust Programming Language book that goes further in depth. +- [You can now use keywords as identifiers using the raw identifiers + syntax (`r#`).][53236] e.g. `let r#bool = true;` +- [Using anonymous parameters in traits is now deprecated with a warning and + will be a hard error in the 2018 edition.][53272] +- [You can now use `crate` in paths.][54404] This allows you to refer to the + crate root in the path. e.g. `use crate::foo;` refers to `foo` in `src/lib.rs`. +- [Using a external crate now no longer requires being prefixed with `::`.][54404] + e.g. previously using a external crate in a module without a use statement + required `let json = ::serde_json::from_str(foo);` can now be written + as `let json = serde_json::from_str(foo);`. +- [You can now apply the `#[used]` attribute to static items to prevent the + compiler from optimising them away even if they appear to be unused.][51363] + e.g. `#[used] static FOO: u32 = 1;` +- [You can now import and reexport macros from other crates with the `use` + syntax.][50911] Macros exported with `#[macro_export]` are now placed into + the root module of the crate. If your macro relies on calling other local + macros it is recommended to export with the + `#[macro_export(local_inner_macros)]` attribute so that users won't have to + import those macros. +- [`mod.rs` files are now optional.][54146] Previously if you had a `foo` module + with a `bar` submodule, you would have `src/foo/mod.rs` and `src/foo/bar.rs`. + Now you can have `src/foo.rs` and `src/foo/bar.rs` to achieve the same effect. +- [You can now catch visibility keywords (e.g. `pub`, `pub(crate)`) in macros + using the `vis` specifier.][53370] +- [Non-macro attributes now allow all forms of literals not just + strings.][53044] e.g. Previously you would write `#[attr("true")]` you can now + write `#[attr(true)]`. +- [You can now specify a function to handle a panic in the Rust runtime with the + `#[panic_handler]` attribute.][51366] + +Compiler +-------- +- [Added the `riscv32imc-unknown-none-elf` target.][53822] +- [Added the `aarch64-unknown-netbsd` target][53165] + +Libraries +--------- +- [`ManuallyDrop` now allows the inner type to be unsized.][53033] + +Stabilized APIs +--------------- +- [`Ipv4Addr::BROADCAST`] +- [`Ipv4Addr::LOCALHOST`] +- [`Ipv4Addr::UNSPECIFIED`] +- [`Ipv6Addr::LOCALHOST`] +- [`Ipv6Addr::UNSPECIFIED`] +- [`Iterator::find_map`] + + The following methods are a replacement methods for `trim_left`, `trim_right`, + `trim_left_matches`, and `trim_right_matches`. Which will be deprecated + in 1.33.0. +- [`str::trim_end_matches`] +- [`str::trim_end`] +- [`str::trim_start_matches`] +- [`str::trim_start`] + +Cargo +---- +- [`cargo run` doesn't require specifying a package in workspaces.][cargo/5877] +- [`cargo doc` now supports `--message-format=json`.][cargo/5878] This is + equivalent to calling `rustdoc --error-format=json`. +- [You can specify which edition to create a project in cargo + with `cargo new --edition`.][cargo/5984] Currently only `2015` is a + valid option. +- [Cargo will now provide a progress bar for builds.][cargo/5995] + +Misc +---- +- [`rustdoc` allows you to specify what edition to treat your code as with the + `--edition` option.][54057] +- [`rustdoc` now has the `--color` (Specify whether to output color) and + `--error-format` (Specify error format e.g. `json`) options.][53003] +- [We now distribute a `rust-gdbgui` script that invokes `gdbgui` with Rust + debug symbols.][53774] +- [Attributes from Rust tools such as `rustfmt` or `clippy` are now + available.][53459] e.g. `#[rustfmt::skip]` will skip formatting the next item. + +[50911]: https://github.com/rust-lang/rust/pull/50911/ +[51363]: https://github.com/rust-lang/rust/pull/51363/ +[51366]: https://github.com/rust-lang/rust/pull/51366/ +[52081]: https://github.com/rust-lang/rust/pull/52081/ +[53003]: https://github.com/rust-lang/rust/pull/53003/ +[53033]: https://github.com/rust-lang/rust/pull/53033/ +[53044]: https://github.com/rust-lang/rust/pull/53044/ +[53165]: https://github.com/rust-lang/rust/pull/53165/ +[53213]: https://github.com/rust-lang/rust/pull/53213/ +[53236]: https://github.com/rust-lang/rust/pull/53236/ +[53272]: https://github.com/rust-lang/rust/pull/53272/ +[53370]: https://github.com/rust-lang/rust/pull/53370/ +[53459]: https://github.com/rust-lang/rust/pull/53459/ +[53774]: https://github.com/rust-lang/rust/pull/53774/ +[53822]: https://github.com/rust-lang/rust/pull/53822/ +[54057]: https://github.com/rust-lang/rust/pull/54057/ +[54146]: https://github.com/rust-lang/rust/pull/54146/ +[54404]: https://github.com/rust-lang/rust/pull/54404/ +[cargo/5877]: https://github.com/rust-lang/cargo/pull/5877/ +[cargo/5878]: https://github.com/rust-lang/cargo/pull/5878/ +[cargo/5984]: https://github.com/rust-lang/cargo/pull/5984/ +[cargo/5995]: https://github.com/rust-lang/cargo/pull/5995/ +[proc-macros]: https://doc.rust-lang.org/book/2018-edition/ch19-06-macros.html + +[`Ipv4Addr::BROADCAST`]: https://doc.rust-lang.org/nightly/std/net/struct.Ipv4Addr.html#associatedconstant.BROADCAST +[`Ipv4Addr::LOCALHOST`]: https://doc.rust-lang.org/nightly/std/net/struct.Ipv4Addr.html#associatedconstant.LOCALHOST +[`Ipv4Addr::UNSPECIFIED`]: https://doc.rust-lang.org/nightly/std/net/struct.Ipv4Addr.html#associatedconstant.UNSPECIFIED +[`Ipv6Addr::LOCALHOST`]: https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#associatedconstant.LOCALHOST +[`Ipv6Addr::UNSPECIFIED`]: https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#associatedconstant.UNSPECIFIED +[`Iterator::find_map`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.find_map +[`str::trim_end_matches`]: https://doc.rust-lang.org/nightly/std/primitive.str.html#method.trim_end_matches +[`str::trim_end`]: https://doc.rust-lang.org/nightly/std/primitive.str.html#method.trim_end +[`str::trim_start_matches`]: https://doc.rust-lang.org/nightly/std/primitive.str.html#method.trim_start_matches +[`str::trim_start`]: https://doc.rust-lang.org/nightly/std/primitive.str.html#method.trim_start + + Version 1.29.2 (2018-10-11) =========================== @@ -6,6 +127,7 @@ Version 1.29.2 (2018-10-11) [54639]: https://github.com/rust-lang/rust/pull/54639 + Version 1.29.1 (2018-09-25) =========================== @@ -19,6 +141,7 @@ Security Notes Thank you to Scott McMurray for responsibily disclosing this vulnerability to us. + Version 1.29.0 (2018-09-13) ========================== @@ -73,7 +196,10 @@ Compatibility Notes Consider using the `home_dir` function from https://crates.io/crates/dirs instead. - [`rustc` will no longer silently ignore invalid data in target spec.][52330] +- [`cfg` attributes and `--cfg` command line flags are now more + strictly validated.][53893] +[53893]: https://github.com/rust-lang/rust/pull/53893/ [52861]: https://github.com/rust-lang/rust/pull/52861/ [52656]: https://github.com/rust-lang/rust/pull/52656/ [52239]: https://github.com/rust-lang/rust/pull/52239/ From 7403d55f06f2837cb5d2b0ba7ee790f52e044ce6 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 18 Oct 2018 14:17:01 +0200 Subject: [PATCH 18/18] Make OpTy field op public for priroda --- src/librustc_mir/interpret/operand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 2d6b19ca4a7fc..83b5831f3ad67 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -291,7 +291,7 @@ impl Operand { #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub struct OpTy<'tcx, Tag=()> { - crate op: Operand, // ideally we'd make this private, but const_prop needs this + pub op: Operand, // This is used by [priroda](https://github.com/oli-obk/priroda) pub layout: TyLayout<'tcx>, }