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

Add all remaining DefKinds. #70043

Merged
merged 7 commits into from
Apr 26, 2020
Merged

Add all remaining DefKinds. #70043

merged 7 commits into from
Apr 26, 2020

Conversation

mark-i-m
Copy link
Member

@mark-i-m mark-i-m commented Mar 16, 2020

r? @eddyb or @Centril

I'm not sure if this is what you were thinking of. There are also a few places where I'm not sure what the correct choice is because I don't fully understand the meaning of some variants.

In general, it feels a bit odd to add some of these as DefKinds (e.g. Arm) because they don't feel like definitions. Are there things that it makes sense not to add?

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 16, 2020
@mark-i-m
Copy link
Member Author

Also, is this the right def_kind method? And are there Hir node types that will have no corresponding DefId?

@Centril
Copy link
Contributor

Centril commented Mar 16, 2020

In general, it feels a bit odd to add some of these as DefKinds (e.g. Arm) because they don't feel like definitions.

I concur, this feels rather strange to me as well. cc @petrochenkov

@eddyb
Copy link
Member

eddyb commented Mar 16, 2020

@mark-i-m So, the only things with DefIds should correspond to create_def calls in https://github.com/rust-lang/rust/blob/master/src/librustc_resolve/def_collector.rs.

I'll leave comments on the variants which you have and I think can't have DefIds.

src/librustc_hir/def.rs Outdated Show resolved Hide resolved
src/librustc/hir/map/mod.rs Outdated Show resolved Hide resolved
src/librustc/hir/map/mod.rs Outdated Show resolved Hide resolved
src/librustc/hir/map/mod.rs Outdated Show resolved Hide resolved
src/librustc/hir/map/mod.rs Outdated Show resolved Hide resolved
src/librustc_hir/def.rs Outdated Show resolved Hide resolved
src/librustc_hir/def.rs Outdated Show resolved Hide resolved
src/librustc_hir/def.rs Outdated Show resolved Hide resolved
@eddyb
Copy link
Member

eddyb commented Mar 16, 2020

Btw, you'd also want to change this:

EntryKind::ForeignMod
| EntryKind::GlobalAsm
| EntryKind::Impl(_)
| EntryKind::Field
| EntryKind::Generator(_)
| EntryKind::Closure => return None,

If you're committed to adding all DefKinds at once, that function will return DefKind instead of Option<DefKind> after you're done.

cc @petrochenkov @Zoxc

src/librustc/hir/map/mod.rs Outdated Show resolved Hide resolved
src/librustc_hir/def.rs Outdated Show resolved Hide resolved
src/librustc/hir/map/mod.rs Outdated Show resolved Hide resolved
src/librustc/hir/map/mod.rs Outdated Show resolved Hide resolved
@mark-i-m

This comment has been minimized.

@mark-i-m

This comment has been minimized.

@mark-i-m mark-i-m marked this pull request as ready for review March 17, 2020 04:08
@rust-highfive

This comment has been minimized.

src/librustc/hir/map/mod.rs Outdated Show resolved Hide resolved
src/librustc_hir/def.rs Outdated Show resolved Hide resolved
src/librustc_hir/def.rs Outdated Show resolved Hide resolved
EntryKind::ForeignMod => DefKind::ForeignMod,
EntryKind::GlobalAsm => DefKind::GlobalAsm,
EntryKind::Field => DefKind::Field,
EntryKind::Generator(_) => DefKind::Closure,
Copy link
Member

Choose a reason for hiding this comment

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

Hmm should we have a DefKind::Generator? I forgot about that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that too. I think it would be preferable, but how would we tell from a Node::Expr if something is a generator (i.e. in librustc_hir)?

Copy link
Member

Choose a reason for hiding this comment

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

You have a TyCtxt so you probably have a way to check. @Zoxc would know more.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can tell if it's a generator from ExprKind::Closure, not sure if we should have a DefKind::Generator though.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Zoxc Sorry, I don't quite understand. How can you tell from ExprKind::Closure?

