-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rollup of 7 pull requests #133682
Closed
Closed
Rollup of 7 pull requests #133682
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
Cargo's -Zbuild-std has recently started checking this field, which causes it to fail to compile even though we have full support for the standard library on these targets.
Signed-off-by: onur-ozkan <[email protected]>
…ck, r=RalfJung use stores of the correct size to set discriminants Resolves an old HACK /FIXME. Note that I haven't worked much with codegen so I'm not sure if I'm using the functions correctly and I was surprised seeing out-of-range values being fed into `const_uint_big` but apparently they're wrapped implicitly? By making it explicit we can pass in-range values instead.
Stabilize unsigned and float variants of `num_midpoint` feature This PR proposes that we stabilize the unsigned variants of the [`num_midpoint`](rust-lang#110840 (comment)) feature as well as the floats variants, since they are not subject to any unresolved questions, which is equivalent to doing `(a + b) / 2` (and `(a + b) >> 1`) in a sufficiently large number. The stabilized API surface would be: ```rust /// Calculates the middle point of `self` and `rhs`. /// /// `midpoint(a, b)` is `(a + b) / 2` as if it were performed in a sufficiently-large unsigned integral type. /// This implies that the result is always rounded towards negative infinity and that no overflow will ever occur. impl u{8,16,32,64,128,size} { pub const fn midpoint(self, rhs: Self) -> Self; } impl NonZeroU{8,16,32,64,size} { pub const fn midpoint(self, rhs: Self) -> Self; } impl f{32,64} { pub const fn midpoint(self, rhs: Self) -> Self; } ``` The signed variants `u{8,16,32,64,128,size}` would remain gated, until a decision is made about the rounding mode, in other words that the [unresolved questions](rust-lang#110840 (comment)) are resolved. cc `@rust-lang/libs-api` cc `@scottmcm` r? libs-api
…ratrieb Mark visionOS as supporting `std` Cargo's -Zbuild-std has recently started checking this field, which causes it to fail to compile even though we have full support for the standard library on these targets. [Example of failed build](https://github.com/rust-random/getrandom/actions/runs/12069033154/job/33655430622). Affected targets: `aarch64-apple-visionos` and `aarch64-apple-visionos-sim`. r? Noratrieb (because you've worked with `rustc` target metadata IIRC) `@rustbot` label O-visionos
Eliminate print_expr_maybe_paren function from pretty printers This PR is part of backporting Syn's expression precedence design into rustc. (See rust-lang#133603 for other work on this.) In Syn, our version of `print_expr_cond_paren` is called `print_subexpression` and it is called from 19 places. Of those calls, 12 of them need a "custom" behavior for the `needs_paren` argument, whereas only 7 use a "standard" behavior resembling `print_subexpression($e, $e.precedence() < Precedence::$Variant, ...)`. In other words the behavior that rustc_ast_pretty's `print_expr_maybe_paren` implements is actually not what you want most of the time. The current usage you see in rustc is overuse. <details> <summary>Aside: am I confident about the correctness of Syn's parenthesization? Yes. Click for details.</summary> --- The behavior is constrained by the following pair of tests which both run over every Rust source file of rustc and the standard library and tools and test suites: - To rule out **false positives**: for every expression in every source file, print the expression, parse it back, and verify that not a single new parenthesis got added. Since these are expressions parsed from source code, not macro-generated syntax trees, we know they must never need automatic parenthesis insertion. Rustc's pretty printer does not pass this. Pseudocode: `assert(expr == parse(print(expr)))` - To rule out **false negatives**: for every expression in every source file, replace every Expr::Paren node in the syntax tree with just its contents, i.e. stripping the parentheses but otherwise preserving the syntax tree structure. Then print the stripped expression performing parenthesis insertion wherever needed, and reparse it. Verify that the reparsed expression has identical structure to the original, despite there being no parentheses in the original prior to printing, i.e. all the right parentheses got re-inserted by the printer to preserve the expression's structure. Rustc's pretty printer does not pass this. See dtolnay/syn#1788 which reveals multiple rustc_ast_pretty bugs. Pseudocode: `assert(unparenthesize(expr) == unparenthesize(parse(print(unparenthesize(expr)))))` --- </details> If `print_expr_maybe_paren` is usually not correct, is there harm in keeping it for the minority of cases where it is correct? I think the answer is yes and Syn doesn't use any equivalent of this helper function. The problems with it are: - Having both `print_expr_maybe_paren` and `print_expr_cond_paren` applies counterproductive inertia against moving from the first to the second. When looking at a call site like `print_expr_maybe_paren(e, Precedence::$Variant, ...)` with parentheses not being inserted where they should be, anyone's first inclination would be to solve the bug by tweaking $Variant because that is the only knob that visibly appears in the function call. For example to pass "prec + 1", like tweaking the code to conditionally pass `Precedence::Prefix` instead of `Precedence::Cast`. Experience in Syn shows this is (almost?) never what you want the person to do. In a call `print_expr_cond_paren(e, e.precedence() < ExprPrecedence::$Variant, ...)` almost always the best fix involves one of: - Changing `e.precedence()`, e.g. to `fixup.leading_precedence(e)` and `fixup.trailing_precedence(e)` in cases of asymmetrical precedence (`(return 1) + 1` vs `1 + return 1`). - Changing `<` to `<=`, to handle associativity and other grammar restrictions like chained comparisons (which rustc gets wrong today). - Adding `||` and/or `&&` clauses to the condition. By using these 3 better knobs instead of $Variant, it upholds the property that any time we talk about precedence, it is always the precedence of some actual expression that our code is actively manipulating, instead of a value standing in for some imaginary precedence level that would exist between two consecutive [real levels](https://doc.rust-lang.org/1.83.0/reference/expressions.html#expression-precedence). For example consider that "`Cast` + 1" might be `Prefix` today, but only until some new Rust syntax ends up adding a level between those. - The `print_expr_maybe_paren` call sites look shorter, but they are not clearer. For myself, a function argument that says "does this subexpression need parenthesization" is a concrete thing that is easy to think about, while a function argument that is "what is the effective precedence level associated with this subexpression's placement inside its parent expression" is abstract and tricky to even state a precise meaning for. I expect that for someone less familiar with the pretty printer working on adding a new expression kind (like postfix match, recently), having every subexpression consistently printed using `print_expr_cond_paren` will be more beneficial, for the same reason, than having `print_expr_maybe_paren` available. r? `@lcnr`
…e, r=clubby789 Remove unused code
bump hashbrown version This pulls in rust-lang/hashbrown#586, in preparation for rust-lang#102575. Cc `@Amanieu`
replace hard coded error id with `ErrorKind::DirectoryNotEmpty` Resolves an internal bootstrap FIXME.
rustbot
added
A-run-make
Area: port run-make Makefiles to rmake.rs
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
T-bootstrap
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
T-libs
Relevant to the library team, which will review and decide on the PR/issue.
rollup
A PR which is a rollup
labels
Nov 30, 2024
@bors r+ rollup=never p=5 |
bors
added
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
Nov 30, 2024
The job Click to see the possible cause of the failure (guessed by this bot)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-run-make
Area: port run-make Makefiles to rmake.rs
rollup
A PR which is a rollup
S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
T-bootstrap
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
T-libs
Relevant to the library team, which will review and decide on the PR/issue.
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:
num_midpoint
feature #131784 (Stabilize unsigned and float variants ofnum_midpoint
feature)std
#133571 (Mark visionOS as supportingstd
)ErrorKind::DirectoryNotEmpty
#133673 (replace hard coded error id withErrorKind::DirectoryNotEmpty
)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup