Skip to content

in which we decline to lint single-use lifetimes in derived impls#61824

Merged
bors merged 1 commit intomasterfrom
single_derive
Jun 15, 2019
Merged

in which we decline to lint single-use lifetimes in derived impls#61824
bors merged 1 commit intomasterfrom
single_derive

Conversation

@zackmdavis
Copy link
Contributor

@zackmdavis zackmdavis commented Jun 14, 2019

Resolves #53738.

r? @eddyb

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

zackmdavis commented Jun 14, 2019

(Pushing the branch directly to rust-lang/rust rather than my "fork" was a mistake, but probably an inconsequential one?)

@rust-highfive
Copy link
Contributor

rust-highfive commented Jun 14, 2019

The job x86_64-gnu-llvm-6.0 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. 

[00:04:38] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:39] tidy error: /checkout/src/librustc/middle/resolve_lifetime.rs:1595: line longer than 100 chars
[00:04:44] some tidy checks failed
[00:04:44] 
[00:04:44] 
[00:04:44] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:44] 
[00:04:44] 
[00:04:44] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:44] Build completed unsuccessfully in 0:01:14
---
travis_time:end:0ce616d0:start=1560493917882537935,finish=1560493917888206705,duration=5668770
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0e1e86ed
$ 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:181b93a0
travis_time:start:181b93a0
$ 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:166925a6
$ 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)

@Centril
Copy link
Contributor

Centril commented Jun 14, 2019

but probably an inconsequential one?

Yep; we have branch protection on the important branches; please just cleanup after you're done. 😃

@Centril
Copy link
Contributor

Centril commented Jun 14, 2019

@scottmcm suggested on Discord that we should change the macro expansion of the built-in derives to use '_ instead. That seems like it might generate less code that might be good for perf and possibly solve this issue (but not for custom derives!). In either case, even if we land this PR in its current form, we should probably also change the expansion?

@zackmdavis
Copy link
Contributor Author

zackmdavis commented Jun 14, 2019

(fixed tidy)

@petrochenkov
Copy link
Contributor

petrochenkov commented Jun 14, 2019

Derives are a special case of "macros from other crates", which normally suppress lints.
Why doesn't that generic mechanism work here?

@oli-obk
Copy link
Contributor

oli-obk commented Jun 14, 2019

Derives are a special case of "macros from other crates", which normally suppress lints.
Why doesn't that generic mechanism work here?

The problem is (as explained by @estebank) that the derive doesn't mark the spans of idents it's using as coming from an expansion.

@eddyb
Copy link
Member

eddyb commented Jun 14, 2019

@bors r+ (feel free to open an issue about changing the deriving output)

@bors
Copy link
Collaborator

bors commented Jun 14, 2019

📌 Commit 17653dd has been approved by eddyb

@bors
Copy link
Collaborator

bors commented Jun 14, 2019

🌲 The tree is currently closed for pull requests below priority 500, 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 14, 2019
Centril added a commit to Centril/rust that referenced this pull request Jun 14, 2019
@Centril
Rollup merge of rust-lang#61824 - rust-lang:single_derive, r=eddyb
20aba33 
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
@Centril Centril mentioned this pull request Jun 14, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jun 14, 2019
@Centril
Rollup merge of rust-lang#61824 - rust-lang:single_derive, r=eddyb
dd5f3b0 
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
@Centril Centril mentioned this pull request Jun 14, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jun 14, 2019
@Centril
Rollup merge of rust-lang#61824 - rust-lang:single_derive, r=eddyb
3f28c60 
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
@Centril Centril mentioned this pull request Jun 14, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Jun 15, 2019
@Centril
Rollup merge of rust-lang#61824 - rust-lang:single_derive, r=eddyb
eb188f1 
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
@Centril Centril mentioned this pull request Jun 15, 2019
Merged
bors added a commit that referenced this pull request Jun 15, 2019
@bors
Auto merge of #61868 - Centril:rollup-gglsecp, r=Centril
bb16e72 
Rollup of 6 pull requests

Successful merges:

 - #61785 (note some safety concerns of raw-ptr-to-ref casts)
 - #61805 (typeck: Fix ICE for blocks in repeat expr count.)
 - #61813 (Remove some unnecessary symbol interner ops)
 - #61824 (in which we decline to lint single-use lifetimes in `derive`d impls)
 - #61844 (Change `...` to `..=` where applicable)
 - #61854 (Minor cosmetic improvements to accompany PR 61825)

Failed merges:

r? @ghost
@bors bors merged commit 17653dd into master Jun 15, 2019
@Centril Centril deleted the single_derive branch June 15, 2019 21:17
@Centril Centril mentioned this pull request Jun 15, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@eddyb eddyb eddyb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@eddyb eddyb

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

single_use_lifetimes warns when deriving a struct

7 participants

@zackmdavis @rust-highfive @Centril @petrochenkov @oli-obk @eddyb @bors