-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Tweak parameter mismatch explanation to not say {unknown}
#133430
Conversation
compiler/rustc_errors/src/lib.rs
Outdated
@@ -1995,6 +1995,17 @@ pub fn display_list_with_comma_and<T: std::fmt::Display>(v: &[T]) -> String { | |||
} | |||
} | |||
|
|||
pub fn ordinalize(x: usize) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We moved away from ordinalizing via suffix in #125105 and #125042 preferring "#n
" over "nth
". #125105 had previously removed fn ordinalize
: https://github.com/rust-lang/rust/pull/125105/files#diff-0e459b4cbb41dd93274d7c68b18a2a3eab7152e9aea107839a7bddff0b4e9195L3031
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i feel like it's kinda strange that an arbitrary pr gets to set that precedent for how we present diagnostics but ok i guess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I get that. I don't even remember why this change was done.
We can decide to go back to nth over #n.
I just want to avoid inconsistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If y'all decide to go with nth, I'll add an item to my backlog to migrate the other places back to it from #n.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5134a04 says this:
Some minor (English only) heroics are performed to print error messages
like "5th rule of macrom
is never used". The form "rule#5
of macro
m
is never used" is just as good and much simpler to implement.
I'm not sure I agree? For me, #5
is harder to read than 5-th
. And the function that implemented this is like 10 lines, so I'm really unsure what is the win here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we'd have a style guide for that, but I'm way to exhausted to start a zulip discussion /_=
either way is arguably fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, seems like a strange thing to justify as a cleanup when it's more like a style change. But whatever, I really don't want to litigate this; I just want to fix a blatant diagnostics shortcoming. So I'll fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @nnethercote
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not have a discussion on this PR. If you want to discuss this, do it elsewhere :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r? WaffleLapkin
r=me with green CI
78fa5a1
to
d26e29f
Compare
@bors r=WaffleLapkin |
@bors rollup |
…WaffleLapkin Tweak parameter mismatch explanation to not say `{unknown}` * Tweak parameter mismatch explanation not to call parameters with no identifier `{unknown}` * Say "both" when there are two parameters * Backtick a type parameter name for consistency
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#132605 (CI: increase timeout from 4h to 6h) - rust-lang#133042 (btree: add `{Entry,VacantEntry}::insert_entry`) - rust-lang#133070 (Lexer tweaks) - rust-lang#133134 (Don't use a SyntheticProvider for literally every type) - rust-lang#133411 (the emscripten OS no longer exists on non-wasm targets) - rust-lang#133419 (Added a doc test for std::path::strip_prefix) - rust-lang#133430 (Tweak parameter mismatch explanation to not say `{unknown}`) - rust-lang#133435 (miri: disable test_downgrade_observe test on macOS) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#132605 (CI: increase timeout from 4h to 6h) - rust-lang#133042 (btree: add `{Entry,VacantEntry}::insert_entry`) - rust-lang#133070 (Lexer tweaks) - rust-lang#133134 (Don't use a SyntheticProvider for literally every type) - rust-lang#133411 (the emscripten OS no longer exists on non-wasm targets) - rust-lang#133419 (Added a doc test for std::path::strip_prefix) - rust-lang#133430 (Tweak parameter mismatch explanation to not say `{unknown}`) - rust-lang#133435 (miri: disable test_downgrade_observe test on macOS) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#132605 (CI: increase timeout from 4h to 6h) - rust-lang#133042 (btree: add `{Entry,VacantEntry}::insert_entry`) - rust-lang#133070 (Lexer tweaks) - rust-lang#133134 (Don't use a SyntheticProvider for literally every type) - rust-lang#133411 (the emscripten OS no longer exists on non-wasm targets) - rust-lang#133419 (Added a doc test for std::path::strip_prefix) - rust-lang#133430 (Tweak parameter mismatch explanation to not say `{unknown}`) - rust-lang#133435 (miri: disable test_downgrade_observe test on macOS) r? `@ghost` `@rustbot` modify labels: rollup
…WaffleLapkin Tweak parameter mismatch explanation to not say `{unknown}` * Tweak parameter mismatch explanation not to call parameters with no identifier `{unknown}` * Say "both" when there are two parameters * Backtick a type parameter name for consistency
Rollup of 28 pull requests Successful merges: - rust-lang#132605 (CI: increase timeout from 4h to 6h) - rust-lang#133042 (btree: add `{Entry,VacantEntry}::insert_entry`) - rust-lang#133070 (Lexer tweaks) - rust-lang#133136 (Support ranges in `<[T]>::get_many_mut()`) - rust-lang#133140 (Inline ExprPrecedence::order into Expr::precedence) - rust-lang#133248 (CI: split x86_64-msvc-ext job) - rust-lang#133282 (Shorten the `MaybeUninit` `Debug` implementation) - rust-lang#133304 (Revert diagnostics hack to fix ICE 132920) - rust-lang#133326 (Remove the `DefinitelyInitializedPlaces` analysis.) - rust-lang#133362 (No need to re-sort existential preds in relate impl) - rust-lang#133367 (Simplify array length mismatch error reporting (to not try to turn consts into target usizes)) - rust-lang#133394 (Bail on more errors in dyn ty lowering) - rust-lang#133410 (target check_consistency: ensure target feature string makes some basic sense) - rust-lang#133411 (the emscripten OS no longer exists on non-wasm targets) - rust-lang#133419 (Added a doc test for std::path::strip_prefix) - rust-lang#133430 (Tweak parameter mismatch explanation to not say `{unknown}`) - rust-lang#133435 (miri: disable test_downgrade_observe test on macOS) - rust-lang#133443 (Remove dead code stemming from the old effects desugaring (II)) - rust-lang#133449 (std: expose `const_io_error!` as `const_error!`) - rust-lang#133450 (remove "onur-ozkan" from users_on_vacation) - rust-lang#133454 (Update test expectations to accept LLVM 'initializes' attribute) - rust-lang#133458 (Fix `Result` and `Option` not getting a jump to def link generated) - rust-lang#133462 (Use ReadCache for archive reading in bootstrap) - rust-lang#133464 (std::thread: avoid leading whitespace in some panic messages) - rust-lang#133467 (tests: Add recursive associated type bound regression tests) - rust-lang#133470 (Cleanup: delete `//@ pretty-expanded` directive) - rust-lang#133473 (tests: Add regression test for recursive enum with Cow and Clone) - rust-lang#133481 (Disable `avr-rjmp-offset` on Windows for now) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 28 pull requests Successful merges: - rust-lang#132605 (CI: increase timeout from 4h to 6h) - rust-lang#133042 (btree: add `{Entry,VacantEntry}::insert_entry`) - rust-lang#133070 (Lexer tweaks) - rust-lang#133136 (Support ranges in `<[T]>::get_many_mut()`) - rust-lang#133140 (Inline ExprPrecedence::order into Expr::precedence) - rust-lang#133248 (CI: split x86_64-msvc-ext job) - rust-lang#133282 (Shorten the `MaybeUninit` `Debug` implementation) - rust-lang#133304 (Revert diagnostics hack to fix ICE 132920) - rust-lang#133326 (Remove the `DefinitelyInitializedPlaces` analysis.) - rust-lang#133362 (No need to re-sort existential preds in relate impl) - rust-lang#133367 (Simplify array length mismatch error reporting (to not try to turn consts into target usizes)) - rust-lang#133394 (Bail on more errors in dyn ty lowering) - rust-lang#133410 (target check_consistency: ensure target feature string makes some basic sense) - rust-lang#133411 (the emscripten OS no longer exists on non-wasm targets) - rust-lang#133419 (Added a doc test for std::path::strip_prefix) - rust-lang#133430 (Tweak parameter mismatch explanation to not say `{unknown}`) - rust-lang#133435 (miri: disable test_downgrade_observe test on macOS) - rust-lang#133443 (Remove dead code stemming from the old effects desugaring (II)) - rust-lang#133449 (std: expose `const_io_error!` as `const_error!`) - rust-lang#133450 (remove "onur-ozkan" from users_on_vacation) - rust-lang#133454 (Update test expectations to accept LLVM 'initializes' attribute) - rust-lang#133458 (Fix `Result` and `Option` not getting a jump to def link generated) - rust-lang#133462 (Use ReadCache for archive reading in bootstrap) - rust-lang#133464 (std::thread: avoid leading whitespace in some panic messages) - rust-lang#133467 (tests: Add recursive associated type bound regression tests) - rust-lang#133470 (Cleanup: delete `//@ pretty-expanded` directive) - rust-lang#133473 (tests: Add regression test for recursive enum with Cow and Clone) - rust-lang#133481 (Disable `avr-rjmp-offset` on Windows for now) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 28 pull requests Successful merges: - rust-lang#132605 (CI: increase timeout from 4h to 6h) - rust-lang#133042 (btree: add `{Entry,VacantEntry}::insert_entry`) - rust-lang#133070 (Lexer tweaks) - rust-lang#133136 (Support ranges in `<[T]>::get_many_mut()`) - rust-lang#133140 (Inline ExprPrecedence::order into Expr::precedence) - rust-lang#133248 (CI: split x86_64-msvc-ext job) - rust-lang#133282 (Shorten the `MaybeUninit` `Debug` implementation) - rust-lang#133304 (Revert diagnostics hack to fix ICE 132920) - rust-lang#133326 (Remove the `DefinitelyInitializedPlaces` analysis.) - rust-lang#133362 (No need to re-sort existential preds in relate impl) - rust-lang#133367 (Simplify array length mismatch error reporting (to not try to turn consts into target usizes)) - rust-lang#133394 (Bail on more errors in dyn ty lowering) - rust-lang#133410 (target check_consistency: ensure target feature string makes some basic sense) - rust-lang#133411 (the emscripten OS no longer exists on non-wasm targets) - rust-lang#133419 (Added a doc test for std::path::strip_prefix) - rust-lang#133430 (Tweak parameter mismatch explanation to not say `{unknown}`) - rust-lang#133435 (miri: disable test_downgrade_observe test on macOS) - rust-lang#133443 (Remove dead code stemming from the old effects desugaring (II)) - rust-lang#133449 (std: expose `const_io_error!` as `const_error!`) - rust-lang#133450 (remove "onur-ozkan" from users_on_vacation) - rust-lang#133454 (Update test expectations to accept LLVM 'initializes' attribute) - rust-lang#133458 (Fix `Result` and `Option` not getting a jump to def link generated) - rust-lang#133462 (Use ReadCache for archive reading in bootstrap) - rust-lang#133464 (std::thread: avoid leading whitespace in some panic messages) - rust-lang#133467 (tests: Add recursive associated type bound regression tests) - rust-lang#133470 (Cleanup: delete `//@ pretty-expanded` directive) - rust-lang#133473 (tests: Add regression test for recursive enum with Cow and Clone) - rust-lang#133481 (Disable `avr-rjmp-offset` on Windows for now) r? `@ghost` `@rustbot` modify labels: rollup
…llaumeGomez Rollup of 13 pull requests Successful merges: - rust-lang#133411 (the emscripten OS no longer exists on non-wasm targets) - rust-lang#133419 (Added a doc test for std::path::strip_prefix) - rust-lang#133430 (Tweak parameter mismatch explanation to not say `{unknown}`) - rust-lang#133443 (Remove dead code stemming from the old effects desugaring (II)) - rust-lang#133450 (remove "onur-ozkan" from users_on_vacation) - rust-lang#133454 (Update test expectations to accept LLVM 'initializes' attribute) - rust-lang#133462 (Use ReadCache for archive reading in bootstrap) - rust-lang#133464 (std::thread: avoid leading whitespace in some panic messages) - rust-lang#133467 (tests: Add recursive associated type bound regression tests) - rust-lang#133470 (Cleanup: delete `//@ pretty-expanded` directive) - rust-lang#133473 (tests: Add regression test for recursive enum with Cow and Clone) - rust-lang#133481 (Disable `avr-rjmp-offset` on Windows for now) - rust-lang#133495 (add test for alias-bound shadowing, rename folder) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#133430 - compiler-errors:param-mismatch, r=WaffleLapkin Tweak parameter mismatch explanation to not say `{unknown}` * Tweak parameter mismatch explanation not to call parameters with no identifier `{unknown}` * Say "both" when there are two parameters * Backtick a type parameter name for consistency
{unknown}