Skip to content
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

Use SourceMap::end_point instead of - BytePos(1) in arg removal suggestion #128864

Merged
merged 2 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use rustc_middle::ty::{self, IsSuggestable, Ty, TyCtxt};
use rustc_middle::{bug, span_bug};
use rustc_session::Session;
use rustc_span::symbol::{kw, Ident};
use rustc_span::{sym, BytePos, Span, DUMMY_SP};
use rustc_span::{sym, Span, DUMMY_SP};
use rustc_trait_selection::error_reporting::infer::{FailureCode, ObligationCauseExt};
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::{self, ObligationCauseCode, SelectionContext};
Expand Down Expand Up @@ -1140,8 +1140,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.get(arg_idx + 1)
.map(|&(_, sp)| sp)
.unwrap_or_else(|| {
// Subtract one to move before `)`
call_expr.span.with_lo(call_expr.span.hi() - BytePos(1))
// Try to move before `)`. Note that `)` here is not necessarily
// the latin right paren, it could be a Unicode-confusable that
// looks like a `)`, so we must not use `- BytePos(1)`
// manipulations here.
self.tcx().sess.source_map().end_point(call_expr.span)
});

// Include next comma
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/typeck/suggest-arg-comma-delete-ice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//! Previously, we tried to remove extra arg commas when providing extra arg removal suggestions.
//! One of the edge cases is having to account for an arg that has a closing delimiter `)`
//! following it. However, the previous suggestion code assumed that the delimiter is in fact
//! exactly the 1-byte `)` character. This assumption was proven incorrect, because we recover
//! from Unicode-confusable delimiters in the parser, which means that the ending delimiter could be
//! a multi-byte codepoint that looks *like* a `)`. Subtracing 1 byte could land us in the middle of
//! a codepoint, triggering a codepoint boundary assertion.
Comment on lines +1 to +7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

//!
//! issue: rust-lang/rust#128717

fn main() {
// The following example has been modified from #128717 to remove irrelevant Unicode as they do
// not otherwise partake in the right delimiter calculation causing the codepoint boundary
// assertion.
main(rahh);
//~^ ERROR unknown start of token
//~| ERROR this function takes 0 arguments but 1 argument was supplied
//~| ERROR cannot find value `rahh` in this scope
}
38 changes: 38 additions & 0 deletions tests/ui/typeck/suggest-arg-comma-delete-ice.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
error: unknown start of token: \u{ff09}
--> $DIR/suggest-arg-comma-delete-ice.rs:15:14
|
LL | main(rahh);
| ^^
|
help: Unicode character ')' (Fullwidth Right Parenthesis) looks like ')' (Right Parenthesis), but it is not
|
LL | main(rahh);
| ~

error[E0425]: cannot find value `rahh` in this scope
--> $DIR/suggest-arg-comma-delete-ice.rs:15:10
|
LL | main(rahh);
| ^^^^ not found in this scope

error[E0061]: this function takes 0 arguments but 1 argument was supplied
--> $DIR/suggest-arg-comma-delete-ice.rs:15:5
|
LL | main(rahh);
| ^^^^ ---- unexpected argument
|
note: function defined here
--> $DIR/suggest-arg-comma-delete-ice.rs:11:4
|
LL | fn main() {
| ^^^^
help: remove the extra argument
|
LL - main(rahh);
LL + main();
Comment on lines +31 to +32
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little bit funny, but oh well

|

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0061, E0425.
For more information about an error, try `rustc --explain E0061`.
Loading