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

place explicit lifetime bound after generic param #124884

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

bvanjoi
Copy link
Contributor

@bvanjoi bvanjoi commented May 8, 2024

Fixes #124785

An easy fix.

@rustbot
Copy link
Collaborator

rustbot commented May 8, 2024

r? @estebank

rustbot has assigned @estebank.
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

@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 May 8, 2024
@bvanjoi bvanjoi force-pushed the fix-124785 branch 2 times, most recently from c3a73dc to 58b918f Compare May 9, 2024 02:22
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Minor changes required, but after that r=me.

.params
.iter()
.find(|param| param.def_id == def_id)
.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm mildly concerned about this unwrap() and would prefer if we instead used if let for this, but I can't think of a case where a Param from another item might end up here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, the Self item will cause a panic when using unwrap, but it has been handled in other branches. Anyway, I will use if let for added safety.

Comment on lines +2402 to +2401
let span = self.tcx.def_span(def_id);
span.shrink_to_hi()
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, is there any case where this can be reached?

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 also think no cases can reach this branch, but using unreachable!() here seems unnecessary...

@estebank
Copy link
Contributor

Let me know if you have time to address the nitpick.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 13, 2024

📌 Commit 4174600 has been approved by estebank

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 13, 2024

🌲 The tree is currently closed for pull requests below priority 13. This pull request will be tested once the tree is reopened.

@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 Jun 13, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 14, 2024
place explicit lifetime bound after generic param

Fixes rust-lang#124785

An easy fix.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#123962 (change method resolution to constrain hidden types instead of rejecting method candidates)
 - rust-lang#124884 (place explicit lifetime bound after generic param)
 - rust-lang#126244 (Update fuchsia commit, and SDK to 21.20240610.2.1)
 - rust-lang#126270 (Migrate run make const fn mir)
 - rust-lang#126320 (Avoid ICES after reporting errors on erroneous patterns)
 - rust-lang#126343 (Remove some msys2 utils)
 - rust-lang#126351 (std::unix::fs::link using direct linkat call for Solaris.)
 - rust-lang#126368 (Remove some unnecessary crate dependencies.)
 - rust-lang#126386 (Migrate `run-make/allow-non-lint-warnings-cmdline` to `rmake.rs`)
 - rust-lang#126449 (Fill out missing Windows support information)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#124884 (place explicit lifetime bound after generic param)
 - rust-lang#126343 (Remove some msys2 utils)
 - rust-lang#126351 (std::unix::fs::link using direct linkat call for Solaris.)
 - rust-lang#126368 (Remove some unnecessary crate dependencies.)
 - rust-lang#126386 (Migrate `run-make/allow-non-lint-warnings-cmdline` to `rmake.rs`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 74e8232 into rust-lang:master Jun 14, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2024
Rollup merge of rust-lang#124884 - bvanjoi:fix-124785, r=estebank

place explicit lifetime bound after generic param

Fixes rust-lang#124785

An easy fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Invalid error help of struct generic param with default value
5 participants