Copy link
Contributor

Choose a reason for hiding this comment

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

The Option<Movability> field will be Some for generators.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 24, 2020
@bors
Copy link
Contributor

bors commented Apr 24, 2020

☔ The latest upstream changes (presumably #71215) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 24, 2020
@rust-highfive
Copy link
Collaborator

Your PR failed (pretty log, 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.
2020-04-24T18:45:10.6994723Z ========================== Starting Command Output ===========================
2020-04-24T18:45:10.6997834Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/954d7b27-ecc6-4184-ba15-7a14a3446041.sh
2020-04-24T18:45:10.6998075Z 
2020-04-24T18:45:10.7002230Z ##[section]Finishing: Disable git automatic line ending conversion
2020-04-24T18:45:10.7019861Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/70043/merge to s
2020-04-24T18:45:10.7023452Z Task         : Get sources
2020-04-24T18:45:10.7023731Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-04-24T18:45:10.7024022Z Version      : 1.0.0
2020-04-24T18:45:10.7024208Z Author       : Microsoft
---
2020-04-24T18:45:11.9437897Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-04-24T18:45:11.9445533Z ##[command]git config gc.auto 0
2020-04-24T18:45:11.9449039Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-04-24T18:45:11.9453370Z ##[command]git config --get-all http.proxy
2020-04-24T18:45:11.9462478Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/70043/merge:refs/remotes/pull/70043/merge
---
2020-04-24T18:47:25.7410142Z  ---> f7353ccad5b1
2020-04-24T18:47:25.7410395Z Step 6/7 : ENV RUN_CHECK_WITH_PARALLEL_QUERIES 1
2020-04-24T18:47:25.7411827Z  ---> Using cache
2020-04-24T18:47:25.7412149Z  ---> ed38efbaa060
2020-04-24T18:47:25.7413430Z Step 7/7 : ENV SCRIPT python3 ../x.py test src/tools/expand-yaml-anchors &&            python3 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu &&            python3 ../x.py build --stage 0 src/tools/build-manifest &&            python3 ../x.py test --stage 0 src/tools/compiletest &&            python3 ../x.py test src/tools/tidy &&            /scripts/validate-toolstate.sh
2020-04-24T18:47:25.7414721Z  ---> c5008ef7ae8e
2020-04-24T18:47:25.7414918Z Successfully built c5008ef7ae8e
2020-04-24T18:47:25.7452316Z Successfully tagged rust-ci:latest
2020-04-24T18:47:25.7717286Z Built container sha256:c5008ef7ae8e94d7ef502e3cef26e61208e14ebdb36913f3a8bb86291bd6430b
2020-04-24T18:47:25.7717286Z Built container sha256:c5008ef7ae8e94d7ef502e3cef26e61208e14ebdb36913f3a8bb86291bd6430b
2020-04-24T18:47:25.7733168Z Looks like docker image is the same as before, not uploading
2020-04-24T18:47:33.8091506Z [CI_JOB_NAME=mingw-check]
2020-04-24T18:47:33.8341230Z [CI_JOB_NAME=mingw-check]
2020-04-24T18:47:33.8375751Z == clock drift check ==
2020-04-24T18:47:33.8384121Z   local time: Fri Apr 24 18:47:33 UTC 2020
2020-04-24T18:47:34.1289705Z   network time: Fri, 24 Apr 2020 18:47:34 GMT
2020-04-24T18:47:34.1347079Z Starting sccache server...
2020-04-24T18:47:34.2490999Z configure: processing command line
2020-04-24T18:47:34.2491284Z configure: 
2020-04-24T18:47:34.2492149Z configure: rust.parallel-compiler := True
---
2020-04-24T18:51:21.2417650Z     Checking rustc_feature v0.0.0 (/checkout/src/librustc_feature)
2020-04-24T18:51:21.4123920Z     Checking fmt_macros v0.0.0 (/checkout/src/libfmt_macros)
2020-04-24T18:51:21.6063548Z     Checking rustc_ast_pretty v0.0.0 (/checkout/src/librustc_ast_pretty)
2020-04-24T18:51:21.6316343Z     Checking rustc_hir v0.0.0 (/checkout/src/librustc_hir)
2020-04-24T18:51:22.1827699Z     Checking rustc_query_system v0.0.0 (/checkout/src/librustc_query_system)
2020-04-24T18:51:24.4353193Z     Checking rustc_attr v0.0.0 (/checkout/src/librustc_attr)
2020-04-24T18:51:24.8974175Z     Checking rustc_parse v0.0.0 (/checkout/src/librustc_parse)
2020-04-24T18:51:26.8696160Z     Checking rustc_hir_pretty v0.0.0 (/checkout/src/librustc_hir_pretty)
2020-04-24T18:51:27.2824853Z     Checking rustc_ast_lowering v0.0.0 (/checkout/src/librustc_ast_lowering)
---
2020-04-24T18:51:59.5295573Z     Checking rustc_passes v0.0.0 (/checkout/src/librustc_passes)
2020-04-24T18:52:00.0338691Z error[E0308]: mismatched types
2020-04-24T18:52:00.0339374Z    --> src/librustc_passes/dead.rs:561:61
2020-04-24T18:52:00.0339883Z     |
2020-04-24T18:52:00.0340513Z 561 |                 let descr = self.tcx.def_kind(def_id).descr(def_id);
2020-04-24T18:52:00.0342288Z 
2020-04-24T18:52:00.4855249Z error[E0308]: mismatched types
2020-04-24T18:52:00.4856425Z    --> src/librustc_passes/stability.rs:346:57
2020-04-24T18:52:00.4857327Z     |
2020-04-24T18:52:00.4857327Z     |
2020-04-24T18:52:00.4858453Z 346 |             let descr = self.tcx.def_kind(def_id).descr(def_id);
2020-04-24T18:52:00.4863261Z 
2020-04-24T18:52:00.5369038Z error: aborting due to 2 previous errors
2020-04-24T18:52:00.5369673Z 
2020-04-24T18:52:00.5370386Z For more information about this error, try `rustc --explain E0308`.
2020-04-24T18:52:00.5370386Z For more information about this error, try `rustc --explain E0308`.
2020-04-24T18:52:00.5417273Z error: could not compile `rustc_passes`.
2020-04-24T18:52:00.5417717Z 
2020-04-24T18:52:00.5418180Z To learn more, run the command again with --verbose.
2020-04-24T18:52:00.5418759Z warning: build failed, waiting for other jobs to finish...
2020-04-24T18:52:02.6918867Z error: build failed
2020-04-24T18:52:02.6955347Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "2" "--release" "--color" "always" "--features" " llvm" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json-render-diagnostics"
2020-04-24T18:52:02.6969710Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check
2020-04-24T18:52:02.6970173Z Build completed unsuccessfully in 0:04:28
2020-04-24T18:52:02.7084261Z == clock drift check ==
2020-04-24T18:52:02.7084261Z == clock drift check ==
2020-04-24T18:52:02.7100751Z   local time: Fri Apr 24 18:52:02 UTC 2020
2020-04-24T18:52:02.7758043Z   network time: Fri, 24 Apr 2020 18:52:02 GMT
2020-04-24T18:52:03.4623602Z 
2020-04-24T18:52:03.4623602Z 
2020-04-24T18:52:03.4692247Z ##[error]Bash exited with code '1'.
2020-04-24T18:52:03.4705676Z ##[section]Finishing: Run build
2020-04-24T18:52:03.4759047Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/70043/merge to s
2020-04-24T18:52:03.4764675Z Task         : Get sources
2020-04-24T18:52:03.4764993Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-04-24T18:52:03.4765303Z Version      : 1.0.0
2020-04-24T18:52:03.4765512Z Author       : Microsoft
2020-04-24T18:52:03.4765512Z Author       : Microsoft
2020-04-24T18:52:03.4765843Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-04-24T18:52:03.4766228Z ==============================================================================
2020-04-24T18:52:03.8263331Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-04-24T18:52:03.8314113Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/70043/merge to s
2020-04-24T18:52:03.8400677Z Cleaning up task key
2020-04-24T18:52:03.8402125Z Start cleaning up orphan processes.
2020-04-24T18:52:03.8606543Z Terminate orphan process: pid (3617) (python)
2020-04-24T18:52:03.8792595Z ##[section]Finishing: Finalize Job

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 @rust-lang/infra. (Feature Requests)

@mark-i-m
Copy link
Member Author

@JohnTitor @eddyb rebased, ci passing

@eddyb
Copy link
Member

eddyb commented Apr 25, 2020

@bors r+ delegate=mark-i-m

@bors
Copy link
Contributor

bors commented Apr 25, 2020

📌 Commit 258ebfe has been approved by eddyb

@bors
Copy link
Contributor

bors commented Apr 25, 2020

✌️ @mark-i-m can now approve this pull request

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 25, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 25, 2020
Add all remaining `DefKind`s.

r? @eddyb or @Centril

~~I'm not sure if this is what you were thinking of. There are also a few places where I'm not sure what the correct choice is because I don't fully understand the meaning of some variants.~~

~~In general, it feels a bit odd to add some of these as `DefKind`s (e.g. `Arm`) because they don't feel like definitions. Are there things that it makes sense not to add?~~
This was referenced Apr 25, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 26, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#70043 (Add all remaining `DefKind`s.)
 - rust-lang#71140 ([breaking change] Disallow statics initializing themselves)
 - rust-lang#71392 (Don't hold the predecessor cache lock longer than necessary)
 - rust-lang#71541 (Add regression test for rust-lang#26376)
 - rust-lang#71554 (Replace thread_local with generator resume arguments in box_region.)

Failed merges:

r? @ghost
@bors bors merged commit e51cbc8 into rust-lang:master Apr 26, 2020
phansch added a commit to phansch/rust-clippy that referenced this pull request Apr 26, 2020
bors added a commit to rust-lang/rust-clippy that referenced this pull request Apr 26, 2020
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 26, 2020
Changes:
````
rustup to rust-lang#70043
map_clone: avoid suggesting `copied()` for &mut
fix redundant_pattern_matching lint
Add tests for rust-lang#1654
Don't trigger while_let_on_iterator when the iterator is recreated every iteration
Update issue_2356.stderr reference file
Update while_let_on_iterator tests
Fix while_let_on_iterator suggestion and make it MachineApplicable
Add lifetime test case for `new_ret_no_self`
rustup rust-lang#71215
Downgrade match_bool to pedantic
Run fetch before testing if master contains beta
The beta branch update should not require a force push
Add a note to the beta sections of release.md
Remove apt-get upgrade again
Always use the deploy script and templates of the master branch
README: fix lit count line
clippy_dev: make it fatal when the regex for updating lint count does not match
`predecessors_for` will be removed soon
Rustup "Remove `BodyAndCache`"
Only run (late) internal lints, when they are warn/deny/forbid
Only run cargo lints, when they are warn/deny/forbid
span_lint_and_note now takes an Option<Span> for the note_span instead of just a span
Make lint also capture blocks and closures, adjust language to mention other mutex types
don't test the code in the lint docs
Switch to matching against full paths instead of just the last element of the path
Lint for holding locks across await points
Also mention `--fix` for nightly users
fix crash on issue-69020-assoc-const-arith-overflow.rs
Address review comments
remark fixes
Update CHANGELOG.md for Rust 1.43 and 1.44
update stderr file
util/fetch_prs_between.sh: Add Markdown formatted Link
factor ifs into function, add differing mutex test
Update the changelog update documentation
Apply suggestions from PR review
update span_lint_and_help call to six args
test for mutex eq, add another test case
use if chain
cargo dev fmt
fix map import to rustc_middle
dev update_lints
fix internal clippy warnings
change visitor name to OppVisitor
use Visitor api to find Mutex::lock calls
add note about update-all-refs script, revert redundant pat to master
move closures to seperate fns, remove known problems
use span_lint_and_help, cargo dev fmt
creating suggestion
progress work on suggestion for auto fix
Implement unsafe_derive_deserialize lint
Update empty_enum.stderr
Formatting and naming
Formatting and naming
Cleanup: `node_id` -> `hir_id`
Fix issue rust-lang#2907.
Don't trigger toplevel_ref_arg for `for` loops
Cleanup: future_not_send: use `return_ty` method
Remove badge FIXME from Cargo.toml
Change note_span argument for span_lint_and_note.
Add an Option<Span> argument to span_lint_and_help.
Fixes internal lint warning in code base.
Implement collapsible_span_lint_calls lint.
````

Fixes rust-lang#71453
@mark-i-m mark-i-m deleted the def-kind-more branch April 26, 2020 17:49
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 5, 2020
Changes:
````
rustup to rust-lang/rust#70043
map_clone: avoid suggesting `copied()` for &mut
fix redundant_pattern_matching lint
Add tests for rust-lang#1654
Don't trigger while_let_on_iterator when the iterator is recreated every iteration
Update issue_2356.stderr reference file
Update while_let_on_iterator tests
Fix while_let_on_iterator suggestion and make it MachineApplicable
Add lifetime test case for `new_ret_no_self`
rustup rust-lang/rust#71215
Downgrade match_bool to pedantic
Run fetch before testing if master contains beta
The beta branch update should not require a force push
Add a note to the beta sections of release.md
Remove apt-get upgrade again
Always use the deploy script and templates of the master branch
README: fix lit count line
clippy_dev: make it fatal when the regex for updating lint count does not match
`predecessors_for` will be removed soon
Rustup "Remove `BodyAndCache`"
Only run (late) internal lints, when they are warn/deny/forbid
Only run cargo lints, when they are warn/deny/forbid
span_lint_and_note now takes an Option<Span> for the note_span instead of just a span
Make lint also capture blocks and closures, adjust language to mention other mutex types
don't test the code in the lint docs
Switch to matching against full paths instead of just the last element of the path
Lint for holding locks across await points
Also mention `--fix` for nightly users
fix crash on issue-69020-assoc-const-arith-overflow.rs
Address review comments
remark fixes
Update CHANGELOG.md for Rust 1.43 and 1.44
update stderr file
util/fetch_prs_between.sh: Add Markdown formatted Link
factor ifs into function, add differing mutex test
Update the changelog update documentation
Apply suggestions from PR review
update span_lint_and_help call to six args
test for mutex eq, add another test case
use if chain
cargo dev fmt
fix map import to rustc_middle
dev update_lints
fix internal clippy warnings
change visitor name to OppVisitor
use Visitor api to find Mutex::lock calls
add note about update-all-refs script, revert redundant pat to master
move closures to seperate fns, remove known problems
use span_lint_and_help, cargo dev fmt
creating suggestion
progress work on suggestion for auto fix
Implement unsafe_derive_deserialize lint
Update empty_enum.stderr
Formatting and naming
Formatting and naming
Cleanup: `node_id` -> `hir_id`
Fix issue rust-lang#2907.
Don't trigger toplevel_ref_arg for `for` loops
Cleanup: future_not_send: use `return_ty` method
Remove badge FIXME from Cargo.toml
Change note_span argument for span_lint_and_note.
Add an Option<Span> argument to span_lint_and_help.
Fixes internal lint warning in code base.
Implement collapsible_span_lint_calls lint.
````

Fixes #71453
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.

9 participants