Skip to content

Avoid empty extra-arg suggestion spans in macro calls #152634

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.

Sign up for GitHub

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

Open
TaKO8Ki wants to merge 2 commits into
base: main
Choose a base branch
from
+211 −29
Open

Avoid empty extra-arg suggestion spans in macro calls #152634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb04a68
avoid empty extra-arg suggestion spans in macro calls
TaKO8Ki Feb 14, 2026
5590dec
remove i31762 crashtest
TaKO8Ki Feb 14, 2026
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
49 changes: 30 additions & 19 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2313,7 +2313,10 @@ impl<'a, 'b, 'tcx> FnCallDiagCtxt<'a, 'b, 'tcx> {
.get(ProvidedIdx::from_usize(arg_idx.index() - 1))
{
// Include previous comma
span = prev.shrink_to_hi().to(span);
let prev_comma = prev.shrink_to_hi();
if prev_comma.eq_ctxt(span) {
span = prev_comma.to(span);
}
}

// Is last argument for deletion in a row starting from the 0-th argument?
Expand Down Expand Up @@ -2345,24 +2348,32 @@ impl<'a, 'b, 'tcx> FnCallDiagCtxt<'a, 'b, 'tcx> {
};

if trim_next_comma {
let next = self
.provided_arg_tys
.get(*arg_idx + 1)
.map(|&(_, sp)| sp)
.unwrap_or_else(|| {
// 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.arg_matching_ctxt
.tcx()
.sess
.source_map()
.end_point(self.call_expr.span)
});

// Include next comma
span = span.until(next);
let next =
self.provided_arg_tys.get(*arg_idx + 1).map(|&(_, sp)| sp).or_else(
|| {
// 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.call_expr.span.find_ancestor_in_same_ctxt(span).map(
|call_span| {
self.arg_matching_ctxt
.tcx()
.sess
.source_map()
.end_point(call_span)
},
)
},
);

if let Some(next) = next
// Avoid `Span::until` context fallback producing unrelated spans.
&& span.eq_ctxt(next)
{
// Include next comma
span = span.until(next);
}
}

suggestions.push((span, String::new()));
Expand Down
10 changes: 0 additions & 10 deletions tests/crashes/131762.rs
View file
Open in desktop

This file was deleted.

19 changes: 19 additions & 0 deletions tests/ui/typeck/generic-assert-extra-args-empty-span-suggestion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Regression test for issue #131762.

#![feature(generic_assert)]

struct FloatWrapper(f64);

fn main() {
assert!(
(0.0 / 0.0 >= 0.0)
== (FloatWrapper(0.0 / 0.0)
>= FloatWrapper( //~ ERROR binary operation `>=` cannot be applied to type `FloatWrapper`
//~| ERROR this struct takes 1 argument but 3 arguments were supplied
//~| ERROR this struct takes 1 argument but 3 arguments were supplied
size_of::<u8>,
size_of::<u16>,
size_of::<usize> as fn() -> usize,
))
);
}
103 changes: 103 additions & 0 deletions tests/ui/typeck/generic-assert-extra-args-empty-span-suggestion.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
error[E0061]: this struct takes 1 argument but 3 arguments were supplied
--> $DIR/generic-assert-extra-args-empty-span-suggestion.rs:11:20
|
LL | >= FloatWrapper(
| ^^^^^^^^^^^^
...
LL | size_of::<u16>,
| -------------- unexpected argument #2 of type `fn() -> usize {std::mem::size_of::<u16>}`
LL | size_of::<usize> as fn() -> usize,
| --------------------------------- unexpected argument #3 of type `fn() -> usize`
|
note: expected `f64`, found fn item
--> $DIR/generic-assert-extra-args-empty-span-suggestion.rs:8:5
|
LL | / assert!(
LL | | (0.0 / 0.0 >= 0.0)
LL | | == (FloatWrapper(0.0 / 0.0)
LL | | >= FloatWrapper(
... |
LL | | ))
LL | | );
| |_____^
= note: expected type `f64`
found fn item `fn() -> usize {std::mem::size_of::<u8>}`
note: tuple struct defined here
--> $DIR/generic-assert-extra-args-empty-span-suggestion.rs:5:8
|
LL | struct FloatWrapper(f64);
| ^^^^^^^^^^^^
help: remove the extra arguments
|
LL - size_of::<u16>,
LL - size_of::<usize> as fn() -> usize,
LL + ,
|

error[E0061]: this struct takes 1 argument but 3 arguments were supplied
--> $DIR/generic-assert-extra-args-empty-span-suggestion.rs:11:20
|
LL | >= FloatWrapper(
| ^^^^^^^^^^^^
...
LL | size_of::<u16>,
| -------------- unexpected argument #2 of type `fn() -> usize {std::mem::size_of::<u16>}`
LL | size_of::<usize> as fn() -> usize,
| --------------------------------- unexpected argument #3 of type `fn() -> usize`
|
note: expected `f64`, found fn item
--> $DIR/generic-assert-extra-args-empty-span-suggestion.rs:8:5
|
LL | / assert!(
LL | | (0.0 / 0.0 >= 0.0)
LL | | == (FloatWrapper(0.0 / 0.0)
LL | | >= FloatWrapper(
... |
LL | | ))
LL | | );
| |_____^
= note: expected type `f64`
found fn item `fn() -> usize {std::mem::size_of::<u8>}`
note: tuple struct defined here
--> $DIR/generic-assert-extra-args-empty-span-suggestion.rs:5:8
|
LL | struct FloatWrapper(f64);
| ^^^^^^^^^^^^
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: remove the extra arguments
|
LL - size_of::<u16>,
LL - size_of::<usize> as fn() -> usize,
LL + ,
|

error[E0369]: binary operation `>=` cannot be applied to type `FloatWrapper`
--> $DIR/generic-assert-extra-args-empty-span-suggestion.rs:11:17
|
LL | == (FloatWrapper(0.0 / 0.0)
| ----------------------- FloatWrapper
LL | >= FloatWrapper(
| _________________^^_-
LL | |
LL | |
LL | | size_of::<u8>,
LL | | size_of::<u16>,
LL | | size_of::<usize> as fn() -> usize,
LL | | ))
| |_________________- FloatWrapper
|
note: an implementation of `PartialOrd` might be missing for `FloatWrapper`
--> $DIR/generic-assert-extra-args-empty-span-suggestion.rs:5:1
|
LL | struct FloatWrapper(f64);
| ^^^^^^^^^^^^^^^^^^^ must implement `PartialOrd`
help: consider annotating `FloatWrapper` with `#[derive(PartialEq, PartialOrd)]`
|
LL + #[derive(PartialEq, PartialOrd)]
LL | struct FloatWrapper(f64);
|

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0061, E0369.
For more information about an error, try `rustc --explain E0061`.
14 changes: 14 additions & 0 deletions tests/ui/typeck/suggestions/suggest-extra-arg-empty-span-ice.rs
View file
Open in desktop

@fmease fmease Mar 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not an MCVE, it contains a lot of distracting elements and leads to distracting errors. Could you please use a more distilled version of the reproducer like:

#![feature(generic_assert)]

fn func(value: usize) {
    assert!(core::mem::size_of(value, 1) >= 1);
}

which only generates 1 error instead of 5.

@fmease fmease Mar 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

However note that this reproducer still relies on implementation details of generic_assert. Ideally, we'd define our own macro to be used in this test file. This way, if the implementation of generic_assert changes, this test won't accidentally & likely silently stop testing the right thing...

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Regression test for https://github.com/rust-lang/rust/issues/152414

#![feature(generic_assert)]
pub const fn make_1u8_bag<T: Copy>() -> BagOfBits<_> {
//~^ ERROR cannot find type `BagOfBits` in this scope
//~| ERROR the placeholder `_` is not allowed within types on item signatures for return types
assert!(core::mem::size_of::<T>(val, 1) >= 1);
//~^ ERROR cannot find value `val` in this scope
//~| ERROR this function takes 0 arguments but 2 arguments were supplied
bag
//~^ ERROR cannot find value `bag` in this scope
}

fn main() {}
45 changes: 45 additions & 0 deletions tests/ui/typeck/suggestions/suggest-extra-arg-empty-span-ice.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
error[E0425]: cannot find type `BagOfBits` in this scope
--> $DIR/suggest-extra-arg-empty-span-ice.rs:4:41
|
LL | pub const fn make_1u8_bag<T: Copy>() -> BagOfBits<_> {
| ^^^^^^^^^ not found in this scope

error[E0425]: cannot find value `val` in this scope
--> $DIR/suggest-extra-arg-empty-span-ice.rs:7:37
|
LL | assert!(core::mem::size_of::<T>(val, 1) >= 1);
| ^^^ not found in this scope

error[E0425]: cannot find value `bag` in this scope
--> $DIR/suggest-extra-arg-empty-span-ice.rs:10:5
|
LL | bag
| ^^^ not found in this scope

error[E0061]: this function takes 0 arguments but 2 arguments were supplied
--> $DIR/suggest-extra-arg-empty-span-ice.rs:7:13
|
LL | assert!(core::mem::size_of::<T>(val, 1) >= 1);
| --------^^^^^^^^^^^^^^^^^^^^^^^--------------
| | |
| | unexpected argument #2 of type `{integer}`
| unexpected argument #1
|
note: function defined here
--> $SRC_DIR/core/src/mem/mod.rs:LL:COL
help: remove the extra argument
|
LL - assert!(core::mem::size_of::<T>(val, 1) >= 1);
LL + assert!(core::mem::size_of::<T>(val, ) >= 1);

@fmease fmease Mar 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The new suggestion isn't quite right though. It should either suggest removing both extra arguments or if that's not possible due to differing syntax contexts we should err on the safe side and just not suggest anything (unless you can make it work).

The diagnostic knows that 2 args are extra but the suggestion no longer does for some reason.

@fmease fmease Mar 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens if the source code was assert!(core::mem::size_of(value,)); does rustc now suggest assert!(core::mem::size_of(,)); under your PR? I don't actually know.

|

error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types
--> $DIR/suggest-extra-arg-empty-span-ice.rs:4:51
|
LL | pub const fn make_1u8_bag<T: Copy>() -> BagOfBits<_> {
| ^ not allowed in type signatures

error: aborting due to 5 previous errors

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