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

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

Merged
merged 1 commit into from
Jun 15, 2019

Conversation

zackmdavis
Copy link
Member

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
Member Author

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

@rust-highfive
Copy link
Collaborator

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
Member Author

(fixed tidy)

@petrochenkov
Copy link
Contributor

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
Contributor

bors commented Jun 14, 2019

📌 Commit 17653dd has been approved by eddyb

@bors
Copy link
Contributor

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
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
Centril added a commit to Centril/rust that referenced this pull request Jun 14, 2019
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
Centril added a commit to Centril/rust that referenced this pull request Jun 14, 2019
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
Centril added a commit to Centril/rust that referenced this pull request Jun 15, 2019
in which we decline to lint single-use lifetimes in `derive`d impls

Resolves rust-lang#53738.

r? @eddyb
bors added a commit that referenced this pull request Jun 15, 2019
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
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.

single_use_lifetimes warns when deriving a struct
7 participants