Rollup of 22 pull requests #91756
Closed
Rollup of 22 pull requests #91756
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As discovered in rust-lang#85461, the MSVC linker treats weak symbols slightly differently than unix-y linkers do. This causes link.exe to fail with LNK1227 "conflicting weak extern definition" where as other targets are able to link successfully. This changes the dead functions from being generated as weak/hidden to private/default which, as the LLVM reference says: > Global values with “private” linkage are only directly accessible by objects in the current module. In particular, linking code into a module with a private global value may cause the private to be renamed as necessary to avoid collisions. Because the symbol is private to the module, all references can be updated. This doesn’t show up in any symbol table in the object file. This fixes the conflicting weak symbols but doesn't address the reason *why* we have conflicting symbols for these dead functions. The test cases added in this commit contain a minimal repro of the fundamental issue which is that the logic used to decide what dead code functions should be codegen'd in the current CGU doesn't take into account that functions can be duplicated across multiple CGUs (for instance, in the case of `#[inline(always)]` functions). Fixing that is likely to be a more complex change (see rust-lang#85461 (comment)). Fixes rust-lang#85461
Also: - Review and edit current docs - Enforce documentation for crate Co-authored-by: r00ster <r00ster91@protonmail.com> Co-authored-by: Camille Gillot <gillot.camille@gmail.com>
It's a) superfluous and b) doesn't work any more.
The documentation previously said the new capacity cannot overflow `usize`, but in fact it cannot exceed `isize::MAX`.
…ad-rustc is ignored
Previously, passing `-v` would emit an overwhelming amount of logging:
```
> Std { stage: 1, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }
> Assemble { target_compiler: Compiler { stage: 1, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
> Assemble { target_compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
< Assemble { target_compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
> Rustc { target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None }, compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
> Std { target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None }, compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
> StartupObjects { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }
< StartupObjects { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }
c Assemble { target_compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
> Libdir { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }
> Sysroot { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
< Sysroot { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
< Libdir { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }
c Libdir { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }
c Sysroot { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
c Assemble { target_compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
> StdLink { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target_compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }
c Libdir { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }
c Libdir { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }
< StdLink { compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target_compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }, target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } }
< Std { target: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None }, compiler: Compiler { stage: 0, host: TargetSelection { triple: "x86_64-unknown-linux-gnu", file: None } } }
... continues for another 150 lines ...
```
This info is occasionally useful when debugging bootstrap itself, but not very useful for figuring
out why a config option was ignored or command wasn't run. Demote it to `-vv` logging so that `-v`
is more useful.
This already enables `download-rustc`, so it's quick to build rustdoc, and this makes it less confusing when changes to rustdoc aren't reflected in the docs. Note that this uses 2 and not 1 because `download-rustc` only affects stage 2 runs.
rust-lang#63684 was merged for 1.39 not 1.32
In Rust, nesting method calls with both require `&mut` access to `self`
produces a borrow-check error:
error[E0499]: cannot borrow `*self` as mutable more than once at a time
--> src/lib.rs:7:14
|
7 | self.foo(self.bar());
| ---------^^^^^^^^^^-
| | | |
| | | second mutable borrow occurs here
| | first borrow later used by call
| first mutable borrow occurs here
That's because Rust has a left-to-right evaluation order, and the method
receiver is passed first. Thus, the argument to the method cannot then
mutate `self`.
There's an easy solution to this error: just extract a local variable
for the inner argument:
let tmp = self.bar();
self.foo(tmp);
However, the error doesn't give any suggestion of how to solve the
problem. As a result, new users may assume that it's impossible to
express their code correctly and get stuck.
This commit adds a (non-structured) suggestion to extract a local
variable for the inner argument to solve the error. The suggestion uses
heuristics that eliminate most false positives, though there are a few
false negatives (cases where the suggestion should be emitted but is
not). Those other cases can be implemented in a future change.
Suggest using a temporary variable to fix borrowck errors Fixes rust-lang#77834. In Rust, nesting method calls with both require `&mut` access to `self` produces a borrow-check error: error[E0499]: cannot borrow `*self` as mutable more than once at a time --> src/lib.rs:7:14 | 7 | self.foo(self.bar()); | ---------^^^^^^^^^^- | | | | | | | second mutable borrow occurs here | | first borrow later used by call | first mutable borrow occurs here That's because Rust has a left-to-right evaluation order, and the method receiver is passed first. Thus, the argument to the method cannot then mutate `self`. There's an easy solution to this error: just extract a local variable for the inner argument: let tmp = self.bar(); self.foo(tmp); However, the error doesn't give any suggestion of how to solve the problem. As a result, new users may assume that it's impossible to express their code correctly and get stuck. This commit adds a (non-structured) suggestion to extract a local variable for the inner argument to solve the error. The suggestion uses heuristics that eliminate most false positives, though there are a few false negatives (cases where the suggestion should be emitted but is not). Those other cases can be implemented in a future change.
…al, r=jackh726 Add tests fixed by rust-lang#90023 The following issues were fixed by rust-lang#90023 Fixes rust-lang#79674 Fixes rust-lang#83765 Fixes rust-lang#86033 Fixes rust-lang#90318 Fixes rust-lang#88468 The following issues were duplicates of rust-lang#90654 Fixes rust-lang#86850 Fixes rust-lang#89022 r? ```@jackh726```
…lacrum Move core/stream/stream/mod.rs to core/stream/stream.rs Removes an unnecessary nested module.
fix typo in `intrinsics::raw_eq` docs
Fix `Vec::reserve_exact` documentation The documentation previously said the new capacity cannot overflow `usize`, but in fact it cannot exceed `isize::MAX`.
…crum Delete Utf8Lossy::from_str This whole type is marked as being for str internals only, but this constructor is never used by str internals. If you had a &str already and wanted to lossy display it or iterate its lossy utf8 chunks, you would simply not use Utf8Lossy because the whole &str is known to be one contiguous valid utf8 chunk. If code really does need to obtain a value of type &Utf8Lossy somewhere, and has only a &str, `Utf8Lossy::from_bytes(s.as_bytes())` remains available. As currently implemented, there is no performance penalty relative to `from_str` i.e. the Utf8Lossy does not "remember" that it was constructed using `from_str` to bypass later utf8 decoding.
…riplett Add unstable book entries for parts of asm that are not being stabilized These are extracted from the existing `asm` documentation in the unstable book that will be removed when `asm` is stabilized. r? ``@joshtriplett``
…-examples, r=jyn514 Replace iterator-based set construction by *Set::From<[T; N]> This uses the array-based construction for `BtreeSet`s and `HashSet`s instead of first creating an iterator. I could also replace the `let mut a = Set::new(); a.insert(...);` fragments if desired.
…crum Improve x.py logging and defaults a bit more r? ``@Mark-Simulacrum``
Add pierwill to .mailmap
…t_new-since, r=dtolnay Fix since attribute for const_linked_list_new feature rust-lang#63684 was merged for 1.39 not 1.32
|
@bors r+ rollup=never p=22 |
|
📌 Commit 8d5ce4a has been approved by |
bors
added
the
S-waiting-on-bors
|
⌛ Testing commit 8d5ce4a with merge 0aa174aae67216b9a09188dab9a8526bfc0672e1... |
|
@matthiaskrgr This looks to subsume #91726, do you want to close that one? |
|
This likely is more PRs than we should include in a rollup. I think around 10 is more reasonable. The more you include, the harder it is to identify regressions or perf regressions. |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
@ehuss iif this merges I would be able to close 91726 while the ice fix is being tested EDIT: nevermind... :D |
Running tests locally doesn't really solve the problem. The PRs could introduce regressions—which would be really hard to bisect later on with a 22 PR rollup—or perf regressions—which would be nearly impossible to track down. |
|
Well so far I have found 5-7 prs for none-rollup=always to be an ok balance. Trying to merge that without rollups would take at least 8 days according to my math (60* ~3.5 hours) assuming no rebases are needed and no failures happen) => 210 hours => 8.75 days. If you have any ideas how we can merge more PRs reliably, please share! It would be great if we could merge every PR somewhere between 3 and 48 hours after approval imo; every minute someone spends resolving merge conflicts after approval could as well be a minute they spent coding :) |
|
@matthiaskrgr I do really appreciate all the rollups you've been doing recently to shrink the queue :) I think it's fine to rollup many documentation PRs, say, since they can't cause correctness or perf regressions. Other than that, I really have any ideas unfortunately. |
|
The queue will sort itself out over time. Even though it might be counterintuitive, merging PRs faster doesn't decrease the chance of needing to resolve merge conflicts (only if PRs consistently jump ahead of an approved PR). There are really two classes of rollup=always: those that are very safe (documentation, moving code without otherwise changing semantics, adding instrumentation), which if they don't conflict shouldn't cause problems; and those that are small, local changes, which can introduce bugs or regressions. PRs of the former shouldn't really count much in terms of rollups, PRs of the latter do count. All I'm saying here is that as far as number of PRs in a rollup, the former are better to include more of than the latter. Other than that, I wouldn't worry about making bigger rollup PRs to move the queue faster. One thing that can be done is to make disjoint rollup PRs with different PRs. So you can make the rollups and not have to worry about them. It doesn't necessarily cut down on CI time like that, but can cut down on the need to watch the queue or such |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
rustc_incremental#90407 (Document all public items inrustc_incremental)from()to initializeHashMaps andBTreeMaps #91482 (Update documentation to usefrom()to initializeHashMaps andBTreeMaps)core::ready::Ready#91646 (Fix documentation forcore::ready::Ready)ErrorKind::Other#91668 (Remove the match onErrorKind::Other)intrinsics::raw_eqdocs #91681 (fix typo inintrinsics::raw_eqdocs)Vec::reserve_exactdocumentation #91686 (FixVec::reserve_exactdocumentation)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup