Fix ICE in label_fn_like for splatted args and add UI tests#157434
Fix ICE in label_fn_like for splatted args and add UI tests#157434Ajay-singh1 wants to merge 10 commits into
label_fn_like for splatted args and add UI tests#157434Conversation
|
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 cc @oli-obk Some changes occurred to the CTFE machinery 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 |
|
Failed to set assignee to
|
|
I'd recommend opening such PRs as a draft until they are ready for review. That avoids pinging everyone. :) |
|
r? teor2345 |
There was a problem hiding this comment.
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.)
| //@ 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" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 {} |
There was a problem hiding this comment.
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.
|
☔ The latest upstream changes (presumably #156816) made this pull request unmergeable. Please resolve the merge conflicts. |
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-homeexample, I discovered an ICE when calling a splatted function with an argument combination that has no registered impl:The condition
!tuple_arguments.is_splatted()inlabel_fn_likewas inverted , the safeuse_splat_fallbackpath ran for non-splatted functions, and thespan_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 i32CC: @teor2345