Eliminate some other bound checks when index comes from an enum
#36962 introduced an assumption for the upper limit of the enum's value. This PR adds an assumption to the lower value as well.
I've modified the original codegen test to show that derived (in that case, adding 1) values also don't generate bounds checks.
However, this test is actually carefully crafted to not hit a bug: if the enum's variants are modified to 1 and 2 instead of 2 and 3, the test fails by adding a bounds check. I suppose this is an LLVM issue and #75525, while not exactly in this context should be tracking it.
I'm not at all confident if this patch can be accepted, or even if it _should_ be accepted in this state. But I'm curious about what others think :)
~Improves~ Should improve #13926 but does not close it because it's not exactly predictable, where bounds checks may pop up against the assumptions.
Previously, this would say no such macro existed, but this was
misleading, since the macro _did_ exist, it was just already seen.
- Say where the macro was previously defined
- Add long-form error message
Make to_immediate/from_immediate configurable by backends
`librustc_codegen_ssa` has the concept of an immediate vs. memory type, and `librustc_codegen_llvm` uses this distinction to implement `bool`s being `i8` in memory, and `i1` in immediate contexts. However, some of that implementation leaked into `codegen_ssa` when converting to/from immediate values. So, move those methods into builder traits, so that behavior can be configured by backends.
This is useful if a backend is able to keep bools as bools, or, needs to do more trickery than just bools to bytes.
(Note that there's already a large amount of things abstracted with "immediate types" - this is just bringing this particular thing in line to be abstracted as well)
---
Pinging @eddyb since that's who I was talking about this change with when they suggested I submit a PR.
Similar to `-Z dump-mir-graphviz`, this adds the option to write
HTML+CSS files that allow users to analyze the spans associated with MIR
elements (by individual statement, just terminator, or overall basic
block).
This PR was split out from PR #76004, and exposes an API for spanview
HTML+CSS files that is also used to analyze code regions chosen for
coverage instrumentation (in a follow-on PR).
Rust compiler MCP rust-lang/compiler-team#278
Relevant issue: #34701 - Implement support for LLVMs code coverage
instrumentation
Update expect-test to 1.0
The only change is that `expect_file` now uses path relative to the
current file (same as `include!`). Before, it used paths relative to
the workspace root, which makes no sense.
Adds two source span utility functions used in source-based coverage
`span.is_empty()` - returns true if `lo()` and `hi()` are equal. This is
not only a convenience, but makes it clear that a `Span` can be empty
(that is, retrieving the source for an empty `Span` will return an empty
string), and codifies the (otherwise undocumented--in the rustc_span
package, at least) fact that `Span` is a half-open interval (where
`hi()` is the open end).
`source_map.lookup_file_span()` - returns an enclosing `Span`
representing the start and end positions of the file enclosing the given
`BytePos`. This gives developers a clear way to quickly determine if any
any other `BytePos` or `Span` is also from the same file (for example,
by simply calling `file_span.contains(span)`).
This results in much simpler code and is much more runtime efficient
compared with the obvious alternative: calling `source_map.lookup_line()`
for any two `Span`'s byte positions, handle both arms of the `Result`
(both contain the file), and then compare files. It is also more
efficient than the non-public method `lookup_source_file_idx()` for each
`BytePos`, because, while comparing the internal source file indexes
would be efficient, looking up the source file index for every `BytePos`
or `Span` to be compared requires a binary search (worst case
performance being O(log n) for every lookup).
`source_map.lookup_file_span()` performs the binary search only once, to
get the `file_span` result that can be used to compare to any number of
other `BytePos` or `Span` values and those comparisons are always O(1).
This PR was split out from PR #75828 .
r? @tmandry
FYI: @wesleywiser
Fix `-Z instrument-coverage` on MSVC
Found that `-C link-dead-code` (which was enabled automatically
under `-Z instrument-coverage`) was causing the linking error that
resulted in segmentation faults in coverage instrumented binaries. Link
dead code is now disabled under MSVC, allowing `-Z instrument-coverage`
to be enabled under MSVC for the first time.
More details are included in Issue #76038 .
Note this PR makes it possible to support `Z instrument-coverage` but
does not enable instrument coverage for MSVC in existing tests. It will be
enabled in another PR to follow this one (both PRs coming from original
PR #75828).
r? @tmandry
FYI: @wesleywiser
`span.is_empty()` - returns true if `lo()` and `hi()` are equal. This is
not only a convenience, but makes it clear that a `Span` can be empty
(that is, retrieving the source for an empty `Span` will return an empty
string), and codifies the (otherwise undocumented--in the rustc_span
package, at least) fact that `Span` is a half-open interval (where
`hi()` is the open end).
`source_map.lookup_file_span()` - returns an enclosing `Span`
representing the start and end positions of the file enclosing the given
`BytePos`. This gives developers a clear way to quickly determine if any
any other `BytePos` or `Span` is also from the same file (for example,
by simply calling `file_span.contains(span)`).
This results in much simpler code and is much more runtime efficient
compared with the obvious alternative: calling `source_map.lookup_line()`
for any two `Span`'s byte positions, handle both arms of the `Result`
(both contain the file), and then compare files. It is also more
efficient than the non-public method `lookup_source_file_idx()` for each
`BytePos`, because, while comparing the internal source file indexes
would be efficient, looking up the source file index for every `BytePos`
or `Span` to be compared requires a binary search (worst case
performance being O(log n) for every lookup).
`source_map.lookup_file_span()` performs the binary search only once, to
get the `file_span` result that can be used to compare to any number of
other `BytePos` or `Span` values and those comparisons are always O(1).
Found that -C link-dead-code (which was enabled automatically
under -Z instrument-coverage) was causing the linking error that
resulted in segmentation faults in coverage instrumented binaries. Link
dead code is now disabled under MSVC, allowing `-Z instrument-coverage`
to be enabled under MSVC for the first time.
More details are included in Issue #76038.
(This PR was broken out from PR #75828)
The only change is that `expect_file` now uses path relative to the
current file (same as `include!`). Before, it used paths relative to
the workspace root, which makes no sense.
cg_llvm: `fewer_names` in `uncached_llvm_type`
This PR changes `uncached_llvm_type` so that a named struct type (with an empty name) is always created when the `fewer_names` option is enabled. By skipping the generation of names, we can improve perf. Giving `LLVMStructCreateNamed` an empty name works because LLVM will perform random renames to avoid collisions. Needs a perf run!
cc @eddyb (whom I've discussed this with)
Restore public visibility on some parsing functions for rustfmt
In #74826 the visibility of several parsing functions was reduced. However, rustfmt is an external consumer of some of these functions as well and needs the visibility to be public, similar to other elements in rustc_parse such as `parse_ident`
db534b3ac2/src/librustc_parse/parser/mod.rs (L433-L436)
This commit changes `uncached_llvm_type` so that a named struct type
(with an empty name) is always created when the `fewer_names` option
is enabled. By skipping the generation of names, we can improve perf.
Giving `LLVMStructCreateNamed` an empty name works because LLVM will
perform random renames to avoid collisions.
Signed-off-by: David Wood <david@davidtw.co>
ty: remove obsolete pretty printer
Fixes#61139.
This PR removes the obsolete printer and replaces all uses of it with `FmtPrinter`. Of the replaced uses, all but one use was in `debug!` logging, two cases were notable:
- `MonoItem::to_string` is used in `-Z print-mono-items` and therefore affects the output of all codegen-units tests (which have been updated).
- `DefPathBasedNames` was used in `librustc_codegen_llvm/type_of.rs` with `LLVMStructCreateNamed` and that'll now get different values, but nothing will break as a result of this.
cc @eddyb (whom I've discussed this with)
Add `-Z proc-macro-backtrace` to allow showing proc-macro panics
Fixes#75050
Previously, we would unconditionally suppress the panic hook during
proc-macro execution. This commit adds a new flag
`-Z proc-macro-backtrace`, which allows running the panic hook for
easier debugging.
Fixes#75050
Previously, we would unconditionally suppress the panic hook during
proc-macro execution. This commit adds a new flag
-Z proc-macro-backtrace, which allows running the panic hook for
easier debugging.
Previous implementation used the `Parser::parse_expr` function in order
to extract the format expression. If the first comma following the
format expression was mistakenly replaced with a dot, then the next
format expression was eaten by the function, because it looked as a
syntactically valid expression, which resulted in incorrectly spanned
error messages.
The way the format expression is exctracted is changed: we first look at
the first available token in the first argument supplied to the
`format!` macro call. If it is a string literal, then it is promoted as
a format expression immediatly, otherwise we fall back to the original
`parse_expr`-related method.
This allows us to ensure that the parser won't consume too much tokens
when a typo is made.
A test has been created so that it is ensured that the issue is properly
fixed.
I've tried a few ways of implementing this, but each fell short.
Adding an auxiliary `_Idx` associated type to `Analysis` that defaults
to `!` but is overridden in the blanket impl of `Analysis` for `A:
GenKillAnalysis` to `A::Idx` seems promising, but the trait solver is
unable to prove equivalence between `A::Idx` and `A::_Idx` within the
overridden version of `into_engine`. Without full-featured
specialization, removing `into_engine` or splitting it into a different
trait would have a significant ergonomic penalty.
Alternatively, we could erase the index type and store a
`GenKillSet<u32>` as well as a function pointer for transmuting between
`&mut A::Domain` and `&mut BitSet<u32>` in the hopes that LLVM can
devirtualize a simple function pointer better than the boxed closure.
However, this is brittle, requires `unsafe` code, and doesn't work for
index types that aren't the same size as a `u32` (e.g. `usize`) since
`GenKillSet` stores a `HybridBitSet`, which may be a `Vec<I>`. Perhaps
safe transmute could help here?
A few small cleanups to `BitSet` and friends:
- Overload `clone_from` for `BitSet`.
- Improve `Debug` represenation of `HybridBitSet`.
- Make `HybridBitSet::domain_size` public.
- Don't require `T: Idx` at the type level. The `Idx` bound is still on
most `BitSet` methods, but like `HashMap`, it doesn't need to be
satisfied for the type to exist.
This commit removes the obsolete printer and replaces all uses of it
with `FmtPrinter`. Of the replaced uses, all but one use was in `debug!`
logging, two cases were notable:
- `MonoItem::to_string` is used in `-Z print-mono-items` and therefore
affects the output of all codegen-units tests.
- `DefPathBasedNames` was used in `librustc_codegen_llvm/type_of.rs`
with `LLVMStructCreateNamed` and that'll now get different values, but
this should result in no functional change.
Signed-off-by: David Wood <david@davidtw.co>
This commit moves `transparent_newtype_field` and `is_zst` to
`LateContext` where they are used, rather than being on the `VariantDef`
and `TyS` types.
Signed-off-by: David Wood <david@davidtw.co>
StringReader is an intornal abstraction which at the moment changes a
lot, so these unit tests cause quite a bit of friction.
Moving them to rustc_lexer and more ingerated-testing style should
make them much less annoying, hopefully without decreasing their
usefulness much.
Note that coloncolon tests are removed (it's unclear what those are
testing).
\r\n tests are removed as well, as we normalize line endings even
before lexing.
Fixes#75930
This changes the tokens seen by a proc-macro. However, ising a `#[cfg]` attribute
on a generic paramter is unusual, and combining it with a proc-macro
derive is probably even more unusual. I don't expect this to cause any
breakage.
