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

Point to lifetime spans on lifetime errors #51862

Merged
merged 6 commits into from
Jun 30, 2018

Conversation

estebank
Copy link
Contributor

No description provided.

@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 28, 2018
@estebank
Copy link
Contributor Author

r? @nikomatsakis

let mut sp = cm.def_span(self.hir.span(node));
if let Some(generics) = self.hir.get_generics(scope) {
for param in &generics.params {
if param.name.name().as_str() == br.name.as_str() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This loop feels like something that occurs more often (e.g. in lowering code or other diagnostics). Maybe pull it out into a method on Generics and do a quick grep around the source to see if any other places could benefit from an fn(&Generics, Name) -> Option<&Param>

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 made a cursory look for other possible uses but I couldn't any. I'm sure I'll find some while working on something else down the line, though.

@bors

This comment has been minimized.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Looks good, if we extract a helper would look better. 😛

),
ty::ReEarlyBound(ref br) => {
let mut sp = cm.def_span(self.hir.span(node));
if let Some(generics) = self.hir.get_generics(scope) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @oli-obk that we should not "open code" this loop but rather extract a helper. Bonus points for finding other code to use it =)

let mut sp = cm.def_span(self.hir.span(node));
if let Some(generics) = self.hir.get_generics(scope) {
for param in &generics.params {
if param.name.name().as_str() == name.as_str() {
Copy link
Contributor

Choose a reason for hiding this comment

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

If nothing else, the same loop seems to be down here?

@nikomatsakis nikomatsakis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 28, 2018
@estebank
Copy link
Contributor Author

Helper added.

@rust-highfive

This comment has been minimized.

@estebank estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 29, 2018
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 29, 2018

📌 Commit 8449c5a has been approved by nikomatsakis

@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 29, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 30, 2018
…akis

Point to lifetime spans on lifetime errors
bors added a commit that referenced this pull request Jun 30, 2018
Point to lifetime spans on lifetime errors
@bors
Copy link
Contributor

bors commented Jun 30, 2018

⌛ Testing commit 8449c5a with merge 8772747...

@bors
Copy link
Contributor

bors commented Jun 30, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 8772747 to master...

@bors bors merged commit 8449c5a into rust-lang:master Jun 30, 2018
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants