Rollup of 10 pull requests #100595
Merged
Rollup of 10 pull requests #100595
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
Having to replace - with _ (and vice versa) makes the slugs less greppable
and thus constitutes a contributor roadblock.
Result of running this repeatedly up until reaching a fixpoint:
find compiler/rustc_error_messages/locales/en-US/ -type f -exec sed -i 's/\(.+\)-\(.*\)=/\1_\2=/' {} \;
Plus some fixes to update usages of slugs leading with -.
For the most part, the macro actually worked with _ slugs, but the prefix_something -> prefix::something conversion was not implemented. We don't want to accept - slugs for consistency reasons. We thus error if a name is found with - inside. This ensures a consistent style.
…ect due to other malformed impls
These use the same names as LLVM and is_riscv_feature_detected!: - zba (address generation instructions) - zbb (basic bit manipulation) - zbc (carry-less multiplication) - zbs (single-bit manipulation)
Let-chaining avoids some code duplication.
`Parser::parse_bottom_expr` currently constructs an empty `attrs` and then passes it to a large number of other functions. This makes the code harder to read than it should be, because it's not clear that many `attrs` arguments are always empty. This commit removes `attrs` and the passing, simplifying a lot of functions. The commit also renames `Parser::mk_expr` (which takes an `attrs` argument) as `mk_expr_with_attrs`, and introduces a new `mk_expr` which creates an expression with no attributes, which is the more common case.
…fest and rustc_session"" This reverts commit 1ae4b25.
Fixes a bug where impl of items that were imported from a private module would be striped Fixes rust-lang#100252 Fixes rust-lang#100242
… r=michaelwoerister improve "try ignoring the field" diagnostic Closes rust-lang#95795
…uillaumeGomez Rustdoc-Json: Don't remove impls for items imported from private modules After rust-lang#99287, items in private modules may still be in the json output, if a public import accesses them. To reflect this, items that are imported need to be marked as retained in the `Stripper` pass, so their impls arn't removed by `ImplStripper`. [More context on zulip](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Populating.20cache.2Eimpls), thanks to @ jyn514 for helping debug this. ``@rustbot`` modify labels: +A-rustdoc-json +T-rustdoc r? ``@GuillaumeGomez`` Fixes rust-lang#100252 Fixes rust-lang#100242
Replace - with _ in fluent slugs to improve developer workflows This is a proposal to smoothen the compiler contribution experience in the face of the move to fluent. ## Context The fluent project has introduced a layer of abstraction to compiler errors. Previously, people would write down error messages directly in the same file the code was located to emit them. Now, there is a slug that connects the code in the compiler to the error message in the ftl file. You can look at 7ef610c to see an example of the changes: Old: ```Rust let msg = format!( "bounds on `{}` are most likely incorrect, consider instead \ using `{}` to detect whether a type can be trivially dropped", predicate, cx.tcx.def_path_str(needs_drop) ); lint.build(&msg).emit(); ``` New (Rust side): ```Rust lint.build(fluent::lint::drop_trait_constraints) .set_arg("predicate", predicate) .set_arg("needs_drop", cx.tcx.def_path_str(needs_drop)) .emit(); ``` New (Fluent side): ```fluent lint-drop-trait-constraints = bounds on `{$predicate}` are most likely incorrect, consider instead using `{$needs_drop}` to detect whether a type can be trivially dropped ``` You will note that in the ftl file, the slug is slightly different from the slug in the Rust file: The ftl slug uses `-` (e.g. `lint-drop-trait-constraints`) while the rust slug uses `::` and `_` (e.g. `lint::drop_trait_constraints`). This choice was probably done due to: * Rust not accepting `-` in identifiers (as it is an operator) * fluent not supporting the `:` character in slug names (parse error upon attempts) * all official fluent documentation using `-` instead of `_` ## The problem The two different types of slugs, one with `-`, and one with `_`, cause difficulties for contributors. Imagine you don't have perfect knowledge of where stuff is in the compiler (i would say this is most people), and you encounter an error for which you think there is something you could improve that is not just a rewording. So you want to find out where in the compiler's code that error is being emitted. The best way is via grepping. 1. you grep for the message in the compiler's source code. You discover the ftl file and find out the slug for that error. 2. That slug however contains `-` instead of `_`, so you have to manually translate the `-`'s into `_`s, and furthermore either remove the leading module name, or replace the first `-` with a `::`. 3. you do a second grep to get to the emitting location in the compiler's code. This translation difficulty in step 2 appears also in the other direction when you want to figure out what some code in the compiler is doing and use error messages to help your understanding. Comments and variable names are way less exposed to users so [are more likely going to lie](rust-lang@cc3c5d2) than error messages. I think that at least the `-`→`_` translation which makes up most of step 2 can be removed at low cost. ## The solution If you look closely, the practice of fluent to use `-` is only a stylistic choice and it is not enforced by fluent implementations, neither the playground nor the one the rust compiler uses, that slugs may not contain `_`. Thus, we can in fact migrate the ftl side to `_`. So now we'll have slugs like `lint_drop_trait_constraints` on the ftl side. You only have to do one replacement now to get to the Rust slug: remove the first `_` and place a `::` in its stead. I would argue that this change is in fact useful as it allows you to control whether you want to look at the rust side of things or the ftl side of things via changing the query string only: with an increased number of translations checked into the repository, grepping for raw slugs will return the slug in many ftl files, so an explicit step to look for the source code is always useful. In the other direction (rust to fluent), you don't need a translation at all any more, as you can just take the final piece of the slug (e.g. `drop_trait_constraints`) and grep for that. The PR also adds enforcement to forbid usage of `_` in slug names. Internal slug names (those leading with a `-`) are exempt from that enforcement. As another workflow that benefits from this change, people who add new errors don't have to do that `-` conversion either. | Before/After | Fluent slug | Rust slug (no change) | |--|--|--| | Before | `lint-drop-trait-constraints` | `lint::drop_trait_constraints`| | After | `lint_drop_trait_constraints` | `lint::drop_trait_constraints`| Note that I've suggested this previously in the translation thread on zulip. I think it's important to think about non-translator contribution impact of fluent. I have certainly plans for more improvements, but this is a good first step. ``@rustbot`` label A-diagnostics
…r=estebank
Adjust span of fn argument declaration
Span of a fn argument declaration goes from:
```
fn foo(i : i32 , ...)
^^^^^^^^
```
to:
```
fn foo(i : i32 , ...)
^^^^^^^
```
That is, we don't include the extra spacing up to the trailing comma, which I think is more correct.
cc rust-lang#99646 (comment)
r? ``@estebank``
---
The two tests that had dramatic changes in their rendering I think actually are improved, though they are kinda poor spans both before and after the changes. 🤷 Thoughts?
…astorino Delay span bug when failing to normalize negative coherence impl subject due to other malformed impls Fixes rust-lang#100191 r? ``@spastorino``
…vidtwco Support 1st group of RISC-V Bitmanip backend target features These target features use the same names as LLVM and `is_riscv_feature_detected!`, they are: - zba (address generation instructions) - zbb (basic bit manipulation) - zbc (carry-less multiplication) - zbs (single-bit manipulation) The extension is frozen and ratified, and I don't think we should expect LLVM to change those feature names in the future. For reference, the specification for the B extension can be found here: https://github.com/riscv/riscv-bitmanip/releases/download/1.0.0/bitmanip-1.0.0-38-g865e7a7.pdf) On my current project, I see a 7.6% reduction in binary size with these features on, so I have some incentive to try to silence the "unknown feature" warning from `-Ctarget-feature` =)
…, r=compiler-errors Parser simplifications Best reviewed one commit at a time. r? ``@compiler-errors``
Fix STD build for ESP-IDF We have accidentally broken the STD build for the Tier 3 `target_os="espidf"` (only) by pushing non-buildable changes to `libc` which ended up in version 0.2.229. `libc` [was fixed](rust-lang/libc@d0e3ff0) from V0.2.230 onwards. This PR is only upgrading the `libc` dependency in `Cargo.lock` to latest (V0.2.231). `Cargo.lock` was modified by using `cargo update -p libc`.
…ed-enum-variant, r=notriddle [rustdoc] Fix handling of stripped enum variant in JSON output format Fixes rust-lang#100529. cc ``@aDotInTheVoid`` ``@Enselic`` r? ``@notriddle``
…=jyn514 Reland changes replacing num_cpus with available_parallelism Since rust-lang#97925 added cgroupv1 support the problem in rust-lang#97549 which lead to the previous revert should be addressed now. Cargo has reapplied the replacement too rust-lang/cargo#10969 Reverts 1ae4b25 (part of rust-lang#97911) Relands rust-lang#94524
rustbot
added
T-bootstrap
|
@bors r+ rollup=never p=10 |
bors
added
the
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
📌 Perf builds for each rolled up PR: In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
This was referenced Aug 15, 2022
|
Finished benchmarking commit (4033686): comparison url. Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results
CyclesThis benchmark run did not return any relevant results for this metric. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
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:
Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup