rustc_middle: return `LocalDefId` where possible in hir::map module#70986

marmeladema · 2020-04-10T11:31:08Z

This changes the return type of the following functions to return a LocalDefId instead of a DefId:

opt_local_def_id_from_node_id
opt_local_def_id
body_owner_def_id
local_def_id_from_node_id
get_parent_id

This is another step in the right direction for #70853

This pull request will be followed by another (substantial one) which changes the return type of local_def_id function but this change being more invasive, we might want to wait for #70956 or #70961 (or some other form it) to land first.

…id_from_node_id

…rom_node_id

marmeladema · 2020-04-10T11:33:43Z

cc @ecstatic-morse @eddyb

eddyb · 2020-04-10T11:54:36Z

src/librustc_mir/transform/mod.rs


    // All body-owners have MIR associated with them.
-    set.extend(tcx.body_owners());
+    set.extend(tcx.body_owners().map(LocalDefId::to_def_id));


This query should return a &'tcx FxHashSet<LocalDefId> instead of &'tcx DefIdSet (which AFAIK is just FxHashSet<DefId> - IMO we should stop using the aliases, they only exist because NodeMap was the first iteration of FxHashMap, ages ago, and we never moved on from that).

(this doesn't have to be done in this PR, especially if you plan to tweak queries in a different PR)

eddyb · 2020-04-10T11:57:16Z

@bors r+ rollup=never (for perf)

bors · 2020-04-10T11:57:17Z

📌 Commit b6b0057 has been approved by eddyb

Centril · 2020-04-10T12:09:02Z

Let's check in advance if it has any perf impact; @bors try @rust-timer queue

bors · 2020-04-10T12:09:04Z

🙅 Please do not try after a pull request has been r+ed. If you need to try, unapprove (r-) it first.

rust-timer · 2020-04-10T12:09:04Z

Awaiting bors try build completion

eddyb · 2020-04-10T15:15:48Z

@Centril I don't see why it could cause a regression, but it may result in improvements, maybe I should be clearer on that.

Centril · 2020-04-10T16:15:27Z

@bors p=1

bors · 2020-04-10T23:13:18Z

⌛ Testing commit b6b0057 with merge f363450...

…dle-local-def-id, r=eddyb rustc_middle: return `LocalDefId` where possible in hir::map module This changes the return type of the following functions to return a `LocalDefId` instead of a `DefId`: * opt_local_def_id_from_node_id * opt_local_def_id * body_owner_def_id * local_def_id_from_node_id * get_parent_id This is another step in the right direction for rust-lang#70853 This pull request will be followed by another (substantial one) which changes the return type of `local_def_id` function but this change being more invasive, we might want to wait for rust-lang#70956 or rust-lang#70961 (or some other form it) to land first.

bors · 2020-04-11T02:15:33Z

☀️ Test successful - checks-azure
Approved by: eddyb
Pushing f363450 to master...

eddyb · 2020-04-12T00:46:14Z

perf results look pretty neutral, within noise margins.

Changes: ```` Allow UUID style formatting for `inconsistent_digit_grouping` lint rustup rust-lang#70986 rustup rust-lang#69745 Rustup to rust-lang#70913 compare with the second largest instead of the smallest variant Revert "Downgrade new_ret_no_self to pedantic" Check for clone-on-copy in argument positions Check fn header along with decl when suggesting to implement trait Downgrade implicit_hasher to pedantic Move cognitive_complexity to nursery Run fmt and update test Use int assoc consts in MANUAL_SATURATING_ARITHMETIC Use int assoc consts in checked_conversions lint Use primitive type assoc consts in more tests Use integer assoc consts in more lint example code Don't import primitive type modules Use assoc const NAN for zero_div_zero lint Fix float cmp to use assoc fxx::EPSILON Fix NAN comparison lint to use assoc NAN Refine lint message. Lint on opt.as_ref().map(|x| &**x). Include OpAssign in suspicious_op_assign_impl result_map_or_into_option: fix syntax error in example result_map_or_into: fix dogfood_clippy error => {h,l}int CONTRIBUTING.md: fix broken triage link result_map_or_into_option: fix `cargo dev fmt --check` errors result_map_or_into_option: move arg checks into tuple assignment result_map_or_into_option: add `opt.map_or(None, |_| Some(y))` test result_map_or_into_option: destructure lint tuple or return early result_map_or_into_option: add good and bad examples result_map_or_into_option: explicitly note absence of known problems Downgrade new_ret_no_self to pedantic Downgrade unreadable_literal to pedantic Update CONTRIBUTING.md Rename rustc -> rustc_middle in doc links result_map_or_into_option: add lint to catch manually adpating Result -> Option Move matches test in matches module Run update_lints Make lint modules private Don't filter lints in code generation functions Build lint lists once and the reuse them to update files Get rid of Lint::is_internal method Clean up update_lints Downgrade inefficient_to_string to pedantic Downgrade trivially_copy_pass_by_ref to pedantic Downgrade let_unit_value to pedantic ```` Fixes rust-lang#70993

@Dylan-DPC

submodules: update clippy from d342cee to af5940b Changes: ```` Allow UUID style formatting for `inconsistent_digit_grouping` lint rustup rust-lang#70986 rustup rust-lang#69745 Rustup to rust-lang#70913 compare with the second largest instead of the smallest variant Revert "Downgrade new_ret_no_self to pedantic" Check for clone-on-copy in argument positions Check fn header along with decl when suggesting to implement trait Downgrade implicit_hasher to pedantic Move cognitive_complexity to nursery Run fmt and update test Use int assoc consts in MANUAL_SATURATING_ARITHMETIC Use int assoc consts in checked_conversions lint Use primitive type assoc consts in more tests Use integer assoc consts in more lint example code Don't import primitive type modules Use assoc const NAN for zero_div_zero lint Fix float cmp to use assoc fxx::EPSILON Fix NAN comparison lint to use assoc NAN Refine lint message. Lint on opt.as_ref().map(|x| &**x). Include OpAssign in suspicious_op_assign_impl result_map_or_into_option: fix syntax error in example result_map_or_into: fix dogfood_clippy error => {h,l}int CONTRIBUTING.md: fix broken triage link result_map_or_into_option: fix `cargo dev fmt --check` errors result_map_or_into_option: move arg checks into tuple assignment result_map_or_into_option: add `opt.map_or(None, |_| Some(y))` test result_map_or_into_option: destructure lint tuple or return early result_map_or_into_option: add good and bad examples result_map_or_into_option: explicitly note absence of known problems Downgrade new_ret_no_self to pedantic Downgrade unreadable_literal to pedantic Update CONTRIBUTING.md Rename rustc -> rustc_middle in doc links result_map_or_into_option: add lint to catch manually adpating Result -> Option Move matches test in matches module Run update_lints Make lint modules private Don't filter lints in code generation functions Build lint lists once and the reuse them to update files Get rid of Lint::is_internal method Clean up update_lints Downgrade inefficient_to_string to pedantic Downgrade trivially_copy_pass_by_ref to pedantic Downgrade let_unit_value to pedantic ```` Fixes rust-lang#70993 r? @Dylan-DPC

Changes: ```` Allow UUID style formatting for `inconsistent_digit_grouping` lint rustup rust-lang/rust#70986 rustup rust-lang/rust#69745 Rustup to rust-lang/rust#70913 compare with the second largest instead of the smallest variant Revert "Downgrade new_ret_no_self to pedantic" Check for clone-on-copy in argument positions Check fn header along with decl when suggesting to implement trait Downgrade implicit_hasher to pedantic Move cognitive_complexity to nursery Run fmt and update test Use int assoc consts in MANUAL_SATURATING_ARITHMETIC Use int assoc consts in checked_conversions lint Use primitive type assoc consts in more tests Use integer assoc consts in more lint example code Don't import primitive type modules Use assoc const NAN for zero_div_zero lint Fix float cmp to use assoc fxx::EPSILON Fix NAN comparison lint to use assoc NAN Refine lint message. Lint on opt.as_ref().map(|x| &**x). Include OpAssign in suspicious_op_assign_impl result_map_or_into_option: fix syntax error in example result_map_or_into: fix dogfood_clippy error => {h,l}int CONTRIBUTING.md: fix broken triage link result_map_or_into_option: fix `cargo dev fmt --check` errors result_map_or_into_option: move arg checks into tuple assignment result_map_or_into_option: add `opt.map_or(None, |_| Some(y))` test result_map_or_into_option: destructure lint tuple or return early result_map_or_into_option: add good and bad examples result_map_or_into_option: explicitly note absence of known problems Downgrade new_ret_no_self to pedantic Downgrade unreadable_literal to pedantic Update CONTRIBUTING.md Rename rustc -> rustc_middle in doc links result_map_or_into_option: add lint to catch manually adpating Result -> Option Move matches test in matches module Run update_lints Make lint modules private Don't filter lints in code generation functions Build lint lists once and the reuse them to update files Get rid of Lint::is_internal method Clean up update_lints Downgrade inefficient_to_string to pedantic Downgrade trivially_copy_pass_by_ref to pedantic Downgrade let_unit_value to pedantic ```` Fixes #70993

marmeladema added 5 commits April 10, 2020 12:13

librustc_middle: return LocalDefId instead of DefId in opt_local_def_…

bc30e4d

…id_from_node_id

librustc_middle: return LocalDefId instead of DefId in opt_local_def_id

1dc363b

librustc_middle: return LocalDefId instead of DefId in body_owner_def_id

f62c6e1

librustc_middle: return LocalDefId instead of DefId in local_def_id_f…

555e024

…rom_node_id

librustc_middle: return LocalDefId instead of DefId in get_parent_did

b6b0057

rust-highfive assigned estebank Apr 10, 2020

eddyb reviewed Apr 10, 2020

View reviewed changes

eddyb approved these changes Apr 10, 2020

View reviewed changes

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

Dylan-DPC-zz mentioned this pull request Apr 10, 2020

Rollup of 4 pull requests #71011

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 11, 2020

bors merged commit f363450 into rust-lang:master Apr 11, 2020

matthiaskrgr mentioned this pull request Apr 11, 2020

rustup https://github.com/rust-lang/rust/pull/69745 rust-lang/rust-clippy#5450

Merged

matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this pull request Apr 11, 2020

rustup rust-lang/rust#70986

4352c85

marmeladema deleted the issue70853/librustc_middle-local-def-id branch April 18, 2020 12:42

Uh oh!

Conversation

marmeladema commented Apr 10, 2020

Uh oh!

marmeladema commented Apr 10, 2020

Uh oh!

eddyb Apr 10, 2020

Choose a reason for hiding this comment

Uh oh!

eddyb Apr 10, 2020

Choose a reason for hiding this comment

Uh oh!

eddyb commented Apr 10, 2020

Uh oh!

bors commented Apr 10, 2020

Uh oh!

Centril commented Apr 10, 2020

Uh oh!

bors commented Apr 10, 2020

Uh oh!

rust-timer commented Apr 10, 2020

Uh oh!

eddyb commented Apr 10, 2020

Uh oh!

Centril commented Apr 10, 2020

Uh oh!

bors commented Apr 10, 2020

Uh oh!

bors commented Apr 11, 2020

Uh oh!

eddyb commented Apr 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants