| Age | Commit message (Collapse) | Author | Lines |
|---|
|
Preserve `SyntaxContext` for invalid/dummy spans in crate metadata
Fixes #85197
We already preserved the `SyntaxContext` for invalid/dummy spans in the
incremental cache, but we weren't doing the same for crate metadata.
If an invalid (lo/hi from different files) span is written to the
incremental cache, we will decode it with a 'dummy' location, but keep
the original `SyntaxContext`. Since the crate metadata encoder was only
checking for `DUMMY_SP` (dummy location + root `SyntaxContext`),
the metadata encoder would treat it as a normal span, encoding the
`SyntaxContext`. As a result, the final span encoded to the metadata
would change across sessions, even if the crate itself was unchanged.
This could lead to an 'unstable fingerprint' ICE under the following conditions:
1. We compile a crate with an invalid span using incremental compilation. The metadata encoder discards the `SyntaxContext` since the span is invalid, while the incremental cache encoder preserves the `SyntaxContext`
2. From another crate, we execute a foreign query, decoding the invalid span from the metadata as `DUMMY_SP` (e.g. with `SyntaxContext::root()`). This span gets hashed into the query fingerprint. So far, this has always happened through the `optimized_mir` query.
3. We recompile the first crate using our populated incremental cache, without changing anything. We load the (previously) invalid span from our incremental cache - it gets converted to a span with a dummy (but valid) location, along with the original `SyntaxContext`. This span gets written out to the crate metadata - since it now has a valid location, we preserve its `SyntaxContext`.
4. We recompile the second crate, again using a populated incremental cache. We now re-run the foreign query `optimized_mir` - the foreign crate hash is unchanged, but we end up decoding a different span (it now ha a non-root `SyntaxContext`). This results in the fingerprint changing, resulting in an ICE.
This PR updates our encoding of spans in the crate metadata to mirror
the encoding of spans into the incremental cache. We now always encode a
`SyntaxContext`, and encode location information for spans with a
non-dummy location.
|
|
|
|
Fixes #85197
We already preserved the `SyntaxContext` for invalid/dummy spans in the
incremental cache, but we weren't doing the same for crate metadata.
If an invalid (lo/hi from different files) span is written to the
incremental cache, we will decode it with a 'dummy' location, but keep
the original `SyntaxContext`. Since the crate metadata encoder was only
checking for `DUMMY_SP` (dummy location + root `SyntaxContext`),
the metadata encoder would treat it as a normal span, encoding the
`SyntaxContext`. As a result, the final span encoded to the metadata
would change across sessions, even if the crate itself was unchanged.
This PR updates our encoding of spans in the crate metadata to mirror
the encoding of spans into the incremental cache. We now always encode a
`SyntaxContext`, and encode location information for spans with a
non-dummy location.
|
|
|
|
|
|
Fixes #84252
This query reads from untracked global state in `CStore`.
|
|
|
|
|
|
Fixes #80691
When we evaluate a trait predicate, we convert an
`EvaluatedToOk` result to `EvaluatedToOkModuloRegions` if we erased any
regions. We cache the result under a region-erased 'freshened'
predicate, so `EvaluatedToOk` may not be correct for other predicates
that have the same cache key.
|
|
|
|
Fixes issue #82920
Even if an item does not change between compilation sessions, it may end
up with a different `DefId`, since inserting/deleting an item affects
the `DefId`s of all subsequent items. Therefore, we use a `DefPathHash`
in the incremental compilation system, which is stable in the face of
changes to unrelated items.
In particular, the query system will consider the inputs to a query to
be unchanged if any `DefId`s in the inputs have their `DefPathHash`es
unchanged. Queries are pure functions, so the query result should be
unchanged if the query inputs are unchanged.
Unfortunately, it's possible to inadvertantly make a query result
incorrectly change across compilations, by relying on the specific value
of a `DefId`. Specifically, if the query result is a slice that gets
sorted by `DefId`, the precise order will depend on how the `DefId`s got
assigned in a particular compilation session. If some definitions end up
with different `DefId`s (but the same `DefPathHash`es) in a subsequent
compilation session, we will end up re-computing a *different* value for
the query, even though the query system expects the result to unchanged
due to the unchanged inputs.
It turns out that we have been sorting the predicates computed during
`astconv` by their `DefId`. These predicates make their way into the
`super_predicates_that_define_assoc_type`, which ends up getting used to
compute the vtables of trait objects. This, re-ordering these predicates
between compilation sessions can lead to undefined behavior at runtime -
the query system will re-use code built with a *differently ordered*
vtable, resulting in the wrong method being invoked at runtime.
This PR avoids sorting by `DefId` in `astconv`, fixing the
miscompilation. However, it's possible that other instances of this
issue exist - they could also be easily introduced in the future.
To fully fix this issue, we should
1. Turn on `-Z incremental-verify-ich` by default. This will cause the
compiler to ICE whenver an 'unchanged' query result changes between
compilation sessions, instead of causing a miscompilation.
2. Remove the `Ord` impls for `CrateNum` and `DefId`. This will make it
difficult to introduce ICEs in the first place.
|
|
|
|
r=Mark-Simulacrum"
This reverts commit b4def89d76896eec73b4af33642ba7e5eb53c567, reversing
changes made to 7dc1e852d43cb8c9e77dc1e53014f0eb85d2ebfb.
|
|
Reintroduce hir::ExprKind::If
Basically copied and paste #59288/https://github.com/rust-lang/rust-clippy/pull/4080 with some modifications.
The vast majority of tests were fixed and now there are only a few remaining. Since I am still unable to figure out the missing pieces, any help with the following list is welcome.
- [ ] **Unnecessary `typeck` exception**: [Cheated on this one to make CI green.](https://github.com/rust-lang/rust/pull/79328/files#diff-3faee9ba23fc54a12b7c43364ba81f8c5660045c7e1d7989a02a0cee1c5b2051)
- [x] **Incorrect span**: [Span should reference `then` and `else` separately.](https://github.com/rust-lang/rust/pull/79328/files#diff-cf2c46e82222ee4b1037a68fff8a1af3c4f1de7a6b3fd798aacbf3c0475abe3d)
- [x] **New note regarding `assert!`**: [Modified but not "wrong". Maybe can be a good thing?](https://github.com/rust-lang/rust/pull/79328/files#diff-9e0d7c89ed0224e2b62060c957177c27db43c30dfe3c2974cb6b5091cda9cfb5)
- [x] **Inverted report location**: [Modified but not "wrong". Locations were inverted.](https://github.com/rust-lang/rust/pull/79328/files#diff-f637ce7c1f68d523a165aa9651765df05e36c4d7d279194b1a6b28b48a323691)
- [x] **`src/test/ui/point-to-type-err-cause-on-impl-trait-return.rs` has weird errors**: [Not sure why this is happening.](https://github.com/rust-lang/rust/pull/79328/files#diff-c823c09660f5b112f95e97e8ff71f1797b6c7f37dbb3d16f8e98bbaea8072e95)
- [x] **Missing diagnostic**: [???](https://github.com/rust-lang/rust/pull/79328/files#diff-6b8ab09360d725ba4513933827f9796b42ff9522b0690f80b76de067143af2fc)
|
|
Fixes #80336
Due to macro expansion, we may end up with spans with an invalid
location and non-root `SyntaxContext`. This commits preserves the
`SyntaxContext` of such spans in the incremental cache, and ensures
that we always hash the `SyntaxContext` when computing the `Fingerprint`
of a `Span`
Previously, we would discard the `SyntaxContext` during serialization to
the incremental cache, causing the span's `Fingerprint` to change across
compilation sessions.
|
|
|
|
|
|
r=davidtwco,wesleywiser"
This reverts commit ddf2cc7f8eb34f1a63b491d6a52e3e8208393c09, reversing
changes made to 937f629535f38c655267f1ed21ce6830f592f5df.
|
|
Do not make local copies of inline fns in debug mode
r? `@wesleywiser`
cc `@rust-lang/wg-incr-comp`
If this is correct it supersedes https://github.com/rust-lang/rust/pull/76889
Related to #54089
|
|
|
|
Fixes #79890
Previously, we just copied a `RawDefId` from the 'old' map to the 'new'
map. However, the `RawDefId` for a given `DefPathHash` may be different
in the current compilation session. Using `def_path_hash_to_def_id`
ensures that the `RawDefId` we use is valid in the current session.
|
|
On the nopt builders, we disable optimization by default for all tests
which causes the new behavior to take effect and causes the tests to
fail when they should not. By passing the `-O` flag explicitly, we will
always run these tests with optimizations enabled.
|
|
Fixes #79661
In incremental compilation mode, we update a `DefPathHash -> DefId`
mapping every time we create a `DepNode` for a foreign `DefId`.
This mapping is written out to the on-disk incremental cache, and is
read by the next compilation session to allow us to lazily decode
`DefId`s.
When we decode a `DepNode` from the current incremental cache, we need
to ensure that any previously-recorded `DefPathHash -> DefId` mapping
gets recorded in the new mapping that we write out. However, PR #74967
didn't do this in all cases, leading to us being unable to decode a
`DefPathHash` in certain circumstances.
This PR refactors some of the code around `DepNode` deserialization to
prevent this kind of mistake from happening again.
|
|
r=nikomatsakis"
This reverts commit 349b3b324dade7ca638091db93ba08bbc443c63d, reversing
changes made to b776d1c3e3db8befabb123ebb1e46c3531eaed46.
|
|
Allow Trait inheritance with cycles on associated types
Fixes #35237
r? `@nikomatsakis`
cc `@estebank`
|
|
|
|
|
|
|
|
|
|
Update affected ui & incremental tests to use a user declared variable
bindings instead of temporaries. The former are preserved because of
debuginfo, the latter are not.
|
|
Use llvm::computeLTOCacheKey to determine post-ThinLTO CGU reuse
During incremental ThinLTO compilation, we attempt to re-use the
optimized (post-ThinLTO) bitcode file for a module if it is 'safe' to do
so.
Up until now, 'safe' has meant that the set of modules that our current
modules imports from/exports to is unchanged from the previous
compilation session. See PR #67020 and PR #71131 for more details.
However, this turns out be insufficient to guarantee that it's safe
to reuse the post-LTO module (i.e. that optimizing the pre-LTO module
would produce the same result). When LLVM optimizes a module during
ThinLTO, it may look at other information from the 'module index', such
as whether a (non-imported!) global variable is used. If this
information changes between compilation runs, we may end up re-using an
optimized module that (for example) had dead-code elimination run on a
function that is now used by another module.
Fortunately, LLVM implements its own ThinLTO module cache, which is used
when ThinLTO is performed by a linker plugin (e.g. when clang is used to
compile a C proect). Using this cache directly would require extensive
refactoring of our code - but fortunately for us, LLVM provides a
function that does exactly what we need.
The function `llvm::computeLTOCacheKey` is used to compute a SHA-1 hash
from all data that might influence the result of ThinLTO on a module.
In addition to the module imports/exports that we manually track, it
also hashes information about global variables (e.g. their liveness)
which might be used during optimization. By using this function, we
shouldn't have to worry about new LLVM passes breaking our module re-use
behavior.
In LLVM, the output of this function forms part of the filename used to
store the post-ThinLTO module. To keep our current filename structure
intact, this PR just writes out the mapping 'CGU name -> Hash' to a
file. To determine if a post-LTO module should be reused, we compare
hashes from the previous session.
This should unblock PR #75199 - by sheer chance, it seems to have hit
this issue due to the particular CGU partitioning and optimization
decisions that end up getting made.
|
|
These need to only be bounds to avoid cycle errors in trait checking.
|
|
|
|
|
|
During incremental ThinLTO compilation, we attempt to re-use the
optimized (post-ThinLTO) bitcode file for a module if it is 'safe' to do
so.
Up until now, 'safe' has meant that the set of modules that our current
modules imports from/exports to is unchanged from the previous
compilation session. See PR #67020 and PR #71131 for more details.
However, this turns out be insufficient to guarantee that it's safe
to reuse the post-LTO module (i.e. that optimizing the pre-LTO module
would produce the same result). When LLVM optimizes a module during
ThinLTO, it may look at other information from the 'module index', such
as whether a (non-imported!) global variable is used. If this
information changes between compilation runs, we may end up re-using an
optimized module that (for example) had dead-code elimination run on a
function that is now used by another module.
Fortunately, LLVM implements its own ThinLTO module cache, which is used
when ThinLTO is performed by a linker plugin (e.g. when clang is used to
compile a C proect). Using this cache directly would require extensive
refactoring of our code - but fortunately for us, LLVM provides a
function that does exactly what we need.
The function `llvm::computeLTOCacheKey` is used to compute a SHA-1 hash
from all data that might influence the result of ThinLTO on a module.
In addition to the module imports/exports that we manually track, it
also hashes information about global variables (e.g. their liveness)
which might be used during optimization. By using this function, we
shouldn't have to worry about new LLVM passes breaking our module re-use
behavior.
In LLVM, the output of this function forms part of the filename used to
store the post-ThinLTO module. To keep our current filename structure
intact, this PR just writes out the mapping 'CGU name -> Hash' to a
file. To determine if a post-LTO module should be reused, we compare
hashes from the previous session.
This should unblock PR #75199 - by sheer chance, it seems to have hit
this issue due to the particular CGU partitioning and optimization
decisions that end up getting made.
|
|
If a symbol name can only be imported from one place for a type, and
as long as it was not glob-imported anywhere in the current crate, we
can trim its printed path and print only the name.
This has wide implications on error messages with types, for example,
shortening `std::vec::Vec` to just `Vec`, as long as there is no other
`Vec` importable anywhere.
This adds a new '-Z trim-diagnostic-paths=false' option to control this
feature.
On the good path, with no diagnosis printed, we should try to avoid
issuing this query, so we need to prevent trimmed_def_paths query on
several cases.
This change also relies on a previous commit that differentiates
between `Debug` and `Display` on various rustc types, where the latter
is trimmed and presented to the user and the former is not.
|
|
|
|
|
|
|
|
|
|
Clear MIR local type annotations after borrowck
|
|
|
|
Fixes #72386
This type also needs to get allocated on the `librustc_middle` arena
when we deserialize MIR.
|
|
Incomplete features can also be unsound
Some incomplete features do not just ICE, they are also currently unsound (e.g. https://github.com/rust-lang/rust/pull/72029, and also `specialization` -- which is not yet marked incomplete but [should be](https://github.com/rust-lang/rust/pull/71420)). This makes the message reflect that.
While at it I also added a link to the tracking issue, which hopefully should explain what is incomplete/unsound about the feature.
|
|
This commit fixes an issue where the codegen backend's selection of LTO
disagreed with what the codegen later thought was being done. Discovered
in #72006 we have a longstanding issue where if `-Clinker-plugin-lto` in
optimized mode is compiled incrementally it will always panic on the
second compilation. The underlying issue turned out to be that the
production of the original artifact determined that LTO should not be
done (because it's being postponed to the linker) but the CGU reuse
selection thought that LTO was done so it was trying to load pre-LTO
artifacts which were never generated.
The fix here is to ensure that the logic when generating code which
determines what kind of LTO is being done is shared amongst the CGU
reuse decision and the backend actually doing LTO. This means that
they'll both be in agreement about whether the previous compilation did
indeed produce incremental pre-LTO artifacts.
Closes #72006
|
|
|
|
One or two tests became build-pass without the FIXME because they really
needed build-pass (were failing without it).
Helps with #62277
|
|
|
|
Namely, a regression test for issue #69798 (export added), and the inverse of
that test (export removd).
|
|
|
