| Age | Commit message (Collapse) | Author | Lines |
|---|
|
A change in #41911 had made `for_all_relevant_impls` do a linear scan over
all impls, instead of using an HashMap. Use an HashMap again to avoid
quadratic blowup when there is a large number of structs with impls.
I think this fixes #43141 completely, but I want better measurements in
order to be sure. As a perf patch, please don't roll this up.
|
|
This reverts commit 9b85e1cfa5aa2aaa4b5df4359a023ad793983ffc, reversing
changes made to 13157c4ebcca735a0842bd03c3dad1de7c429f9f.
|
|
nikomatsakis:incr-comp-anonymize-trait-selection, r=michaelwoerister"
This reverts commit b4502f7c0b51526d0177204a71dc2b3200f7348b, reversing
changes made to 23ecebd6bd4362142ac586014aec44070a177a3d.
|
|
This reverts commit 309ab478d31a699493fdb1593d9cb133705f51f0.
|
|
|
|
|
|
r=michaelwoerister
integrate anon dep nodes into trait selection
Use anonymous nodes for trait selection. In all cases, we use the same basic "memoization" strategy:
- Store the `DepNodeIndex` in the slot along with value.
- If value is present, return it, and add a read of the dep-node-index.
- Else, start an anonymous task, and store resulting node.
We apply this strategy to a number of caches in trait selection:
- The "trans" caches of selection and projection
- The "evaluation" cache
- The "candidate selection" cache
In general, for our cache strategy to be "dep-correct", the computation of the value is permitted to rely on the *value in the key* but nothing else. The basic argument is this: in order to look something up, you have to produce the key, and to do that you must have whatever reads were needed to create the key. Then, you get whatever reads were further needed to produce the value. But if the "closure" that produced the value made use of *other* environmental data, not derivable from the key, that would be bad -- but that would **also** suggest that the cache is messed up (though it's not proof).
The structure of these caches do not obviously prove that the correctness criteria are met, and I aim to address that in further refactorings. But I *believe* it to be the case that, if we assume that the existing caches are correct, there are also no dependency failures (in other words, if there's a bug, it's a pre-existing one). Specifically:
- The trans caches: these take as input just a `tcx`, which is "by definition" not leaky, the `trait-ref` etc, which is part of the key, and sometimes a span (doesn't influence the result). So they seem fine.
- The evaluation cache:
- This computation takes as input the "stack" and has access to the infcx.
- The infcx is a problem -- would be better to take the tcx -- and this is exactly one of the things I plan to improve in later PRs. Let's ignore it for now. =)
- The stack itself is also not great, in that the *key* only consists of the top-entry in the stack.
- However, the stack can cause a problem in two ways:
- overflow (we panic)
- cycle check fails (we do not update the cache, I believe)
- The candidate selection cache:
- as before, takes the "stack" and has access to the infcx.
- here it is not as obvious that we avoid caching stack-dependent computations. However, to the extent that we do, this is a pre-existing bug, in that we are making cache entries we shouldn't.
- I aim to resolve this by -- following the chalk-style of evaluation -- merging candidate selection and evaluation.
- The infcx is a problem -- would be better to take the tcx -- and this is exactly one of the things I plan to improve in later PRs. Let's ignore it for now. =)
- The stack itself is also not great, in that the *key* only consists of the top-entry in the stack.
- Moreover, the stack would generally just introduce ambiguities and errors anyhow, so that lessens the risk.
Anyway, the existing approach to handle dependencies in the trait code carries the same risks or worse, so this seems like a strict improvement!
r? @michaelwoerister
cc @arielb1
|
|
Refactor: {Lvalue,Rvalue,Operand}::ty only need the locals' types, not the full &Mir
I am writing code that needs to call these `ty` methods while mutating MIR -- which is impossible with the current API.
Even with the refactoring the situation is not great: I am cloning the `local_decls` and then passing the clone to the `ty` methods. I have to clone because `Mir::basic_blocks_mut` borrows the entire `Mir` including the `local_decls`. But even that is better than not being able to get these types at all...
Cc @nikomatsakis
|
|
|
|
|
|
|
|
Rollup of 8 pull requests
- Successful merges: #42670, #42826, #43000, #43011, #43098, #43100, #43136, #43137
- Failed merges:
|
|
`rustc_on_unimplemented` supports referring to trait
Add support to `rustc_on_unimplemented` to reference the full path of
the annotated trait. For the following code:
```rust
pub mod Bar {
#[rustc_on_unimplemented = "test error `{Self}` with `{Bar}` `{Baz}` `{Quux}` in `{Foo}`"]
pub trait Foo<Bar, Baz, Quux> {}
}
```
the error message will be:
```
test error `std::string::String` with `u8` `_` `u32` in `Bar::Foo`
```
|
|
Note different versions of same crate when absolute paths of different types match.
The current check to address #22750 only works when the paths of the mismatched types relative to the current crate are equal, but this does not always work if one of the types is only included through an indirect dependency. If reexports are involved, the indirectly included path can e.g. [contain private modules](https://github.com/rust-lang/rust/issues/22750#issuecomment-302755516).
This PR takes care of these cases by also comparing the *absolute* path, which is equal if the type hasn't moved in the module hierarchy between versions. A more coarse check would be to compare only the crate names instead of full paths, but that might lead to too many false positives.
Additionally, I believe it would be helpful to show where the differing crates came from, i.e. the information in `rustc::middle::cstore::CrateSource`, but I'm not sure yet how to nicely display all of that, so I'm leaving it to a future PR.
|
|
Refactor pretty printing slightly
This doesn't introduce any functional changes (that I'm aware of). The primary intention here is to clean up the code a little. Each commit is intended to stand alone, reviewing commit-by-commit may be easiest.
|
|
|
|
incr.comp.: Don't include span information in the ICH of type definitions
This should improve some of the `regex` tests on perf.rlo. Not including spans into the ICH is harmless until we also cache warnings. To really solve the problem, we need to do more refactoring (see #43088).
r? @nikomatsakis
|
|
full &Mir
|
|
The literal index was increased in only next_lit, so it isn't
necessary: code now uses an iterator. The cur_cmnt field is also moved
to be increased in print_comment instead of after each call to
print_comment.
|
|
No (intentional) changes to behavior. This is intended to avoid the
anti-pattern of having to import individual methods throughout code.
|
|
Addresses the second part of #42171 by removing the `TraitRef` from
`ProjectionTy`, and directly storing its `Substs`.
Closes #42171.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Implemented `TypeFoldable` for `TypeError`s.
This is quite handy in some user code, for instance to pull out type errors from an inference context when `fresh_substs_for_item` has been used before.
|
|
|
|
|
|
|
|
|
|
Make sufficiently old or low-impact compatibility lints deny-by-default
Needs crater run before proceeding.
r? @nikomatsakis
|
|
Fix some tests on Linux
|
|
This is going to be a hard error while all private-in-public errors from rustc_privacy will be reclassified into lints.
|
|
|
|
|
|
Replace the global fulfillment cache with the evaluation cache
This uses the new "Chalk" ParamEnv refactoring to check "global" predicates in an empty environment, which should be correct because global predicates aren't affected by a consistent environment.
Fixes #39970.
Fixes #42796.
r? @nikomatsakis
|
|
This helps avoid cache pollution. Also add more comments explaining
that.
|
|
The evaluation cache already exists, and it can handle differing
parameter environments natively.
Fixes #39970.
Fixes #42796.
|
|
This is just duplicating the logic from the old fulfillment cache, so
I'm not sure it is 100% correct, but it should not be more wrong than
the old logic.
|
|
Previously, coinductive matching was only blocked on the fulfillment
path, and ignored on the evaluation path.
|
