Skip to content

Add help diag. for const = Enum missing braces around Enum #106283

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.

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

Merged
merged 1 commit into from
Jan 7, 2023

Conversation

JulianKnodt
Copy link
Contributor

Previously it was not clear why this errored or if it was even supported, as there was no diagnostic that suggested wrapping it in braces.

Thus, add a simple diagnostic that suggests wrapping enum variants in braces.

Fixes #105927

@JulianKnodt JulianKnodt marked this pull request as ready for review December 30, 2022 05:10
@rustbot
Copy link
Collaborator

rustbot commented Dec 30, 2022

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

@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 Dec 30, 2022
@JulianKnodt JulianKnodt changed the title Add note about wrapping in braces Add help diag. for const = Enum missing braces around Enum Dec 30, 2022
format!("{{ {path_str} }}"),
Applicability::MaybeIncorrect,
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this suggestion correct in general?
AFAIK, disambiguating using braces only works for equality associated constraints.
The suggestion needs to be restricted to this case, doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean as opposed to other const generics? Disambiguating with braces also works for functions in generic const positions.
By this case, do you mean specifically for when the constraint is a const?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean as opposed to all the other places where one can put a type.
Does this suggestion fire with this?

fn foo() -> Mode::Cool { /* ... */ }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does fire, I'll try to see where to change to make it not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed it to be only for associated equality, but it's not so specific to enums.

Previously it was not clear why this errored or if it was even supported, as there was no
diagnostic that suggested wrapping it in braces.

Thus, add a simple diagnostic that suggests wrapping enum variants in braces.
@cjgillot
Copy link
Contributor

cjgillot commented Jan 7, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 7, 2023

📌 Commit 077fae9 has been approved by cjgillot

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 7, 2023
@bors
Copy link
Collaborator

bors commented Jan 7, 2023

⌛ Testing commit 077fae9 with merge d72b7d2...

@bors
Copy link
Collaborator

bors commented Jan 7, 2023

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing d72b7d2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 7, 2023
@bors bors merged commit d72b7d2 into rust-lang:master Jan 7, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 7, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d72b7d2): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.3% [1.3%, 1.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.9% [-2.9%, -2.9%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.9% [-2.9%, -2.9%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

@JulianKnodt JulianKnodt deleted the enum_err branch January 7, 2023 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

Associated constants equality produces errors with constants
6 participants