Rollup of 9 pull requests #73643
Merged
Rollup of 9 pull requests #73643
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
We can never supply a meaningful implementation of this. Instead, the follow up commits will create two intrinsics that approximate comparisons: * `ptr_maybe_eq` * `ptr_maybe_ne` The fact that `ptr_maybe_eq(a, b)` is not necessarily the same value as `!ptr_maybe_ne(a, b)` is a symptom of this entire problem.
…aining everything in the diagnostic
Fixes rust-lang#64731, rust-lang#73531. See also rust-lang#64402#issuecomment-530852886
This commit fixes typos in the doc comments of 'librustc_mir/monomorphize/collector.rs'
typo fix Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
This commit fixes a regression introduced in rust-lang#73317 where an oversight meant that `config.toml` was assumed to exist. Signed-off-by: David Wood <david@davidtw.co>
Also, update the affected tests. This seems strictly better but it is
actually more permissive than I initially intended. In particular it
accepts this
```
forall<'a, 'b> {
exists<'intersection> {
'a: 'intersection,
'b: 'intersection,
}
}
```
and I'm not sure I want to accept that. It implies that we have a
`'empty` in the new universe intoduced by the `forall`.
…ers" This reverts commit 2e01db4b396a1e161f7a73933fff34bc9421dba0.
In the new leak check, instead of getting a list of placeholders to track, we look for any placeholder that is part of a universe which was created during the snapshot. We are looking for the following error patterns: * P1: P2, where P1 != P2 * P1: R, where R is in some universe that cannot name P1 This new leak check is more precise than before, in that it accepts this patterns: * R: P1, even if R cannot name P1, because R = 'static is a valid sol'n * R: P1, R: P2, as above Note that this leak check, when running during subtyping, is less efficient than before in some sense because it is going to check and re-check all the universes created since the snapshot. We're going to move when the leak check runs to try and correct that.
In particular, it no longer occurs during the subtyping check. This is
important for enabling lazy normalization, because the subtyping check
will be producing sub-obligations that could affect its results.
Consider an example like
for<'a> fn(<&'a as Mirror>::Item) =
fn(&'b u8)
where `<T as Mirror>::Item = T` for all `T`. We will wish to produce a
new subobligation like
<'!1 as Mirror>::Item = &'b u8
This will, after being solved, ultimately yield a constraint that `'!1
= 'b` which will fail. But with the leak-check being performed on
subtyping, there is no opportunity to normalize `<'!1 as
Mirror>::Item` (unless we invoke that normalization directly from
within subtyping, and I would prefer that subtyping and unification
are distinct operations rather than part of the trait solving stack).
The reason to keep the leak check during coherence and trait
evaluation is partly for backwards compatibility. The coherence change
permits impls for `fn(T)` and `fn(&T)` to co-exist, and the trait
evaluation change means that we can distinguish those two cases
without ambiguity errors. It also avoids recreating rust-lang#57639, where we
were incorrectly choosing a where clause that would have failed the
leak check over the impl which succeeds.
The other reason to keep the leak check in those places is that I
think it is actually close to the model we want. To the point, I think
the trait solver ought to have the job of "breaking down"
higher-ranked region obligation like ``!1: '2` into into region
obligations that operate on things in the root universe, at which
point they should be handed off to polonius. The leak check isn't
*really* doing that -- these obligations are still handed to the
region solver to process -- but if/when we do adopt that model, the
decision to pass/fail would be happening in roughly this part of the
code.
This change had somewhat more side-effects than I anticipated. It
seems like there are cases where the leak-check was not being enforced
during method proving and trait selection. I haven't quite tracked
this down but I think it ought to be documented, so that we know what
precisely we are committing to.
One surprising test was `issue-30786.rs`. The behavior there seems a
bit "fishy" to me, but the problem is not related to the leak check
change as far as I can tell, but more to do with the closure signature
inference code and perhaps the associated type projection, which
together seem to be conspiring to produce an unexpected
signature. Nonetheless, it is an example of where changing the
leak-check can have some unexpected consequences: we're now failing to
resolve a method earlier than we were, which suggests we might change
some method resolutions that would have been ambiguous to be
successful.
TODO:
* figure out remainig test failures
* add new coherence tests for the patterns we ARE disallowing
Revert the code that states that upcasting traits requires full equality and change to require that the source type is a subtype of the target type, as one would expect. As the comment states, this was an old bug that we didn't want to fix yet as it interacted poorly with the old leak-check. This fixes the old-lub-glb-object test, which was previously reporting too many errors (i.e., in the previous commit).
Motivation: - we want to use leak-check sparingly, first off - these calls were essentially the same as doing the check during subtyping
The bug was revealed by the behavior of the old-lub-glb-hr-noteq1.rs test. The old-lub-glb-hr-noteq2 test shows the current 'order dependent' behavior of coercions around higher-ranked functions, at least when running with `-Zborrowck=mir`. Also, run compare-mode=nll.
…thewjasper move leak-check to during coherence, candidate eval Implementation of MCP rust-lang/compiler-team#295. I'd like to do a crater run on this. Note to @rust-lang/lang: This PR is a breaking change (bugfix). It causes tests like the following to go from a future-compatibility warning rust-lang#56105 to a hard error: ```rust trait Trait {} impl Trait for for<'a, 'b> fn(&'a u32, &'b u32) {} impl Trait for for<'c> fn(&'c u32, &'c u32) {} // now rejected, used to warn ``` I am not aware of any instances of this code in the wild, but that is why we are doing a crater run. The reason for this change is that those two types are, in fact, the same type, and hence the two impls are overlapping. There will still be impls that trigger rust-lang#56105 after this lands, however -- I hope that we will eventually just accept those impls without warning, for the most part. One example of such an impl is this pattern, which is used by wasm-bindgen and other crates as well: ```rust trait Trait {} impl<T> Trait for fn(&T) { } impl<T> Trait for fn(T) { } // still accepted, but warns ```
…,RalfJung,nagisa A way forward for pointer equality in const eval r? @varkor on the first commit and @RalfJung on the second commit cc rust-lang#53020
…an-DPC Clean up E0689 explanation r? @Dylan-DPC
…, r=nikomatsakis Account for multiple impl/dyn Trait in return type when suggesting `'_` Make `impl` and `dyn` Trait lifetime suggestions a bit more resilient. Follow up to rust-lang#72804. r? @nikomatsakis
…s, r=oli-obk Add second message for LiveDrop errors This is an attempt to fix rust-lang#72907 by adding a second message to the `LiveDrop` diagnostics. Changing from this ``` error[E0493]: destructors cannot be evaluated at compile-time --> src/lib.rs:7:9 | 7 | let mut always_returned = None; | ^^^^^^^^^^^^^^^^^^^ constants cannot evaluate destructors error: aborting due to previous error ``` to this ``` error[E0493]: destructors cannot be evaluated at compile-time --> foo.rs:6:9 | 6 | let mut always_returned = None; | ^^^^^^^^^^^^^^^^^^^ constants cannot evaluate destructors ... 10 | always_returned = never_returned; | --------------- value is dropped here error: aborting due to previous error ``` r? @RalfJung @ecstatic-morse
Clarify --extern documentation. Fixes rust-lang#64731, rust-lang#73531. See also rust-lang#64402 (comment)
Fix typos in doc comments Hello 🦀 , This commit fixes typos in the doc comments of 'librustc_mir/monomorphize/collector.rs' Thank you for reviewing this PR 👍
…var, r=Mark-Simulacrum bootstrap: no `config.toml` exists regression Fixes rust-lang#73574. This PR fixes a regression introduced in rust-lang#73317 where an oversight meant that `config.toml` was assumed to exist.
|
📌 Commit 44900f8 has been approved by |
bors
added
the
S-waiting-on-bors
|
#72493 was marked as rollup=never but is still part of this rollup, is this a bug in bors/homu? |
|
☀️ Test successful - checks-azure |
This was referenced Jun 23, 2020
|
This merge was a big perf win for |
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:
'_#73496 (Account for multiple impl/dyn Trait in return type when suggesting'_)config.tomlexists regression #73590 (bootstrap: noconfig.tomlexists regression)Failed merges:
r? @ghost