Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add needless_maybe_sized lint #10632

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Conversation

Alexendoo
Copy link
Member

changelog: new lint: [needless_maybe_sized]

Closes #10600

@rustbot
Copy link
Collaborator

rustbot commented Apr 11, 2023

r? @giraffate

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 11, 2023
@Alexendoo Alexendoo force-pushed the needless-maybe-sized branch from 0220b55 to 5b81677 Compare August 14, 2023 13:15
@Jarcho
Copy link
Contributor

Jarcho commented Feb 14, 2024

r? Jarcho

I'm not sure if the trace with spans is really worth it. It takes up a lot space and most of the info is not very useful. Is there a way to just print the location without the snippet?

One extension would be to check if any of the traits have functions using Self by value. e.g.

trait T {
  fn foo() -> Self;
}

There's no Sized bound on the trait, but it's also unimplementable for unsized types.

@rustbot rustbot assigned Jarcho and unassigned giraffate Feb 14, 2024
@Alexendoo
Copy link
Member Author

I don't think there is a way to keep the location while hiding the snippet, but we could make them regular notes

@bors
Copy link
Contributor

bors commented Feb 24, 2024

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

@Alexendoo Alexendoo force-pushed the needless-maybe-sized branch from 5b81677 to 0bc60dc Compare March 22, 2024 16:23
@Alexendoo
Copy link
Member Author

I ditched the spans for the trace, doesn't have the location but it seems fine to me. Most of the time it should be clear which traits are the cause but if not the user can always follow the bounds themselves

@Jarcho
Copy link
Contributor

Jarcho commented Apr 4, 2024

Too bad there's no option for printing spans like that.

@Jarcho
Copy link
Contributor

Jarcho commented Apr 12, 2024

Been a week without objections. Getting the spans printed nicely isn't a blocker and can be added in later.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 12, 2024

📌 Commit 0bc60dc has been approved by Jarcho

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 12, 2024

⌛ Testing commit 0bc60dc with merge fb65e02...

bors added a commit that referenced this pull request Apr 12, 2024
Add `needless_maybe_sized` lint

changelog: new lint: [`needless_maybe_sized`]

Closes #10600
@bors
Copy link
Contributor

bors commented Apr 12, 2024

💔 Test failed - checks-action_dev_test

@Alexendoo Alexendoo force-pushed the needless-maybe-sized branch from 0bc60dc to cf0b55e Compare April 14, 2024 14:43
@J-ZhengLi
Copy link
Member

J-ZhengLi commented May 31, 2024

getting blocked by bors for this long is kind of unfortunate.

@Jarcho, could you retry running the tests so this can get through?

@Jarcho
Copy link
Contributor

Jarcho commented Jun 5, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jun 5, 2024

📌 Commit cf0b55e has been approved by Jarcho

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 5, 2024

⌛ Testing commit cf0b55e with merge 6cfd4ac...

@bors
Copy link
Contributor

bors commented Jun 5, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing 6cfd4ac to master...

@bors bors merged commit 6cfd4ac into rust-lang:master Jun 5, 2024
8 checks passed
@Alexendoo Alexendoo deleted the needless-maybe-sized branch June 6, 2024 00:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn on useless ?Sized anti-bound
6 participants