Skip to content

Fix E0191 suggestion for empty dyn trait args#155637

Open
qaijuang wants to merge 2 commits intorust-lang:mainfrom
qaijuang:fix-e0191-empty-dyn-trait-suggestion
Open

Fix E0191 suggestion for empty dyn trait args#155637
qaijuang wants to merge 2 commits intorust-lang:mainfrom
qaijuang:fix-e0191-empty-dyn-trait-suggestion

Conversation

@qaijuang
Copy link
Copy Markdown

@qaijuang qaijuang commented Apr 22, 2026

Fixes #155578.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 22, 2026

HIR ty lowering was modified

cc @fmease

@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 Apr 22, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 22, 2026

r? @jackh726

rustbot has assigned @jackh726.
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 72 candidates
  • Random selection from 18 candidates

@rust-log-analyzer

This comment has been minimized.

@fmease fmease assigned fmease and unassigned jackh726 Apr 22, 2026
Comment on lines 1118 to 1126
let code = if let Some(snippet) = snippet.strip_suffix('>') {
// Don't insert a leading comma for empty generic args like `dyn Trait<>`.
let separator = if snippet.trim_end().ends_with('<') { "" } else { ", " };
// The user wrote `Trait<'a>` or similar and we don't have a term we can suggest,
// but at least we can clue them to the correct syntax `Trait<'a, Item = /* ... */>`
// while accounting for the `<'a>` in the suggestion.
format!("{}, {}>", snippet, bindings.join(", "))
format!("{snippet}{separator}{}>", bindings.join(", "))
} else if in_expr_or_pat {
// The user wrote `Trait`, so we don't have a term we can suggest, but at least we
Copy link
Copy Markdown
Member

@Kivooeo Kivooeo Apr 22, 2026

Choose a reason for hiding this comment

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

Suggested change
let code = if let Some(snippet) = snippet.strip_suffix('>') {
// Don't insert a leading comma for empty generic args like `dyn Trait<>`.
let separator = if snippet.trim_end().ends_with('<') { "" } else { ", " };
// The user wrote `Trait<'a>` or similar and we don't have a term we can suggest,
// but at least we can clue them to the correct syntax `Trait<'a, Item = /* ... */>`
// while accounting for the `<'a>` in the suggestion.
format!("{}, {}>", snippet, bindings.join(", "))
format!("{snippet}{separator}{}>", bindings.join(", "))
} else if in_expr_or_pat {
// The user wrote `Trait`, so we don't have a term we can suggest, but at least we
let code = if let Some(snippet) = snippet.strip_suffix("<>") {
// Empty generics
format!("{snippet}<{}>", bindings.join(", "))
} else if let Some(snippet) = snippet.strip_suffix('>') {
// Non-empty generics
format!("{snippet}, {}>", bindings.join(", "))
} else if in_expr_or_pat {
// ...
};

Here is prototype that I haven't tested, but, something like this should be easier to read and follow

View changes since the review

Copy link
Copy Markdown
Member

@fmease fmease Apr 22, 2026

Choose a reason for hiding this comment

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

Could you please look for an existing test file that exercises this diagnostic and add the test case there? Of course, you should keep the reference to the GH issue in a comment above it.

This is super niche and doesn't warrant its own file.

View changes since the review

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.

Invalid Suggestion in "specify the associated type" help

6 participants