Skip to content

Fix ICE in label_fn_like for splatted args and add UI tests#157434

Draft
Ajay-singh1 wants to merge 10 commits into
rust-lang:mainfrom
Ajay-singh1:pr-153697
Draft

Fix ICE in label_fn_like for splatted args and add UI tests#157434
Ajay-singh1 wants to merge 10 commits into
rust-lang:mainfrom
Ajay-singh1:pr-153697

Conversation

@Ajay-singh1

@Ajay-singh1 Ajay-singh1 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

This PR builds on top of #153697 (teor's #[splat] implementation).

Bug Fix

While writing UI tests for the unhappy path of the overload-at-home example, I discovered an ICE when calling a splatted function with an argument combination that has no registered impl:

foo.method(42i32, 42i32); // no impl for (i32, i32)
// ICE: arg index 1 out of bounds for method with 1 inputs

The condition !tuple_arguments.is_splatted() in label_fn_like was inverted , the safe use_splat_fallback path ran for non-splatted functions, and the span_bug! fired for splatted ones. Fix is removing the !.

After the fix the compiler gives a clean error instead:

error[E0308]: mismatched types expected String, found i32

CC: @teor2345

@rustbot

rustbot commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

HIR ty lowering was modified

cc @fmease

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

The reflection data structures are tied exactly to the implementation
in the compiler. Make sure to also adjust rustc_const_eval/src/const_eval/type_info.rs

cc @oli-obk

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

The Miri subtree was changed

cc @rust-lang/miri

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

The Cranelift subtree was changed

cc @bjorn3

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Jun 4, 2026
@rustbot

rustbot commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Failed to set assignee to teor2345: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@RalfJung

RalfJung commented Jun 4, 2026

Copy link
Copy Markdown
Member

I'd recommend opening such PRs as a draft until they are ready for review. That avoids pinging everyone. :)

@Ajay-singh1 Ajay-singh1 marked this pull request as draft June 4, 2026 13:41
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 4, 2026
teor2345

This comment was marked as resolved.

@teor2345

teor2345 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

r? teor2345

@teor2345

teor2345 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

I cherry-picked the fix and extra tests to #153697, so any code either of us write on top of that PR is run on the full set of tests. I also made the minor comment fixes I suggested in commit 8d7a905

@teor2345 teor2345 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, I have a nitpick on comments, and a suggestion for related tests.

If we can, let's try to keep failing and passing tests in different files. (I know I haven't always done that, if you want you can split existing test files into passing and failing.)

View changes since this review

//@ run-pass
//! Test using `#[splat]` on tuple arguments of const functions with generics.
//! This fills the FIXME in splat-generics-everywhere.rs:
//! "add const fn generics tests"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nitpick: this comment is useful, but it would be better to edit that file and remove the FIXME (and this comment).

Same for the other files in this commit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure! I'll push a commit ASAP.


fn where_splat<T>(#[splat] _t: T) where T: std::marker::Tuple {}

fn where_splat_with_extra<T>(#[splat] _t: T, _extra: u32) where T: std::marker::Tuple {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion:

Can we also test splatting impl Tuple arguments?
https://doc.rust-lang.org/reference/types/impl-trait.html#r-type.impl-trait.intro

If those tests fail, please put them in a separate test file, and add a FIXME to the part of the code that fails. I can help with finding that part of the code once I see the error message.

Or if you know how to add support, you can just fix the code.

Failing Test

We can't test dyn Tuple because the Tuple trait isn't dyn-compatible, but we can test:

fn f<T>(#[splat] t: &dyn AsRef<T>) where T: Tuple {}

I'm not sure if we even want to support dyn or AsRef<Tuple>, but having failing tests like this helps us work out if we ever accidentally add support. And it makes it clear we don't expect it to work right now.

@rust-bors

rust-bors Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #156816) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants