Rollup of 5 pull requests #102586
Merged
Rollup of 5 pull requests #102586
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
Rust's test library allows test functions to return a Result, so that the test is deemed to have failed if the function returns a Result::Err variant. Currently, this works by having Result implement the Termination trait and asserting in assert_test_result that Termination::report() indicates successful completion. This turns a Result::Err into a panic, which is caught and unwound in the test library. This approach is problematic in certain environments where one wishes to save on both binary size and compute resources when running tests by: * Compiling all code with --panic=abort to avoid having to generate unwinding tables, and * Running most tests in-process to avoid the overhead of spawning new processes. This change removes the intermediate panic step and passes a Result::Err directly through to the test runner. To do this, it modifies assert_test_result to return a Result<(), String> where the Err variant holds what was previously the panic message. It changes the types in the TestFn enum to return Result<(), String>. This tries to minimise the changes to benchmark tests, so it calls unwrap() on the Result returned by assert_test_result, effectively keeping the same behaviour as before.
…test, r=Mark-Simulacrum Do not panic when a test function returns Result::Err. Rust's test library allows test functions to return a `Result`, so that the test is deemed to have failed if the function returns a `Result::Err` variant. Currently, this works by having `Result` implement the `Termination` trait and asserting in assert_test_result that `Termination::report()` indicates successful completion. This turns a `Result::Err` into a panic, which is caught and unwound in the test library. This approach is problematic in certain environments where one wishes to save on both binary size and compute resources when running tests by: * Compiling all code with `--panic=abort` to avoid having to generate unwinding tables, and * Running most tests in-process to avoid the overhead of spawning new processes. This change removes the intermediate panic step and passes a `Result::Err` directly through to the test runner. To do this, it modifies `assert_test_result` to return a `Result<(), String>` where the `Err` variant holds what was previously the panic message. It changes the types in the `TestFn` enum to return `Result<(), String>`. This tries to minimise the changes to benchmark tests, so it calls `unwrap()` on the `Result` returned by `assert_test_result`, effectively keeping the same behaviour as before. Some questions for reviewers: * Does the change to the return types in the enum `TestFn` constitute a breaking change for the library API? Namely, the enum definition is public but the test library indicates that "Currently, not much of this is meant for users" and most of the library API appears to be marked unstable. * Is there a way to test this change, i.e., to test that no panic occurs if a test returns `Result::Err`? * Is there a shorter, more idiomatic way to fold `Result<Result<T,E>,E>` into a `Result<T,E>` than the `fold_err` function I added?
…Mark-Simulacrum Use fetch_update in sync::Weak::upgrade Using `fetch_update` makes it more clear that it's CAS loop then manually implementing one.
Give `def_span` the same SyntaxContext as `span_with_body`. rust-lang#102217 I'm not sure how to add a test, since the erroneous span was crafted using a proc macro. The debug assertion in `def_span` will ensure we have the correct behaviour.
… r=Mark-Simulacrum Make `feature(const_btree_len)` implied by `feature(const_btree_new)` ...this should fix code that used the old feature that was changed in rust-lang#102197 cc ```@davidtwco``` it seems like tidy doesn't check `implied_by`, should it?
…k-Simulacrum Add a known-bug test for rust-lang#102498 Self-explanatory
rustbot
added
T-compiler
|
@bors r+ rollup=never p=5 |
bors
added
the
S-waiting-on-bors
|
ye i know i hit esc for the other 2 because i needed to change something but it was too late :P |
|
☀️ Test successful - checks-actions |
|
📌 Perf builds for each rolled up PR: previous master: 91931ec2fc In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
This was referenced Oct 2, 2022
|
Finished benchmarking commit (39323a5): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Footnotes |
Aaron1011
pushed a commit
to Aaron1011/rust
that referenced
this pull request
Jan 6, 2023
Rollup of 5 pull requests Successful merges: - rust-lang#100451 (Do not panic when a test function returns Result::Err.) - rust-lang#102098 (Use fetch_update in sync::Weak::upgrade) - rust-lang#102538 (Give `def_span` the same SyntaxContext as `span_with_body`.) - rust-lang#102556 (Make `feature(const_btree_len)` implied by `feature(const_btree_new)`) - rust-lang#102566 (Add a known-bug test for rust-lang#102498) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
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:
def_spanthe same SyntaxContext asspan_with_body. #102538 (Givedef_spanthe same SyntaxContext asspan_with_body.)feature(const_btree_len)implied byfeature(const_btree_new)#102556 (Makefeature(const_btree_len)implied byfeature(const_btree_new))~constbounded types should not be allowed in generic const exprs #102498)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup