Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Rollup of 10 pull requests #90963

Closed
wants to merge 27 commits into from

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

ChrisDenton and others added 27 commits October 30, 2021 12:03
"echo" is not an application on Windows so `Command` tests could fail even if that's not what's being tested for.
The spans for "trait bound not satisfied" errors in trivial trait bounds referenced the entire item (fn, impl, struct) before.
Now they only reference the obligation itself (`String: Copy`)

Address rust-lang#90869
This also fixes the same suggestion, which was kind of broken, because it just searched for the last occurence of `const` to replace with a `let`. This works great in some cases, but when there is no const and a leading space to the file, it doesn't work and panic with overflow because it thought that it had found a const.

I also changed the suggestion to only trigger if the `const` and the non-constant value are on the same line, because if they aren't, the suggestion is very likely to be wrong.

Also don't trigger the suggestion if the found `const` is on line 0, because that triggers the ICE.
This should significantly reduce the frequency of merge conflicts.
Co-authored-by: Yuki Okushi <jtitor@2k36.org>
Windows: Resolve `process::Command` program without using the current directory

Currently `std::process::Command` searches many directories for the executable to run, including the current directory. This has lead to a [CVE for `ripgrep`](https://cve.circl.lu/cve/CVE-2021-3013) but presumably other command line utilities could be similarly vulnerable if they run commands. This was [discussed on the internals forum](https://internals.rust-lang.org/t/std-command-resolve-to-avoid-security-issues-on-windows/14800). Also discussed was [which directories should be searched](https://internals.rust-lang.org/t/windows-where-should-command-new-look-for-executables/15015).

EDIT: This PR originally removed all implicit paths. They've now been added back as laid out in the rest of this comment.

## Old Search Strategy

The old search strategy is [documented here][1]. Additionally Rust adds searching the child's paths (see also rust-lang#37519). So the full list of paths that were searched was:

1. The directories that are listed in the child's `PATH` environment variable.
2. The directory from which the application loaded.
3. The current directory for the parent process.
4. The 32-bit Windows system directory.
5. The 16-bit Windows system directory.
6. The Windows directory.
7. The directories that are listed in the PATH environment variable.

## New Search Strategy

The new strategy removes the current directory from the searched paths.

1. The directories that are listed in the child's PATH environment variable.
2. The directory from which the application loaded.
3. The 32-bit Windows system directory.
4. The Windows directory.
5. The directories that are listed in the parent's PATH environment variable.

Note that it also removes the 16-bit system directory, mostly because there isn't a function to get it. I do not anticipate this being an issue in modern Windows.

## Impact

Removing the current directory should fix CVE's like the one linked above. However, it's possible some Windows users of affected Rust CLI applications have come to expect the old behaviour.

This change could also affect small Windows-only script-like programs that assumed the current directory would be used. The user would need to use `.\file.exe` instead of the bare application name.

This PR could break tests, especially those that test the exact output of error messages (e.g. Cargo) as this does change the error messages is some cases.

[1]: https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa#parameters
warn on must_use use on async fn's

As referenced in rust-lang#78149

This only works on `async` fn's for now, I can also look into if I can get `Box<dyn Future>` and `impl Future` working at this level (hir)
…shtriplett

Add Vec::retain_mut

This is to continue the discussion started in rust-lang#83218.

Original comment was:

> Take 2 of rust-lang#34265, since I needed this today.

The reason I think why we should add `retain_mut` is for coherency and for discoverability. For example we have `chunks` and `chunks_mut` or `get` and `get_mut` or `iter` and `iter_mut`, etc. When looking for mutable `retain`, I would expect `retain_mut` to exist. It took me a while to find out about `drain_filter`. So even if it provides an API close to `drain_filter`, just for the discoverability, I think it's worth it.

cc ``@m-ou-se`` ``@jonas-schievink`` ``@Mark-Simulacrum``
…r=oli-obk

Add `#[inline]`s to `SortedIndexMultiMap`

They're small enough and good candidates to add `#[inline]` generally.
Print escaped string if char literal has multiple characters, but only one printable character

Fixes rust-lang#90857

I'm not sure about the error message here, it could get rather long and *maybe* using the names of characters would be better? That wouldn't help the length any, though.
…und, r=estebank

Fix span for non-satisfied trivial trait bounds

The spans for "trait bound not satisfied" errors in trivial trait bounds referenced the entire item (fn, impl, struct) before.
Now they only reference the obligation itself (`String: Copy`)

Address rust-lang#90869
Remove workaround for the forward progress handling in LLVM

this workaround was only needed for LLVM < 12 and the minimum LLVM version was updated to 12 in rust-lang#90175
…r=estebank

Fix `non-constant value` ICE (rust-lang#90878)

This also fixes the same suggestion, which was kind of broken, because it just searched for the last occurence of `const` to replace with a `let`. This works great in some cases, but when there is no const and a leading space to the file, it doesn't work and panic with overflow because it thought that it had found a const.

I also changed the suggestion to only trigger if the `const` and the non-constant value are on the same line, because if they aren't, the suggestion is very likely to be wrong.

Also don't trigger the suggestion if the found `const` is on line 0, because that triggers the ICE.

Asking Esteban to review since he was the last one to change the relevant code.

r? `@estebank`

Fixes rust-lang#90878
…htriplett

Alphabetize language features

This should significantly reduce the frequency of merge conflicts.

r? ``@joshtriplett``

``@rustbot`` label: +A-contributor-roadblock +S-waiting-on-review
@rustbot rustbot added the rollup A PR which is a rollup label Nov 16, 2021
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=10

@bors
Copy link
Contributor

bors commented Nov 16, 2021

📌 Commit 1815a17 has been approved by matthiaskrgr

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 16, 2021
@bors
Copy link
Contributor

bors commented Nov 16, 2021

⌛ Testing commit 1815a17 with merge ad91a2f354b3ce97fa29c31ef66b89d1e38228c0...

@Noratrieb
Copy link
Member

Noratrieb commented Nov 16, 2021

Hi, my pull request Fix non-constant value ICE (#90878) #90930 is not on the newest commit here in the rollup, there was still an issue that was just resolved.

edit: wrong pr

@matthiaskrgr
Copy link
Member Author

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 16, 2021
@matthiaskrgr
Copy link
Member Author

@bors force

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@matthiaskrgr matthiaskrgr deleted the rollup-w3ar4iz branch November 20, 2021 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.