Skip to content
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

fix: ADT hover considering only type or const len not lifetimes #16967

Merged
merged 1 commit into from Mar 28, 2024

Conversation

dfireBird
Copy link
Contributor

I feel like test doesn't do much. Please suggest if I should improve it?

Fixes #16963

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 28, 2024
crates/hir/src/lib.rs Outdated Show resolved Hide resolved
@dfireBird
Copy link
Contributor Author

My bad, amended without changes accidentally.

@lnicola
Copy link
Member

lnicola commented Mar 28, 2024

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 28, 2024

📌 Commit 34a8cd6 has been approved by lnicola

It is now in the queue for this repository.

@lnicola
Copy link
Member

lnicola commented Mar 28, 2024

The test does its job (crash without the fix), it's fine.

@bors
Copy link
Collaborator

bors commented Mar 28, 2024

⌛ Testing commit 34a8cd6 with merge ab10eea...

@bors
Copy link
Collaborator

bors commented Mar 28, 2024

☀️ Test successful - checks-actions
Approved by: lnicola
Pushing ab10eea to master...

@bors bors merged commit ab10eea into rust-lang:master Mar 28, 2024
11 checks passed
@dfireBird dfireBird deleted the fix-16963 branch March 28, 2024 17:06
@@ -1418,7 +1418,8 @@ impl Adt {
}

pub fn layout(self, db: &dyn HirDatabase) -> Result<Layout, LayoutError> {
if db.generic_params(self.into()).iter().count() != 0 {
let generic_params = &db.generic_params(self.into());
if generic_params.iter().next().is_some() || generic_params.iter_lt().next().is_some() {
Copy link
Member

Choose a reason for hiding this comment

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

Having a GenericParam::is_empty method would probably be useful here (or given there is more than just params in it, a GenericParam::has_params)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I open a PR with the proposed change of this and the proposed change in another PR?

Feel like a opening a lot of small PRs 😅.

Copy link
Member

Choose a reason for hiding this comment

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

Putting both into one is fine

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic on hovering over type name with lifetime argument
5 participants