Explain method-call move errors in loops
PR #73708 added a more detailed explanation of move errors that occur
due to a call to a method that takes `self`. This PR extends that logic
to work when a move error occurs due to a method call in the previous
iteration of a loop.
Add ABI argument for called function in `find_mir_or_eval_fn` and
`call_extra_fn`. Useful for comparing with expected ABI in interpreters.
Related to [miri/1631](https://github.com/rust-lang/miri/issues/1631)
PR #73708 added a more detailed explanation of move errors that occur
due to a call to a method that takes `self`. This PR extends that logic
to work when a move error occurs due to a method call in the previous
iteration of a loop.
MIR Inline is incompatible with coverage
Fixes: #80060
Fixed by disabling inlining if `-Zinstrument-coverage` is set.
The PR also adds additional use cases to the coverage test for doctests.
r? `@wesleywiser`
cc: `@tmandry`
Make `ExpnData` fields `krate` and `orig_id` private
These fields are only used by hygiene serialized, and should not be
accessed by anything outside of `rustc_span`.
use PlaceRef more consistently instead of loosely coupled local+projection
Instead of working directly with the `projections` array, use `iter_projections` and `last_projection`. This avoids having to construct new `PlaceRef` from the pieces everywhere.
I only did this for a few files, to see how people think about this. If y'all are happy with this, I'll open an E-mentor issue to complete this. I grepped for `Place::ty_from` to find the places that need adjusting -- this could miss some, but I am not sure what else to grep for.
Allow references to interior mutable data behind a feature gate
supercedes #80373 by simply not checking for interior mutability on borrows of locals that have `StorageDead` and thus can never be leaked to the final value of the constant
tracking issue: https://github.com/rust-lang/rust/issues/80384
r? `@RalfJung`
Some Promotion Refactoring
Clean up promotion a bit:
* factor out some common code
* more exhaustive matches
This *should* not break anything... the only potentially-breaking change is that `BorrowKind::Shallow | BorrowKind::Unique` are now rejected for internal references.
r? ``@oli-obk``
Rollup of 7 pull requests
Successful merges:
- #80185 (Fix ICE when pointing at multi bytes character)
- #80260 (slightly more typed interface to panic implementation)
- #80311 (Improvements to NatVis support)
- #80337 (Use `desc` as a doc-comment for queries if there are no doc comments)
- #80381 (Revert "Cleanup markdown span handling")
- #80492 (remove empty wraps, don't return Results from from infallible functions)
- #80509 (where possible, pass slices instead of &Vec or &String (clippy::ptr_arg))
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
validate promoteds
Turn on const-value validation for promoteds. This is made possible now that https://github.com/rust-lang/rust/issues/67534 is resolved.
I don't think this is a breaking change. We don't promote any unsafe operation any more (since https://github.com/rust-lang/rust/pull/77526 landed). We *do* promote `const fn` calls under some circumstances (in `const`/`static` initializers), but union field access and similar operations are not allowed in `const fn`. So now is a perfect time to add this check. :D
r? `@oli-obk`
Fixes https://github.com/rust-lang/rust/issues/67465
Remap instrument-coverage line numbers in doctests
This uses the `SourceMap::doctest_offset_line` method to re-map line
numbers from doctests. Remapping columns is not yet done, and rustdoc
still does not output the correct filename when running doctests in a
workspace.
Part of #79417 although I dont consider that fixed until both filenames
and columns are mapped correctly.
r? `@richkadel`
I might jump on zulip the comming days. Still need to figure out how to properly write tests for this, and deal with other doctest issues in the meantime.
Acknowledge that `[CONST; N]` is stable
When `const_in_array_repeat_expressions` (RFC 2203) got unstably implemented as part of https://github.com/rust-lang/rust/pull/61749, accidentally, the special case of repeating a *constant* got stabilized immediately. That is why the following code works on stable:
```rust
const EMPTY: Vec<i32> = Vec::new();
pub const fn bar() -> [Vec<i32>; 2] {
[EMPTY; 2]
}
fn main() {
let x = bar();
}
```
In contrast, if we had written `[expr; 2]` for some expression that is not *literally* a constant but could be evaluated at compile-time (e.g. `(EMPTY,).0`), this would have failed.
We could take back this stabilization as it was clearly accidental. However, I propose we instead just officially accept this and stabilize a small subset of RFC 2203, while leaving the more complex case of general expressions that could be evaluated at compile-time unstable. Making that case work well is pretty much blocked on inline `const` expressions (to avoid relying too much on [implicit promotion](https://github.com/rust-lang/const-eval/blob/master/promotion.md)), so it could take a bit until it comes to full fruition. `[CONST; N]` is an uncontroversial subset of this feature that has no semantic ambiguities, does not rely on promotion, and basically provides the full expressive power of RFC 2203 but without the convenience (people have to define constants to repeat them, possibly using associated consts if generics are involved).
Well, I said "no semantic ambiguities", that is only almost true... the one point I am not sure about is `[CONST; 0]`. There are two possible behaviors here: either this is equivalent to `let x = CONST; [x; 0]`, or it is a NOP (if we argue that the constant is never actually instantiated). The difference between the two is that if `CONST` has a destructor, it should run in the former case (but currently doesn't, due to https://github.com/rust-lang/rust/issues/74836); but should not run if it is considered a NOP. For regular `[x; 0]` there seems to be consensus on running drop (there isn't really an alternative); any opinions for the `CONST` special case? Should this instantiate the const only to immediately run its destructors? That seems somewhat silly to me. After all, the `let`-expansion does *not* work in general, for `N > 1`.
Cc `@rust-lang/lang` `@rust-lang/wg-const-eval`
Cc https://github.com/rust-lang/rust/issues/49147
Make BoundRegion have a kind of BoungRegionKind
Split from #76814
Also includes making `replace_escaping_bound_vars` only return `T`
Going to r? `@lcnr`
Feel free to reassign
Prefer regions with an `external_name` in `approx_universal_upper_bound`
Fixes#75785
When displaying a MIR borrowcheck error, we may need to find an upper
bound for a region, which gives us a region to point to in the error
message. However, a region might outlive multiple distinct universal
regions, in which case the only upper bound is 'static
To try to display a meaningful error message, we compute an
'approximate' upper bound by picking one of the universal regions.
Currently, we pick the region with the lowest index - however, this
caused us to produce a suboptimal error message in issue #75785
This PR `approx_universal_upper_bound` to prefer regions with an
`external_name`. This causes us to prefer regions from function
arguments/upvars, which seems to lead to a nicer error message in some
cases.
Fixes#75785
When displaying a MIR borrowcheck error, we may need to find an upper
bound for a region, which gives us a region to point to in the error
message. However, a region might outlive multiple distinct universal
regions, in which case the only upper bound is 'static
To try to display a meaningful error message, we compute an
'approximate' upper bound by picking one of the universal regions.
Currently, we pick the region with the lowest index - however, this
caused us to produce a suboptimal error message in issue #75785
This PR `approx_universal_upper_bound` to prefer regions with an
`external_name`. This causes us to prefer regions from function
arguments/upvars, which seems to lead to a nicer error message in some
cases.
Move binder for dyn to each list item
This essentially changes `ty::Binder<&'tcx List<ExistentialTraitRef>>` to `&'tcx List<ty::Binder<ExistentialTraitRef>>`.
This is a first step in moving the `dyn Trait` representation closer to Chalk, which we've talked about in `@rust-lang/wg-traits.`
r? `@nikomatsakis`
Always run intrinsics lowering pass
Move intrinsics lowering pass from the optimization phase (where it
would not run if -Zmir-opt-level=0), to the drop lowering phase where it
runs unconditionally.
The implementation of those intrinsics in code generation and
interpreter is unnecessary. Remove it.
Fixed conflict with drop elaboration and coverage
See
https://github.com/rust-lang/rust/issues/80045#issuecomment-745733339
Coverage statements are moved to the beginning of the BCB. This does
also affect what's counted before a panic, changing some results, but I
think these results may even be preferred? In any case, there are no
guarantees about what's counted when a panic occurs (by design).
r? `@tmandry`
FYI `@wesleywiser` `@ecstatic-morse`
Fix issue #78496
EarlyOtherwiseBranch finds MIR structures like:
```
bb0: {
...
_2 = discriminant(X)
...
switchInt(_2) -> [1_isize: bb1, otherwise: bb3]
}
bb1: {
...
_3 = discriminant(Y)
...
switchInt(_3) -> [1_isize: bb2, otherwise: bb3]
}
bb2: {...}
bb3: {...}
```
And transforms them into something like:
```
bb0: {
...
_2 = discriminant(X)
_3 = discriminant(Y)
_4 = Eq(_2, _3)
switchInt(_4) -> [true: bb4, otherwise: bb3]
}
bb2: {...} // unchanged
bb3: {...} // unchanged
bb4: {
switchInt(_2) -> [1_isize: bb2, otherwise: bb3]
}
```
But that is not always a safe thing to do -- sometimes the early `otherwise` branch is necessary so the later block could assume the value of `discriminant(X)`.
I am not totally sure what's the best way to detect that, but fixing #78496 should be easy -- we just check if `X` is a sub-expression of `Y`. A more precise test might be to check if `Y` contains a `Downcast(1)` of `X`, but I think this might be good enough.
Fix#78496
See
https://github.com/rust-lang/rust/issues/80045#issuecomment-745733339
Coverage statements are moved to the beginning of the BCB. This does
also affect what's counted before a panic, changing some results, but I
think these results may even be preferred? In any case, there are no
guarantees about what's counted when a panic occurs (by design).
make MIR graphviz generation use gsgdt
gsgdt [https://crates.io/crates/gsgdt] is a crate which provides an
interface for stringly typed graphs. It also provides generation of
graphviz dot format from said graph.
This is the first in a series of PRs on moving graphviz code out of rustc into normal crates and then implementating graph diffing on top of these crates.
r? `@oli-obk`
Fixes reported bugs in Rust Coverage
Fixes: #79569Fixes: #79566Fixes: #79565
For the first issue (#79569), I got hit a `debug_assert!()` before
encountering the reported error message (because I have `debug = true`
enabled in my config.toml).
The assertion showed me that some `SwitchInt`s can have more than one
target pointing to the same `BasicBlock`.
I had thought that was invalid, but since it seems to be possible, I'm
allowing this now.
I added a new test for this.
----
In the last two cases above, both tests (intentionally) fail to compile,
but the `InstrumentCoverage` pass is invoked anyway.
The MIR starts with an `Unreachable` `BasicBlock`, which I hadn't
encountered before. (I had assumed the `InstrumentCoverage` pass
would only be invoked with MIRs from successful compilations.)
I don't have test infrastructure set up to test coverage on files that
fail to compile, so I didn't add a new test.
r? `@tmandry`
FYI: `@wesleywiser`
consider assignments of union field of ManuallyDrop type safe
Assigning to `Copy` union fields is safe because that assignment will never drop anything. However, with https://github.com/rust-lang/rust/pull/77547, unions may also have `ManuallyDrop` fields, and their assignments are currently still unsafe. That seems unnecessary though, as assigning `ManuallyDrop` does not drop anything either, and is thus safe even for union fields.
I assume this will at least require FCP.
[mir-opt] Allow debuginfo to be generated for a constant or a Place
Prior to this commit, debuginfo was always generated by mapping a name
to a Place. This has the side-effect that `SimplifyLocals` cannot remove
locals that are only used for debuginfo because their other uses have
been const-propagated.
To allow these locals to be removed, we now allow debuginfo to point to
a constant value. The `ConstProp` pass detects when debuginfo points to
a local with a known constant value and replaces it with the value. This
allows the later `SimplifyLocals` pass to remove the local.
Move intrinsics lowering pass from the optimization phase (where it
would not run if -Zmir-opt-level=0), to the drop lowering phase where it
runs unconditionally.
The implementation of those intrinsics in code generation and
interpreter is unnecessary. Remove it.
Adds checks for:
* `no_core` attribute
* explicitly-enabled `legacy` symbol mangling
* mir_opt_level > 1 (which enables inlining)
I removed code from the `Inline` MIR pass that forcibly disabled
inlining if `-Zinstrument-coverage` was set. The default `mir_opt_level`
does not enable inlining anyway. But if the level is explicitly set and
is greater than 1, I issue a warning.
The new warnings show up in tests, which is much better for diagnosing
potential option conflicts in these cases.
Remove an unused dependency that made `rustdoc` crash
Whilst struggling with https://github.com/rust-lang/rust/issues/79980 I discovered that this dependency was unused, and that made rustdoc crash. This PR removes it.
fix more clippy::complexity findings
fix clippy::unnecessary_filter_map
use if let Some(x) = .. instead of ...map(|x|) to conditionally run fns that return () (clippy::option_map_unit_fn)
fix clippy::{needless_bool, manual_unwrap_or}
don't clone types that are copy (clippy::clone_on_copy)
don't convert types into identical types with .into() (clippy::useless_conversion)
use strip_prefix over slicing (clippy::manual_strip)
r? ``@Dylan-DPC``
Fixes: #79569Fixes: #79566Fixes: #79565
For the first issue (#79569), I got hit a `debug_assert!()` before
encountering the reported error message (because I have `debug = true`
enabled in my config.toml).
The assertion showed me that some `SwitchInt`s can have more than one
target pointing to the same `BasicBlock`.
I had thought that was invalid, but since it seems to be possible, I'm
allowing this now.
I added a new test for this.
----
In the last two cases above, both tests (intentionally) fail to compile,
but the `InstrumentCoverage` pass is invoked anyway.
The MIR starts with an `Unreachable` `BasicBlock`, which I hadn't
encountered before. (I had assumed the `InstrumentCoverage` pass
would only be invoked with MIRs from successful compilations.)
I don't have test infrastructure set up to test coverage on files that
fail to compile, so I didn't add a new test.
Capture precise paths in THIR and MIR
This PR allows THIR and MIR to use the result of the new capture analysis to actually capture precise paths
To achieve we:
- Writeback min capture results to TypeckResults
- Move handling upvars to PlaceBuilder in mir_build
- Lower precise paths in THIR build by reading min_captures
- Search for ancestors in min_capture when trying to build a MIR place which starts off of an upvar
Closes: https://github.com/rust-lang/project-rfc-2229/issues/10
Partly implements: rust-lang/project-rfc-2229#18
Work that remains (not in this PR):
- [ ] [Known bugs when feature gate is enabled](https://github.com/rust-lang/project-rfc-2229/projects/1?card_filter_query=label%3Abug)
- [ ] Use min_capure_map for
- [ ] Liveness analysis
- [ ] rustc_mir/interpret/validity.rs
- [ ] regionck
- [ ] rust-lang/project-rfc-2229#8
- [ ] remove closure_captures and upvar_capture_map
r? `@ghost`
CTFE: tweak abort-on-uninhabited message
Having an "aborted execution:" makes it more consistent with the `Abort` terminator saying "the program aborted execution". Right now, at least one of the two errors will look weird in Miri.
r? `@oli-obk`
Dogfood `str_split_once()`
Part of https://github.com/rust-lang/rust/issues/74773.
Beyond increased clarity, this fixes some instances of a common confusion with how `splitn(2)` behaves: the first element will always be `Some()`, regardless of the delimiter, and even if the value is empty.
Given this code:
```rust
fn main() {
let val = "...";
let mut iter = val.splitn(2, '=');
println!("Input: {:?}, first: {:?}, second: {:?}", val, iter.next(), iter.next());
}
```
We get:
```
Input: "no_delimiter", first: Some("no_delimiter"), second: None
Input: "k=v", first: Some("k"), second: Some("v")
Input: "=", first: Some(""), second: Some("")
```
Using `str_split_once()` makes more clear what happens when the delimiter is not found.
Constier maybe uninit
I was playing around trying to make `[T; N]::zip()` in #79451 be `const fn`. One of the things I bumped into was `MaybeUninit::assume_init`. Is there any reason for the intrinsic `assert_inhabited<T>()` and therefore `MaybeUninit::assume_init` not being `const`?
---
I have as best as I could tried to follow the instruction in [library/core/src/intrinsics.rs](https://github.com/rust-lang/rust/blob/master/library/core/src/intrinsics.rs#L11). I have no idea what I am doing but it seems to compile after some slight changes after the copy paste. Is this anywhere near how this should be done?
Also any ideas for name of the feature gate? I guess `const_maybe_assume_init` is quite misleading since I have added some more methods. Should I add test? If so what should be tested?
Fixes to Rust coverage
Fixes: #79725
Some macros can create a situation where `fn_sig_span` and `body_span`
map to different files.
New documentation on coverage tests incorrectly assumed multiple test
binaries could just be listed at the end of the `llvm-cov` command,
but it turns out each binary needs a `--object` prefix.
This PR fixes the bug and updates the documentation to correct that
issue. It also fixes a few other minor issues in internal implementation
comments, and adds documentation on getting coverage results for doc
tests.
minor stylistic clippy cleanups
simplify if let Some(_) = x to if x.is_some() (clippy::redundant_pattern_matching)
don't create owned values for comparison (clippy::cmp_owned)
use .contains() or .any() instead of find(x).is_some() (clippy::search_is_some)
don't wrap code block in Ok() (clipppy::unit_arg)
Also generate `StorageDead` in constants
r? `@eddyb`
None of this special casing is actually necessary since we started promoting within constants and statics.
We may want to keep some of it around out of perf reasons, but it's not required for user visible behaviour
somewhat related: #68622
remove this weird special case from promotion
Promotion has a special case to ignore interior mutability under some specific circumstances. The purpose of this PR is to figure out what changes if we remove that. Since `Cell::new` and friends only get promoted inside `const`/`static` initializers these days, it actually is not easy to exploit this case: you need something like
```rust
const TEST_INTERIOR_MUT: () = {
// The "0." case is already ruled out by not permitting any interior mutability in `const`.
let _val: &'static _ = &(Cell::new(1), 2).1;
};
```
I assume something like `&Some(&(Cell::new(1), 2).1)` would hit the nested case inside `validate_rvalue`... though I am not sure why that would not just trigger nested promotion, first promoting the inner reference and then the outer one?
Fixes https://github.com/rust-lang/rust/issues/67534 (by simply rejecting that code^^)
r? `@oli-obk` (but for now this is not meant to be merged!)
Cc `@rust-lang/wg-const-eval`
Fixes: #79725
Some macros can create a situation where `fn_sig_span` and `body_span`
map to different files.
New documentation on coverage tests incorrectly assumed multiple test
binaries could just be listed at the end of the `llvm-cov` command,
but it turns out each binary needs a `--object` prefix.
This PR fixes the bug and updates the documentation to correct that
issue. It also fixes a few other minor issues in internal implementation
comments, and adds documentation on getting coverage results for doc
tests.
Prior to this commit, debuginfo was always generated by mapping a name
to a Place. This has the side-effect that `SimplifyLocals` cannot remove
locals that are only used for debuginfo because their other uses have
been const-propagated.
To allow these locals to be removed, we now allow debuginfo to point to
a constant value. The `ConstProp` pass detects when debuginfo points to
a local with a known constant value and replaces it with the value. This
allows the later `SimplifyLocals` pass to remove the local.
Added one more test (two files) showing coverage of generics and unused
functions across crates.
Created and referenced new Issues, as requested.
Added comments.
Added a note about the possible effects of compiler options on LLVM
coverage maps.
Fixes multiple issue with counters, with simplification
Includes a change to the implicit else span in ast_lowering, so coverage
of the implicit else no longer spans the `then` block.
Adds coverage for unused closures and async function bodies.
Fixes: #78542
Adding unreachable regions for known MIR missing from coverage map
Cleaned up PR commits, and removed link-dead-code requirement and tests
Coverage no longer depends on Issue #76038 (`-C link-dead-code` is
no longer needed or enforced, so MSVC can use the same tests as
Linux and MacOS now)
Restrict adding unreachable regions to covered files
Improved the code that adds coverage for uncalled functions (with MIR
but not-codegenned) to avoid generating coverage in files not already
included in the files with covered functions.
Resolved last known issue requiring --emit llvm-ir workaround
Fixed bugs in how unreachable code spans were added.
Tested and validate results for panic unwind, panic abort, assert!()
macro, TerminatorKind::Assert (for example, numeric overflow), and
async/await.
Implemented a previous documented idea to change Assert handling to be
the same as FalseUnwind and Goto, so it doesn't get its own
BasicCoverageBlock anymore. This changed a couple of coverage regions,
but I validated those changes are not any worse than the prior results,
and probably help assure some consistency (even if some people might
disagree with how the code region is consistently computed).
Fixed issue with async/await. AggregateKind::Generator needs to be
handled like AggregateKind::Closure; coverage span for the outer async
function should not "cover" the async body, which is actually executed
in a separate "closure" MIR.
Properly handle attributes on statements
We now collect tokens for the underlying node wrapped by `StmtKind`
nstead of storing tokens directly in `Stmt`.
`LazyTokenStream` now supports capturing a trailing semicolon after it
is initially constructed. This allows us to avoid refactoring statement
parsing to wrap the parsing of the semicolon in `parse_tokens`.
Attributes on item statements
(e.g. `fn foo() { #[bar] struct MyStruct; }`) are now treated as
item attributes, not statement attributes, which is consistent with how
we handle attributes on other kinds of statements. The feature-gating
code is adjusted so that proc-macro attributes are still allowed on item
statements on stable.
Two built-in macros (`#[global_allocator]` and `#[test]`) needed to be
adjusted to support being passed `Annotatable::Stmt`.
Allow using generic trait methods in `const fn`
Next step for https://github.com/rust-lang/rust/issues/67792, this now also allows code like the following:
```rust
struct S;
impl const PartialEq for S {
fn eq(&self, _: &S) -> bool {
true
}
}
const fn equals_self<T: PartialEq>(t: &T) -> bool {
*t == *t
}
pub const EQ: bool = equals_self(&S);
```
This works by threading const-ness of trait predicates through trait selection, in particular through `ParamCandidate`, and exposing it in the resulting `ImplSource`.
Since this change makes two bounds `T: Trait` and `T: ?const Trait` that only differ in their const-ness be treated like different bounds, candidate winnowing has been changed to drop the `?const` candidate in favor of the const candidate, to avoid ambiguities when both a const and a non-const bound is present.
Never inline naked functions
The `#[naked]` attribute disabled prologue / epilogue emission for the
function and it is responsibility of a developer to provide them. The
compiler is no position to inline such functions correctly.
Disable inlining of naked functions at LLVM and MIR level.
Closes#60919.
The `#[naked]` attribute disabled prologue / epilogue emission for the
function and it is responsibility of a developer to provide them. The
compiler is no position to inline such functions correctly.
Disable inlining of naked functions at LLVM and MIR level.
add optimization fuel checks to some mir passes
Fixes#77402
Inserts a bunch of calls to `consider_optimizing`. Note that `consider_optimizing` is the method that actually decrements the fuel count, so the point at which it's called is when the optimization takes place, from a fuel perspective. This means that where we call it has some thought behind it:
1. We probably don't want to decrement the fuel count before other simple checks, otherwise we count an optimization as being performed even if nothing was mutated (ie. it returned early).
2. In cases like `InstCombine`, where we gather optimizations in a pass and then mutate values, we probably would rather skip the gathering pass for performance reasons rather than skip the mutations afterwards.
Introduce `TypeVisitor::BreakTy`
Implements MCP rust-lang/compiler-team#383.
r? `@ghost`
cc `@lcnr` `@oli-obk`
~~Blocked on FCP in rust-lang/compiler-team#383.~~
RFC-2229: Implement Precise Capture Analysis
### This PR introduces
- Feature gate for RFC-2229 (incomplete) `capture_disjoint_field`
- Rustc Attribute to print out the capture analysis `rustc_capture_analysis`
- Precise capture analysis
### Description of the analysis
1. If the feature gate is not set then all variables that are not local to the closure will be added to the list of captures. (This is for backcompat)
2. The rest of the analysis is based entirely on how the captured `Place`s are used within the closure. Precise information (i.e. projections) about the `Place` is maintained throughout.
3. To reduce the amount of information we need to keep track of, we do a minimization step. In this step, we determine a list such that no Place within this list represents an ancestor path to another entry in the list. Check rust-lang/project-rfc-2229#9 for more detailed examples.
4. To keep the compiler functional as before we implement a Bridge between the results of this new analysis to existing data structures used for closure captures. Note the new capture analysis results are only part of MaybeTypeckTables that is the information is only available during typeck-ing.
### Known issues
- Statements like `let _ = x` will make the compiler ICE when used within a closure with the feature enabled. More generally speaking the issue is caused by `let` statements that create no bindings and are init'ed using a Place expression.
### Testing
We removed the code that would handle the case where the feature gate is not set, to enable the feature as default and did a bors try and perf run. More information here: #78762
### Thanks
This has been slowly in the works for a while now.
I want to call out `@Azhng` `@ChrisPardy` `@null-sleep` `@jenniferwills` `@logmosier` `@roxelo` for working on this and the previous PRs that led up to this, `@nikomatsakis` for guiding us.
Closesrust-lang/project-rfc-2229#7Closesrust-lang/project-rfc-2229#9Closesrust-lang/project-rfc-2229#6Closesrust-lang/project-rfc-2229#19
r? `@nikomatsakis`
Allow making `RUSTC_BOOTSTRAP` conditional on the crate name
Motivation: This came up in the [Zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Require.20users.20to.20confirm.20they.20know.20RUSTC_.E2.80.A6.20compiler-team.23350/near/208403962) for https://github.com/rust-lang/compiler-team/issues/350.
See also https://github.com/rust-lang/cargo/pull/6608#issuecomment-458546258; this implements https://github.com/rust-lang/cargo/issues/6627.
The goal is for this to eventually allow prohibiting setting `RUSTC_BOOTSTRAP` in build.rs (https://github.com/rust-lang/cargo/issues/7088).
## User-facing changes
- `RUSTC_BOOTSTRAP=1` still works; there is no current plan to remove this.
- Things like `RUSTC_BOOTSTRAP=0` no longer activate nightly features. In practice this shouldn't be a big deal, since `RUSTC_BOOTSTRAP` is the opposite of stable and everyone uses `RUSTC_BOOTSTRAP=1` anyway.
- `RUSTC_BOOTSTRAP=x` will enable nightly features only for crate `x`.
- `RUSTC_BOOTSTRAP=x,y` will enable nightly features only for crates `x` and `y`.
## Implementation changes
The main change is that `UnstableOptions::from_environment` now requires
an (optional) crate name. If the crate name is unknown (`None`), then the new feature is not available and you still have to use `RUSTC_BOOTSTRAP=1`. In practice this means the feature is only available for `--crate-name`, not for `#![crate_name]`; I'm interested in supporting the second but I'm not sure how.
Other major changes:
- Added `Session::is_nightly_build()`, which uses the `crate_name` of
the session
- Added `nightly_options::match_is_nightly_build`, a convenience method
for looking up `--crate-name` from CLI arguments.
`Session::is_nightly_build()`should be preferred where possible, since
it will take into account `#![crate_name]` (I think).
- Added `unstable_features` to `rustdoc::RenderOptions`
I'm not sure whether this counts as T-compiler or T-lang; _technically_ RUSTC_BOOTSTRAP is an implementation detail, but it's been used so much it seems like this counts as a language change too.
r? `@joshtriplett`
cc `@Mark-Simulacrum` `@hsivonen`
Normalize function type during validation
During inlining, the callee body is normalized and has types revealed,
but some of locals corresponding to the arguments might come from the
caller body which is not. As a result the caller body does not pass
validation without additional normalization.
Closes#78442.
Lower intrinsics calls: forget, size_of, unreachable, wrapping_*
This allows constant propagation to evaluate `size_of` and `wrapping_*`,
and unreachable propagation to propagate a call to `unreachable`.
The lowering is performed as a MIR optimization, rather than during MIR
building to preserve the special status of intrinsics with respect to
unsafety checks and promotion.
Currently enabled by default to determine the performance impact (no
significant impact expected). In practice only useful when combined with
inlining since intrinsics are rarely used directly (with exception of
`unreachable` and `discriminant_value` used by built-in derive macros).
Closes#32716.
This allows constant propagation to evaluate `size_of` and `wrapping_*`,
and unreachable propagation to propagate a call to `unreachable`.
The lowering is performed as a MIR optimization, rather than during MIR
building to preserve the special status of intrinsics with respect to
unsafety checks and promotion.
During inlining, the callee body is normalized and has types revealed,
but some of locals corresponding to the arguments might come from the
caller body which is not. As a result the caller body does not pass
validation without additional normalization.
The inliner looks if a sanitizer is enabled before considering
`no_sanitize` attribute as possible source of incompatibility.
The MIR inlining could happen in a crate with sanitizer disabled, but
code generation in a crate with sanitizer enabled, thus the attribute
would be incorrectly ignored.
To avoid the issue never inline functions with different `no_sanitize`
attributes.
Support inlining diverging function calls
The existing heuristic does penalize diverging calls to some degree, but since
it never inlined them previously it might need some further modifications.
Additionally introduce storage markers for all temporaries created by
the inliner. The temporary introduced for destination rebrorrow, didn't
use them previously.
Add flags customizing behaviour of MIR inlining
* `-Zinline-mir-threshold` to change the default threshold.
* `-Zinline-mir-hint-threshold` to change the threshold used by
functions with inline hint.
Having those as configurable flags makes it possible to experiment with with
different inlining thresholds and substantially increase test coverage of MIR
inlining when used with increased thresholds (for example, necessary to test
#78844).
The discussion seems to have resolved that this lint is a bit "noisy" in
that applying it in all places would result in a reduction in
readability.
A few of the trivial functions (like `Path::new`) are fine to leave
outside of closures.
The general rule seems to be that anything that is obviously an
allocation (`Box`, `Vec`, `vec![]`) should be in a closure, even if it
is a 0-sized allocation.
Add comments to explain memory usage optimization
Add explanatory comments so that people understand that it's just an optimization and doesn't affect behavior.
rustc_target: Further cleanup use of target options
Follow up to https://github.com/rust-lang/rust/pull/77729.
Implements items 2 and 4 from the list in https://github.com/rust-lang/rust/pull/77729#issue-500228243.
The first commit collapses uses of `target.options.foo` into `target.foo`.
The second commit renames some target options to avoid tautology:
`target.target_endian` -> `target.endian`
`target.target_c_int_width` -> `target.c_int_width`
`target.target_os` -> `target.os`
`target.target_env` -> `target.env`
`target.target_vendor` -> `target.vendor`
`target.target_family` -> `target.os_family`
`target.target_mcount` -> `target.mcount`
r? `@Mark-Simulacrum`
Assert that a return place is not used for indexing during integration
The inliner integrates call destination place with callee return place
by remapping the local and adding extra projections as necessary.
If a call destination place contains any projections (which is already
possible) and a return place is used in an indexing projection (most
likely doesn't happen yet) the end result would be incorrect.
Add an assertion to ensure that potential issue won't go unnoticed in
the presence of more sophisticated copy propagation scheme.
* `-Zinline-mir-threshold` to change the default threshold.
* `-Zinline-mir-hint-threshold` to change the threshold used by
functions with inline hint.
inliner: Break inlining cycles
Keep track of all instances inlined so far. When examining a new call
sites from an inlined body, skip those where callee had been inlined
already to avoid potential inlining cycles.
Fixes#78573.
Improve lifetime name annotations for closures & async functions
* Don't refer to async functions as "generators" in error output
* Where possible, emit annotations pointing exactly at the `&` in the return type of closures (when they have explicit return types) and async functions, like we do for arguments.
Addresses #74072, but I wouldn't call that *closed* until annotations are identical for async and non-async functions.
* Emit a better annotation when the lifetime doesn't appear in the full name type, which currently happens for opaque types like `impl Future`. Addresses #74497, but further improves could probably be made (why *doesn't* it appear in the type as `impl Future + '1`?)
This is included in the same PR because the changes to `give_name_if_anonymous_region_appears_in_output` would introduce ICE otherwise (it would return `None` in cases where it didn't previously, which then gets `unwrap`ped)
gsgdt [https://crates.io/crates/gsgdt] is a crate which provides an
interface for stringly typed graphs. It also provides generation of
graphviz dot format from said graph.
inliner: Use substs_for_mir_body
Changes from 68965 extended the kind of instances that are being
inlined. For some of those, the `instance_mir` returns a MIR body that
is already expressed in terms of the types found in substitution array,
and doesn't need further substitution.
Use `substs_for_mir_body` to take that into account.
Resolves#78529.
Resolves#78560.
Additionally introduce storage markers for all temporaries created by
the inliner. The temporary introduced for destination rebrorrow, didn't
use them previously.
When examining candidates for inlining, reject those that are determined
to be recursive either because of self-recursive calls or calls to any
instances already inlined.
with an eye on merging `TargetOptions` into `Target`.
`TargetOptions` as a separate structure is mostly an implementation detail of `Target` construction, all its fields logically belong to `Target` and available from `Target` through `Deref` impls.
Rollup of 19 pull requests
Successful merges:
- #76097 (Stabilize hint::spin_loop)
- #76227 (Stabilize `Poll::is_ready` and `is_pending` as const)
- #78065 (make concurrency helper more pleasant to read)
- #78570 (Remove FIXME comment in print_type_sizes ui test suite)
- #78572 (Use SOCK_CLOEXEC and accept4() on more platforms.)
- #78658 (Add a tool to run `x.py` from any subdirectory)
- #78706 (Fix run-make tests running when LLVM is disabled)
- #78728 (Constantify `UnsafeCell::into_inner` and related)
- #78775 (Bump Rustfmt and RLS)
- #78788 (Correct unsigned equivalent of isize to be usize)
- #78811 (Make some std::io functions `const`)
- #78828 (use single char patterns for split() (clippy::single_char_pattern))
- #78841 (Small cleanup in `TypeFoldable` derive macro)
- #78842 (Honor the rustfmt setting in config.toml)
- #78843 (Less verbose debug logging from inlining integrator)
- #78852 (Convert a bunch of intra-doc links)
- #78860 (rustc_resolve: Use `#![feature(format_args_capture)]`)
- #78861 (typo and formatting)
- #78865 (Don't fire `CONST_ITEM_MUTATION` lint when borrowing a deref)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Don't fire `CONST_ITEM_MUTATION` lint when borrowing a deref
Fixes#78819
This extends the check for dereferences added in PR #77324
to cover mutable borrows, as well as direct writes. If we're operating
on a dereference of a `const` item, we shouldn't be firing the lint.
Less verbose debug logging from inlining integrator
The inlining integrator produces relatively verbose and uninteresting
logs. Move them from a debug log level to a trace level, so that they
can be easily isolated from others.
revert #75443, update mir validator
This PR reverts rust-lang#75443 to fix rust-lang#75992 and instead uses rust-lang#75419 to fix rust-lang#75313.
Adapts rust-lang#75419 to correctly deal with unevaluated constants as otherwise some `feature(const_evaluatable_checked)` tests would ICE.
Note that rust-lang#72793 was also fixed by rust-lang#75443, but as that issue only concerns `feature(type_alias_impl_trait)` I deleted that test case for now and would reopen that issue.
rust-lang#75443 may have also allowed some other code to now successfully compile which would make this revert a breaking change after 2 stable versions, but I hope that this is a purely theoretical concern.
See https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/generator.20upvars/near/214617274 for more reasoning about this.
r? `@nikomatsakis` `@eddyb` `@RalfJung`
Fixes#78819
This extends the check for dereferences added in PR #77324
to cover mutable borrows, as well as direct writes. If we're operating
on a dereference of a `const` item, we shouldn't be firing the lint.
The inlining integrator produces relatively verbose and uninteresting
logs. Move them from a debug log level to a trace level, so that they
can be easily isolated from others.
The main change is that `UnstableOptions::from_environment` now requires
an (optional) crate name. If the crate name is unknown (`None`), then the new feature is not available and you still have to use `RUSTC_BOOTSTRAP=1`. In practice this means the feature is only available for `--crate-name`, not for `#![crate_name]`; I'm interested in supporting the second but I'm not sure how.
Other major changes:
- Added `Session::is_nightly_build()`, which uses the `crate_name` of
the session
- Added `nightly_options::match_is_nightly_build`, a convenience method
for looking up `--crate-name` from CLI arguments.
`Session::is_nightly_build()`should be preferred where possible, since
it will take into account `#![crate_name]` (I think).
- Added `unstable_features` to `rustdoc::RenderOptions`
There is a user-facing change here: things like `RUSTC_BOOTSTRAP=0` no
longer active nightly features. In practice this shouldn't be a big
deal, since `RUSTC_BOOTSTRAP` is the opposite of stable and everyone
uses `RUSTC_BOOTSTRAP=1` anyway.
- Add tests
Check against `Cheat`, not whether nightly features are allowed.
Nightly features are always allowed on the nightly channel.
- Only call `is_nightly_build()` once within a function
- Use booleans consistently for rustc_incremental
Sessions can't be passed through threads, so `read_file` couldn't take a
session. To be consistent, also take a boolean in `write_file_header`.
