structured suggestions for unused-lifetimes lint #54686

zackmdavis · 2018-09-30T04:42:38Z

Regretfully, resolve_lifetime.rs is suffering from a bit of rightward-drift, but

r? @nikomatsakis

src/librustc/middle/resolve_lifetime.rs

nikomatsakis

Apart from the rightward-drift, looks awesome!

r=me if we pull out a helper function. Or @zackmdavis if for some reason that's hard, that's cool too.

nikomatsakis · 2018-10-01T21:04:35Z

src/librustc/middle/resolve_lifetime.rs

+                                    match hir_item.node {
+                                        hir::ItemKind::Fn(_, _, ref generics, _) |
+                                        hir::ItemKind::Impl(_, _, _, ref generics, _, _, _) => {
+                                            let unused_lt_span = if generics.params.len() == 1 {


and maybe the code to get the unused_lt_span would be a nice helper, too? it doesn't seem to use too many free variables

nikomatsakis · 2018-10-01T21:04:47Z

src/librustc/middle/resolve_lifetime.rs

+                        if let Some(parent_def_id) = self.tcx.parent(def_id) {
+                            if let Some(node_id) = self.tcx.hir.as_local_node_id(parent_def_id) {
+                                if let Some(Node::Item(hir_item)) = self.tcx.hir.find(node_id) {
+                                    match hir_item.node {


I'd probably pull this match into a helper

I know that similar code is elsewhere. I'm not against merging this, but we should probably sit down and look for duplication. I've written similar code in the past (although I can't find the right PR).

Also, I believe that get_generics might allow you to reduce a bit of boilerplate.

binding_suggestion is a similar helper method. Maybe we should just have a single file with all of these helpers to avoid having to dig for them every time...

estebank · 2018-10-02T00:46:45Z

The same could be done for lifetimes used only once.

Thanks to reviewers Tyler Mandry (for pointing out that this is ridiculous and we need a helper function), Niko Matsakis (for pointing out that the span-calculation code only has a couple free variables), and Esteban Küber (for pointing out `get_generics`).

zackmdavis · 2018-10-08T03:06:20Z

Pulled out span calculation into a helper method and used get_generics. I believe this satisfies @nikomatsakis conditional approval.

@bors r=nikomatsakis

bors · 2018-10-08T03:06:21Z

📌 Commit efd7a31 has been approved by nikomatsakis

bors · 2018-10-08T05:40:12Z

⌛ Testing commit efd7a31 with merge b5c33a424a250b7891b93f60e31d63c279cb66a3...

bors · 2018-10-08T06:50:20Z

💔 Test failed - status-travis

rust-highfive · 2018-10-08T06:50:21Z

The job dist-i686-freebsd of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

travis_time:end:15976910:start=1538981366252627588,finish=1538981366260775477,duration=8147889
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:01fd3740
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:2f3c404e
travis_time:start:2f3c404e
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:051fa4b5
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

zackmdavis · 2018-10-08T14:04:14Z

curl: (6) Could not resolve host: s3-us-west-1.amazonaws.com

[01:06:34] thread 'main' panicked at 'failed to download openssl source: exit code: 6', bootstrap/native.rs:621:17

🙄 😼

@bors retry

bors · 2018-10-08T22:07:00Z

⌛ Testing commit efd7a31 with merge b1a137d...

@nikomatsakis

structured suggestions for unused-lifetimes lint Regretfully, resolve_lifetime.rs is suffering from a bit of rightward-drift, but ![zero_life](https://user-images.githubusercontent.com/1076988/46253407-010e7880-c430-11e8-8faf-b9afc1405415.png) r? @nikomatsakis

bors · 2018-10-09T01:01:37Z

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

rust-highfive assigned nikomatsakis Sep 30, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 30, 2018

tmandry requested changes Oct 1, 2018

View reviewed changes

src/librustc/middle/resolve_lifetime.rs Show resolved Hide resolved

nikomatsakis approved these changes Oct 1, 2018

View reviewed changes

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 Oct 4, 2018

zackmdavis added 2 commits October 7, 2018 19:17

structured suggestions for unused-lifetimes lint

b1b684e

zackmdavis force-pushed the zero_life branch from fc9986e to efd7a31 Compare October 8, 2018 02:59

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2018

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 8, 2018

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 Oct 8, 2018

tmandry approved these changes Oct 8, 2018

View reviewed changes

bors merged commit efd7a31 into rust-lang:master Oct 9, 2018

zackmdavis deleted the zero_life branch October 9, 2018 01:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

structured suggestions for unused-lifetimes lint #54686

structured suggestions for unused-lifetimes lint #54686

zackmdavis commented Sep 30, 2018

nikomatsakis left a comment •

edited

Loading

nikomatsakis Oct 1, 2018

nikomatsakis Oct 1, 2018

estebank Oct 2, 2018

estebank Oct 2, 2018

estebank commented Oct 2, 2018

zackmdavis commented Oct 8, 2018

bors commented Oct 8, 2018

bors commented Oct 8, 2018

bors commented Oct 8, 2018

rust-highfive commented Oct 8, 2018

zackmdavis commented Oct 8, 2018

bors commented Oct 8, 2018

bors commented Oct 9, 2018

structured suggestions for unused-lifetimes lint #54686

structured suggestions for unused-lifetimes lint #54686

Conversation

zackmdavis commented Sep 30, 2018

nikomatsakis left a comment • edited Loading

Choose a reason for hiding this comment

nikomatsakis Oct 1, 2018

Choose a reason for hiding this comment

nikomatsakis Oct 1, 2018

Choose a reason for hiding this comment

estebank Oct 2, 2018

Choose a reason for hiding this comment

estebank Oct 2, 2018

Choose a reason for hiding this comment

estebank commented Oct 2, 2018

zackmdavis commented Oct 8, 2018

bors commented Oct 8, 2018

bors commented Oct 8, 2018

bors commented Oct 8, 2018

rust-highfive commented Oct 8, 2018

zackmdavis commented Oct 8, 2018

bors commented Oct 8, 2018

bors commented Oct 9, 2018

nikomatsakis left a comment •

edited

Loading