user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Auto merge of #97014 - Mark-Simulacrum:beta-next, r=Mark-Simulacrum

Browse source 
[beta] backports

This backports/rolls up:

*  Quick fix for #96223. #96679
*  [beta] Revert #92519 on beta #96556
*  [beta] Clippy backport ICE/infinite loop fix #96740
*  Revert "Prefer projection candidates instead of param_env candidates for Sized predicates" #96593
This commit is contained in:
bors 2022-05-14 00:35:33 +00:00
commit eb1e588e1d
14 changed files with 205 additions and 96 deletions
Download patch file Download diff file Expand all files Collapse all files

8
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file

@ -713,7 +713,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
return false;
}
let orig_ty = old_pred.self_ty().skip_binder();
// This is a quick fix to resolve an ICE (#96223).
// This change should probably be deeper.
// As suggested by @jackh726, `mk_trait_obligation_with_new_self_ty` could take a `Binder<(TraitRef, Ty)>
// instead of `Binder<Ty>` leading to some changes to its call places.
let Some(orig_ty) = old_pred.self_ty().no_bound_vars() else {
return false;
};
let mk_result = |new_ty| {
let obligation =
self.mk_trait_obligation_with_new_self_ty(param_env, old_pred, new_ty);

4
compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs
View file

@ -175,9 +175,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let needs_infer = stack.obligation.predicate.has_infer_types_or_consts();
let sized_predicate = self.tcx().lang_items().sized_trait()
== Some(stack.obligation.predicate.skip_binder().def_id());
// If there are STILL multiple candidates, we can further
// reduce the list by dropping duplicates -- including
// resolving specializations.
@ -186,7 +183,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
while i < candidates.len() {
let is_dup = (0..candidates.len()).filter(|&j| i != j).any(|j| {
self.candidate_should_be_dropped_in_favor_of(
sized_predicate,
&candidates[i],
&candidates[j],
needs_infer,

20
compiler/rustc_trait_selection/src/traits/select/mod.rs
View file

@ -1553,7 +1553,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
/// See the comment for "SelectionCandidate" for more details.
fn candidate_should_be_dropped_in_favor_of(
&mut self,
sized_predicate: bool,
victim: &EvaluatedCandidate<'tcx>,
other: &EvaluatedCandidate<'tcx>,
needs_infer: bool,
@ -1625,16 +1624,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// Drop otherwise equivalent non-const fn pointer candidates
(FnPointerCandidate { .. }, FnPointerCandidate { is_const: false }) => true,
// If obligation is a sized predicate or the where-clause bound is
// global, prefer the projection or object candidate. See issue
// #50825 and #89352.
(ObjectCandidate(_) | ProjectionCandidate(_), ParamCandidate(ref cand)) => {
sized_predicate || is_global(cand)
}
(ParamCandidate(ref cand), ObjectCandidate(_) | ProjectionCandidate(_)) => {
!(sized_predicate || is_global(cand))
}
// Global bounds from the where clause should be ignored
// here (see issue #50825). Otherwise, we have a where
// clause so don't go around looking for impls.
@ -1650,8 +1639,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
| BuiltinUnsizeCandidate
| TraitUpcastingUnsizeCandidate(_)
| BuiltinCandidate { .. }
| TraitAliasCandidate(..),
| TraitAliasCandidate(..)
| ObjectCandidate(_)
| ProjectionCandidate(_),
) => !is_global(cand),
(ObjectCandidate(_) | ProjectionCandidate(_), ParamCandidate(ref cand)) => {
// Prefer these to a global where-clause bound
// (see issue #50825).
is_global(cand)
}
(
ImplCandidate(_)
| ClosureCandidate

67
library/std/src/sys/windows/process.rs
View file

@ -19,12 +19,12 @@ use crate::path::{Path, PathBuf};
use crate::ptr;
use crate::sys::c;
use crate::sys::c::NonZeroDWORD;
use crate::sys::cvt;
use crate::sys::fs::{File, OpenOptions};
use crate::sys::handle::Handle;
use crate::sys::path;
use crate::sys::pipe::{self, AnonPipe};
use crate::sys::stdio;
use crate::sys::{cvt, to_u16s};
use crate::sys_common::mutex::StaticMutex;
use crate::sys_common::process::{CommandEnv, CommandEnvs};
use crate::sys_common::{AsInner, IntoInner};
@ -269,13 +269,8 @@ impl Command {
None
};
let program = resolve_exe(&self.program, || env::var_os("PATH"), child_paths)?;
// Case insensitive "ends_with" of UTF-16 encoded ".bat" or ".cmd"
let is_batch_file = matches!(
program.len().checked_sub(5).and_then(|i| program.get(i..)),
Some([46, 98 | 66, 97 | 65, 116 | 84, 0] | [46, 99 | 67, 109 | 77, 100 | 68, 0])
);
let mut cmd_str =
make_command_line(&program, &self.args, self.force_quotes_enabled, is_batch_file)?;
make_command_line(program.as_os_str(), &self.args, self.force_quotes_enabled)?;
cmd_str.push(0); // add null terminator
// stolen from the libuv code.
@ -314,6 +309,7 @@ impl Command {
si.hStdOutput = stdout.as_raw_handle();
si.hStdError = stderr.as_raw_handle();
let program = to_u16s(&program)?;
unsafe {
cvt(c::CreateProcessW(
program.as_ptr(),
@ -370,7 +366,7 @@ fn resolve_exe<'a>(
exe_path: &'a OsStr,
parent_paths: impl FnOnce() -> Option<OsString>,
child_paths: Option<&OsStr>,
) -> io::Result<Vec<u16>> {
) -> io::Result<PathBuf> {
// Early return if there is no filename.
if exe_path.is_empty() || path::has_trailing_slash(exe_path) {
return Err(io::const_io_error!(
@ -392,19 +388,19 @@ fn resolve_exe<'a>(
if has_exe_suffix {
// The application name is a path to a `.exe` file.
// Let `CreateProcessW` figure out if it exists or not.
return path::maybe_verbatim(Path::new(exe_path));
return Ok(exe_path.into());
}
let mut path = PathBuf::from(exe_path);
// Append `.exe` if not already there.
path = path::append_suffix(path, EXE_SUFFIX.as_ref());
if let Some(path) = program_exists(&path) {
if program_exists(&path) {
return Ok(path);
} else {
// It's ok to use `set_extension` here because the intent is to
// remove the extension that was just added.
path.set_extension("");
return path::maybe_verbatim(&path);
return Ok(path);
}
} else {
ensure_no_nuls(exe_path)?;
@ -419,7 +415,7 @@ fn resolve_exe<'a>(
if !has_extension {
path.set_extension(EXE_EXTENSION);
}
program_exists(&path)
if program_exists(&path) { Some(path) } else { None }
});
if let Some(path) = result {
return Ok(path);
@ -435,10 +431,10 @@ fn search_paths<Paths, Exists>(
parent_paths: Paths,
child_paths: Option<&OsStr>,
mut exists: Exists,
) -> Option<Vec<u16>>
) -> Option<PathBuf>
where
Paths: FnOnce() -> Option<OsString>,
Exists: FnMut(PathBuf) -> Option<Vec<u16>>,
Exists: FnMut(PathBuf) -> Option<PathBuf>,
{
// 1. Child paths
// This is for consistency with Rust's historic behaviour.
@ -490,18 +486,17 @@ where
}
/// Check if a file exists without following symlinks.
fn program_exists(path: &Path) -> Option<Vec<u16>> {
fn program_exists(path: &Path) -> bool {
unsafe {
let path = path::maybe_verbatim(path).ok()?;
// Getting attributes using `GetFileAttributesW` does not follow symlinks
// and it will almost always be successful if the link exists.
// There are some exceptions for special system files (e.g. the pagefile)
// but these are not executable.
if c::GetFileAttributesW(path.as_ptr()) == c::INVALID_FILE_ATTRIBUTES {
None
} else {
Some(path)
}
to_u16s(path)
.map(|path| {
// Getting attributes using `GetFileAttributesW` does not follow symlinks
// and it will almost always be successful if the link exists.
// There are some exceptions for special system files (e.g. the pagefile)
// but these are not executable.
c::GetFileAttributesW(path.as_ptr()) != c::INVALID_FILE_ATTRIBUTES
})
.unwrap_or(false)
}
}
@ -735,12 +730,7 @@ enum Quote {
// Produces a wide string *without terminating null*; returns an error if
// `prog` or any of the `args` contain a nul.
fn make_command_line(
prog: &[u16],
args: &[Arg],
force_quotes: bool,
is_batch_file: bool,
) -> io::Result<Vec<u16>> {
fn make_command_line(prog: &OsStr, args: &[Arg], force_quotes: bool) -> io::Result<Vec<u16>> {
// Encode the command and arguments in a command line string such
// that the spawned process may recover them using CommandLineToArgvW.
let mut cmd: Vec<u16> = Vec::new();
@ -749,18 +739,17 @@ fn make_command_line(
// need to add an extra pair of quotes surrounding the whole command line
// so they are properly passed on to the script.
// See issue #91991.
let is_batch_file = Path::new(prog)
.extension()
.map(|ext| ext.eq_ignore_ascii_case("cmd") || ext.eq_ignore_ascii_case("bat"))
.unwrap_or(false);
if is_batch_file {
cmd.push(b'"' as u16);
}
// Always quote the program name so CreateProcess to avoid ambiguity when
// the child process parses its arguments.
// Note that quotes aren't escaped here because they can't be used in arg0.
// But that's ok because file paths can't contain quotes.
cmd.push(b'"' as u16);
cmd.extend_from_slice(prog.strip_suffix(&[0]).unwrap_or(prog));
cmd.push(b'"' as u16);
// Always quote the program name so CreateProcess doesn't interpret args as
// part of the name if the binary wasn't found first time.
append_arg(&mut cmd, prog, Quote::Always)?;
for arg in args {
cmd.push(' ' as u16);
let (arg, quote) = match arg {

7
library/std/src/sys/windows/process/tests.rs
View file

@ -3,12 +3,11 @@ use super::Arg;
use crate::env;
use crate::ffi::{OsStr, OsString};
use crate::process::Command;
use crate::sys::to_u16s;
#[test]
fn test_raw_args() {
let command_line = &make_command_line(
&to_u16s("quoted exe").unwrap(),
OsStr::new("quoted exe"),
&[
Arg::Regular(OsString::from("quote me")),
Arg::Raw(OsString::from("quote me *not*")),
@ -17,7 +16,6 @@ fn test_raw_args() {
Arg::Regular(OsString::from("optional-quotes")),
],
false,
false,
)
.unwrap();
assert_eq!(
@ -30,10 +28,9 @@ fn test_raw_args() {
fn test_make_command_line() {
fn test_wrapper(prog: &str, args: &[&str], force_quotes: bool) -> String {
let command_line = &make_command_line(
&to_u16s(prog).unwrap(),
OsStr::new(prog),
&args.iter().map(|a| Arg::Regular(OsString::from(a))).collect::<Vec<_>>(),
force_quotes,
false,
)
.unwrap();
String::from_utf16(command_line).unwrap()

22
src/test/ui/generic-associated-types/bugs/issue-89352.base.stderr Normal file
View file

@ -0,0 +1,22 @@
error[E0308]: mismatched types
--> $DIR/issue-89352.rs:36:13
|
LL | let a = A::reborrow::<'ai, 's>(self.a.clone());
| ^ lifetime mismatch
|
= note: expected type `<<A as GenAssoc<T>>::Iter<'s> as Sized>`
found type `<<A as GenAssoc<T>>::Iter<'ai> as Sized>`
note: the lifetime `'s` as defined here...
--> $DIR/issue-89352.rs:35:13
|
LL | fn iter<'s>(&'s self) -> Self::Iter<'s> {
| ^^
note: ...does not necessarily outlive the lifetime `'ai` as defined here
--> $DIR/issue-89352.rs:30:6
|
LL | impl<'ai, T: 'ai, A: GenAssoc<T>> GenAssoc<T> for Wrapper<'ai, T, A>
| ^^^
error: aborting due to previous error
For more information about this error, try `rustc --explain E0308`.

14
src/test/ui/generic-associated-types/issue-89352.rs → src/test/ui/generic-associated-types/bugs/issue-89352.rs
View file

@ -1,4 +1,16 @@
// check-pass
// revisions: base nll
// ignore-compare-mode-nll
//[nll] compile-flags: -Z borrowck=mir
//[base] check-fail
//[nll] check-pass
// known-bug
// This should pass, but we end up with `A::Iter<'ai>: Sized` for some specific
// `'ai`. We also know that `for<'at> A::Iter<'at>: Sized` from the definition,
// but we prefer param env candidates. We changed this to preference in #92191,
// but this led to unintended consequences (#93262). Suprisingly, this passes
// under NLL. So only a bug in migrate mode.
#![feature(generic_associated_types)]

21
src/test/ui/generic-associated-types/issue-93262.rs Normal file
View file

@ -0,0 +1,21 @@
// check-pass
#![feature(generic_associated_types)]
pub trait Trait {
type Assoc<'a> where Self: 'a;
}
pub trait Foo<T: Trait>
where
for<'a> T::Assoc<'a>: Clone
{}
pub struct Type;
impl<T: Trait> Foo<T> for Type
where
for<'a> T::Assoc<'a>: Clone
{}
fn main() {}

52
src/test/ui/suggestions/issue-96223.rs Normal file
View file

@ -0,0 +1,52 @@
// Previously ICEd because we didn't properly track binders in suggestions
// check-fail
pub trait Foo<'de>: Sized {}
pub trait Bar<'a>: 'static {
type Inner: 'a;
}
pub trait Fubar {
type Bar: for<'a> Bar<'a>;
}
pub struct Baz<T>(pub T);
impl<'de, T> Foo<'de> for Baz<T> where T: Foo<'de> {}
struct Empty;
impl<M> Dummy<M> for Empty
where
M: Fubar,
for<'de> Baz<<M::Bar as Bar<'de>>::Inner>: Foo<'de>,
{
}
pub trait Dummy<M>
where
M: Fubar,
{
}
pub struct EmptyBis<'a>(&'a [u8]);
impl<'a> Bar<'a> for EmptyBis<'static> {
type Inner = EmptyBis<'a>;
}
pub struct EmptyMarker;
impl Fubar for EmptyMarker {
type Bar = EmptyBis<'static>;
}
fn icey_bounds<D: Dummy<EmptyMarker>>(p: &D) {}
fn trigger_ice() {
let p = Empty;
icey_bounds(&p); //~ERROR the trait bound
}
fn main() {}

27
src/test/ui/suggestions/issue-96223.stderr Normal file
View file

@ -0,0 +1,27 @@
error[E0277]: the trait bound `for<'de> EmptyBis<'de>: Foo<'_>` is not satisfied
--> $DIR/issue-96223.rs:49:17
|
LL | icey_bounds(&p);
| ----------- ^^ the trait `for<'de> Foo<'_>` is not implemented for `EmptyBis<'de>`
| |
| required by a bound introduced by this call
|
note: required because of the requirements on the impl of `for<'de> Foo<'de>` for `Baz<EmptyBis<'de>>`
--> $DIR/issue-96223.rs:16:14
|
LL | impl<'de, T> Foo<'de> for Baz<T> where T: Foo<'de> {}
| ^^^^^^^^ ^^^^^^
note: required because of the requirements on the impl of `Dummy<EmptyMarker>` for `Empty`
--> $DIR/issue-96223.rs:20:9
|
LL | impl<M> Dummy<M> for Empty
| ^^^^^^^^ ^^^^^
note: required by a bound in `icey_bounds`
--> $DIR/issue-96223.rs:45:19
|
LL | fn icey_bounds<D: Dummy<EmptyMarker>>(p: &D) {}
| ^^^^^^^^^^^^^^^^^^ required by this bound in `icey_bounds`
error: aborting due to previous error
For more information about this error, try `rustc --explain E0277`.

1
src/tools/clippy/clippy_lints/src/lib.register_all.rs
View file

@ -237,7 +237,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
LintId::of(octal_escapes::OCTAL_ESCAPES),
LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),

1
src/tools/clippy/clippy_lints/src/lib.register_complexity.rs
View file

@ -66,7 +66,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
LintId::of(no_effect::NO_EFFECT),
LintId::of(no_effect::UNNECESSARY_OPERATION),
LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
LintId::of(precedence::PRECEDENCE),

1
src/tools/clippy/clippy_lints/src/lib.register_nursery.rs
View file

@ -20,6 +20,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
LintId::of(mutex_atomic::MUTEX_INTEGER),
LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY),
LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
LintId::of(option_if_let_else::OPTION_IF_LET_ELSE),
LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
LintId::of(redundant_pub_crate::REDUNDANT_PUB_CRATE),

56
src/tools/clippy/clippy_lints/src/only_used_in_recursion.rs
View file

@ -1,6 +1,7 @@
use std::collections::VecDeque;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_lint_allowed;
use itertools::{izip, Itertools};
use rustc_ast::{walk_list, Label, Mutability};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
@ -8,7 +9,7 @@ use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData};
use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
use rustc_hir::intravisit::{walk_expr, walk_stmt, FnKind, Visitor};
use rustc_hir::{
Arm, Block, Body, Expr, ExprKind, Guard, HirId, ImplicitSelfKind, Let, Local, Pat, PatKind, Path, PathSegment,
QPath, Stmt, StmtKind, TyKind, UnOp,
@ -33,6 +34,9 @@ declare_clippy_lint! {
/// and the assigned variables are also only in recursion, it is useless.
///
/// ### Known problems
/// Too many code paths in the linting code are currently untested and prone to produce false
/// positives or are prone to have performance implications.
///
/// In some cases, this would not catch all useless arguments.
///
/// ```rust
@ -85,7 +89,7 @@ declare_clippy_lint! {
/// ```
#[clippy::version = "1.60.0"]
pub ONLY_USED_IN_RECURSION,
complexity,
nursery,
"arguments that is only used in recursion can be removed"
}
declare_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION]);
@ -100,6 +104,9 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
_: Span,
id: HirId,
) {
if is_lint_allowed(cx, ONLY_USED_IN_RECURSION, id) {
return;
}
if let FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) = kind {
let def_id = id.owner.to_def_id();
let data = cx.tcx.def_path(def_id).data;
@ -145,7 +152,8 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
is_method: matches!(kind, FnKind::Method(..)),
has_self,
ty_res,
ty_ctx: cx.tcx,
tcx: cx.tcx,
visited_exprs: FxHashSet::default(),
};
visitor.visit_expr(&body.value);
@ -206,19 +214,13 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
}
pub fn is_primitive(ty: Ty<'_>) -> bool {
match ty.kind() {
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str => true,
ty::Ref(_, t, _) => is_primitive(*t),
_ => false,
}
let ty = ty.peel_refs();
ty.is_primitive() || ty.is_str()
}
pub fn is_array(ty: Ty<'_>) -> bool {
match ty.kind() {
ty::Array(..) | ty::Slice(..) => true,
ty::Ref(_, t, _) => is_array(*t),
_ => false,
}
let ty = ty.peel_refs();
ty.is_array() || ty.is_array_slice()
}
/// This builds the graph of side effect.
@ -250,40 +252,30 @@ pub struct SideEffectVisit<'tcx> {
is_method: bool,
has_self: bool,
ty_res: &'tcx TypeckResults<'tcx>,
ty_ctx: TyCtxt<'tcx>,
tcx: TyCtxt<'tcx>,
visited_exprs: FxHashSet<HirId>,
}
impl<'tcx> Visitor<'tcx> for SideEffectVisit<'tcx> {
fn visit_block(&mut self, b: &'tcx Block<'tcx>) {
b.stmts.iter().for_each(|stmt| {
self.visit_stmt(stmt);
self.ret_vars.clear();
});
walk_list!(self, visit_expr, b.expr);
}
fn visit_stmt(&mut self, s: &'tcx Stmt<'tcx>) {
match s.kind {
StmtKind::Local(Local {
pat, init: Some(init), ..
}) => {
self.visit_pat_expr(pat, init, false);
self.ret_vars.clear();
},
StmtKind::Item(i) => {
let item = self.ty_ctx.hir().item(i);
self.visit_item(item);
self.ret_vars.clear();
},
StmtKind::Expr(e) | StmtKind::Semi(e) => {
self.visit_expr(e);
self.ret_vars.clear();
StmtKind::Item(_) | StmtKind::Expr(_) | StmtKind::Semi(_) => {
walk_stmt(self, s);
},
StmtKind::Local(_) => {},
}
self.ret_vars.clear();
}
fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
if !self.visited_exprs.insert(ex.hir_id) {
return;
}
match ex.kind {
ExprKind::Array(exprs) | ExprKind::Tup(exprs) => {
self.ret_vars = exprs
@ -307,7 +299,7 @@ impl<'tcx> Visitor<'tcx> for SideEffectVisit<'tcx> {
ExprKind::Match(expr, arms, _) => self.visit_match(expr, arms),
// since analysing the closure is not easy, just set all variables in it to side-effect
ExprKind::Closure(_, _, body_id, _, _) => {
let body = self.ty_ctx.hir().body(body_id);
let body = self.tcx.hir().body(body_id);
self.visit_body(body);
let vars = std::mem::take(&mut self.ret_vars);
self.add_side_effect(vars);