Rollup of 3 pull requests #67412
Closed
Rollup of 3 pull requests #67412
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
…roducts. resolves issue 59535.
however, make sure run-make test actually runs, too. (The `# only-target` thing does not work, at least not for cross-compilation. See issue 67018.))
Instead of accumulating (and acting upon) LTO import information over an unbounded number of prior compilations, just see if the curent import set matches the previous import set. if they don't match, then you cannot reuse the PostLTO build product for that module.
…parison slow-path.
…=dtolnay
Implement `DebugStruct::non_exhaustive`.
This patch adds a function (finish_non_exhaustive) to add ellipsis before the closing brace when formatting using `DebugStruct`.
## Example
```rust
#![feature(debug_non_exhaustive)]
use std::fmt;
struct Bar {
bar: i32,
hidden: f32,
}
impl fmt::Debug for Bar {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt.debug_struct("Bar")
.field("bar", &self.bar)
.non_exhaustive(true) // Show that some other field(s) exist.
.finish()
}
}
assert_eq!(
format!("{:?}", Bar { bar: 10, hidden: 1.0 }),
"Bar { bar: 10, .. }",
);
```
…t-lto-imports, r=michaelwoerister save LTO import info and check it when trying to reuse build products Fix rust-lang#59535 Previous runs of LTO optimization on the previous incremental build can import larger portions of the dependence graph into a codegen unit than the current compilation run is choosing to import. We need to take that into account when we choose to reuse PostLTO-optimization object files from previous compiler invocations. This PR accomplishes that by serializing the LTO import information on each incremental build. We load up the previous LTO import data as well as the current LTO import data. Then as we decide whether to reuse previous PostLTO objects or redo LTO optimization, we check whether the LTO import data matches. After we finish with this decision process for every object, we write the LTO import data back to disk. ---- What is the scenario where comparing against past LTO import information is necessary? I've tried to capture it in the comments in the regression test, but here's yet another attempt from me to summarize the situation: 1. Consider a call-graph like `[A] -> [B -> D] <- [C]` (where the letters are functions and the modules are enclosed in `[]`) 2. In our specific instance, the earlier compilations were inlining the call to`B` into `A`; thus `A` ended up with a external reference to the symbol `D` in its object code, to be resolved at subsequent link time. The LTO import information provided by LLVM for those runs reflected that information: it explicitly says during those runs, `B` definition and `D` declaration were imported into `[A]`. 3. The change between incremental builds was that the call `D <- C` was removed. 4. That change, coupled with other decisions within `rustc`, made the compiler decide to make `D` an internal symbol (since it was no longer accessed from other codegen units, this makes sense locally). And then the definition of `D` was inlined into `B` and `D` itself was eliminated entirely. 5. The current LTO import information reported that `B` alone is imported into `[A]` for the *current compilation*. So when the Rust compiler surveyed the dependence graph, it determined that nothing `[A]` imports changed since the last build (and `[A]` itself has not changed either), so it chooses to reuse the object code generated during the previous compilation. 6. But that previous object code has an unresolved reference to `D`, and that causes a link time failure! ---- The interesting thing is that its quite hard to actually observe the above scenario arising, which is probably why no one has noticed this bug in the year or so since incremental LTO support landed (PR rust-lang#53673). I've literally spent days trying to observe the bug on my local machine, but haven't managed to find the magic combination of factors to get LLVM and `rustc` to do just the right set of the inlining and `internal`-reclassification choices that cause this particular problem to arise. ---- Also, I have tried to be careful about injecting new bugs with this PR. Specifically, I was/am worried that we could get into a scenario where overwriting the current LTO import data with past LTO import data would cause us to "forget" a current import. ~~To guard against this, the PR as currently written always asserts, at overwrite time, that the past LTO import-set is a *superset* of the current LTO import-set. This way, the overwriting process should always be safe to run.~~ * The previous note was written based on the first version of this PR. It has since been revised to use a simpler strategy, where we never attempt to merge the past LTO import information into the current one. We just *compare* them, and act accordingly. * Also, as you can see from the comments on the PR itself, I was quite right to be worried about forgetting past imports; that scenario was observable via a trivial transformation of the regression test I had devised.
…=varkor Use structured suggestion for disambiguating method calls Fix rust-lang#65635.
|
@bors r+ p=100 |
|
📌 Commit dced4c2 has been approved by |
|
🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened |
bors
added
the
S-waiting-on-bors
|
⌛ Testing commit dced4c2 with merge 45c7edc53117ae3185133abc07adf17cb2fb358d... |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
💔 Test failed - checks-azure |
bors
added
S-waiting-on-review
|
I would strongly recommend against rolling up things that touch run-make, run-*-fulldeps. |
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:
DebugStruct::non_exhaustive. #66716 (ImplementDebugStruct::non_exhaustive.)Failed merges:
r? @ghost