Skip to content

add regression tests for the fn sig ice - #160746

Open
im-lunex wants to merge 1 commit into
rust-lang:mainfrom
im-lunex:test_for_160628
Open

add regression tests for the fn sig ice#160746
im-lunex wants to merge 1 commit into
rust-lang:mainfrom
im-lunex:test_for_160628

Conversation

@im-lunex

@im-lunex im-lunex commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

follow-up to #160628 which fixed the suggest_add_reference_to_arg fn_sig ice

adds the regression tests @teor2345 asked for in review

  • closure call with a missing-field struct literal (iced without the guard)
  • method call whose where-clause fails (covers the method-call branch)

the impl def_id case cant be constructed, those obligations get dropped during probing and turn into e0599 instead

@rustbot rustbot added 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. labels Aug 8, 2026
@rustbot

rustbot commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

r? @camelid

rustbot has assigned @camelid.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 75 candidates
  • Random selection from 18 candidates

@teor2345

teor2345 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Cc @JohnTitor @Kivooeo who worked on who have context on #160628

@Kivooeo

Kivooeo commented Aug 9, 2026

Copy link
Copy Markdown
Member

feel free to review i will r= you

@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.

Thanks, these are good tests and improve the coverage of PR #160628.

the impl def_id case cant be constructed, those obligations get dropped during probing and turn into e0599 instead

I try to avoid modifying code we can't test, or code for things that can never happen. But I think this part of PR #160628 is still a good idea, because it removes an ICE that could become reachable after future code changes.

Sometimes, I add tests that are "as close as we can get" to reaching the changed code. But I think we've done a lot here already, so it's up to you if you want to add something like that in another PR. I don't mind either way.

View changes since this review

@Kivooeo Kivooeo Aug 10, 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.

also add a links to issue in both tests

View changes since the review

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.

done

@im-lunex

Copy link
Copy Markdown
Contributor Author

Thanks, these are good tests and improve the coverage of PR #160628.

the impl def_id case cant be constructed, those obligations get dropped during probing and turn into e0599 instead

I try to avoid modifying code we can't test, or code for things that can never happen. But I think this part of PR #160628 is still a good idea, because it removes an ICE that could become reachable after future code changes.

Sometimes, I add tests that are "as close as we can get" to reaching the changed code. But I think we've done a lot here already, so it's up to you if you want to add something like that in another PR. I don't mind either way.

View changes since this review

thanks letting me choose and yeah i would like to keep this much for now.. and sure im open if you say i should add more.

@rust-log-analyzer

This comment has been minimized.

@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.

Thanks, this looks good to me!

View changes since this review

@JohnTitor

Copy link
Copy Markdown
Member

@bors squash

@rust-bors

This comment has been minimized.

* add regression tests for the fn sig ice
* add issuelinks in on top of the tests
* regenerate stderr snapshots
@rust-bors

rust-bors Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

🔨 3 commits were squashed into 92196db.

@rust-bors
rust-bors Bot force-pushed the test_for_160628 branch from 2daa62f to 92196db Compare August 10, 2026 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants