Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[beta] backports #97014

Merged
merged 7 commits into from
May 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down
20 changes: 8 additions & 12 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
67 changes: 28 additions & 39 deletions library/std/src/sys/windows/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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!(
Expand All @@ -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)?;
Expand All @@ -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);
Expand All @@ -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.
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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();
Expand All @@ -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 {
Expand Down
7 changes: 2 additions & 5 deletions library/std/src/sys/windows/process/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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*")),
Expand All @@ -17,7 +16,6 @@ fn test_raw_args() {
Arg::Regular(OsString::from("optional-quotes")),
],
false,
false,
)
.unwrap();
assert_eq!(
Expand All @@ -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()
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/generic-associated-types/bugs/issue-89352.base.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Original file line number Diff line number Diff line change
@@ -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)]

Expand Down
21 changes: 21 additions & 0 deletions src/test/ui/generic-associated-types/issue-93262.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 52 additions & 0 deletions src/test/ui/suggestions/issue-96223.rs
Original file line number Diff line number Diff line change
@@ -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() {}
Loading