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

hir: add HirId to main Hir nodes #58079

Merged
merged 1 commit into from
Feb 3, 2019

Conversation

ljedrz
Copy link
Contributor

@ljedrz ljedrz commented Feb 2, 2019

This is the 1st PR in a series dedicated to HirId-ification, i.e. deprecating ast::NodeIds after the AST > HIR lowering process. The bigger proof of concept can be seen in #57578.

Phase 1: store HirId in all remaining (some already have it) main HIR nodes (excluding *Id objects).

  • Field
  • FieldPat
  • ForeignItem
  • GenericParam
  • Lifetime
  • MacroDef
  • PathSegment
  • PatKind::Binding
  • Stmt
  • StructField
  • TypeBinding
  • VariantData
  • WhereClause
  • WhereEqPredicate

r? @Zoxc
Cc @varkor

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 2, 2019
src/librustc/hir/lowering.rs Outdated Show resolved Hide resolved
src/librustc/hir/lowering.rs Outdated Show resolved Hide resolved
src/librustc/hir/lowering.rs Outdated Show resolved Hide resolved
@Zoxc
Copy link
Contributor

Zoxc commented Feb 2, 2019

I've looked over this and it looks good. r=me when you've addressed the comments

@ljedrz ljedrz force-pushed the HirIdification_phase_1 branch from 94a9507 to 55ef78e Compare February 2, 2019 16:35
@ljedrz
Copy link
Contributor Author

ljedrz commented Feb 2, 2019

@Zoxc thanks; comments addressed.

@Zoxc
Copy link
Contributor

Zoxc commented Feb 2, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Feb 2, 2019

📌 Commit 55ef78e has been approved by Zoxc

@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 Feb 2, 2019
@bors
Copy link
Contributor

bors commented Feb 2, 2019

⌛ Testing commit 55ef78e with merge 1c02eaefaea5870575f46c3edc77a1a5e9182664...

@bors
Copy link
Contributor

bors commented Feb 2, 2019

💔 Test failed - status-appveyor

@bors 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 Feb 2, 2019
@ljedrz
Copy link
Contributor Author

ljedrz commented Feb 2, 2019

Build exceeded allowed resource quotas

Is this spurious? Looks like it failed only in one of the environments.

@Mark-Simulacrum
Copy link
Member

I hope so, yes. If not we'll need to investigate which resource limit was exceeded -- I suspect disk, but not sure.

@bors retry

@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 Feb 2, 2019
@bors
Copy link
Contributor

bors commented Feb 3, 2019

⌛ Testing commit 55ef78e with merge ec7ecb3...

bors added a commit that referenced this pull request Feb 3, 2019
hir: add HirId to main Hir nodes

This is the 1st PR in a series dedicated to `HirId`-ification, i.e. deprecating `ast::NodeId`s after the AST > HIR lowering process. The bigger proof of concept can be seen in #57578.

**Phase 1**: store `HirId` in all remaining (some already have it) main HIR nodes (excluding `*Id` objects).

- [x] `Field`
- [x] `FieldPat`
- [x] `ForeignItem`
- [x] `GenericParam`
- [x] `Lifetime`
- [x] `MacroDef`
- [x] `PathSegment`
- [x] `PatKind::Binding`
- [x] `Stmt`
- [x] `StructField`
- [x] `TypeBinding`
- [x] `VariantData`
- [x] `WhereClause`
- [x] `WhereEqPredicate`

r? @Zoxc
Cc @varkor
@bors
Copy link
Contributor

bors commented Feb 3, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Zoxc
Pushing ec7ecb3 to master...

@bors bors merged commit 55ef78e into rust-lang:master Feb 3, 2019
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #58079!

Tested on commit ec7ecb3.
Direct link to PR: #58079

💔 clippy-driver on windows: test-fail → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
💔 clippy-driver on linux: test-fail → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Feb 3, 2019
Tested on commit rust-lang/rust@ec7ecb3.
Direct link to PR: <rust-lang/rust#58079>

💔 clippy-driver on windows: test-fail → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
💔 clippy-driver on linux: test-fail → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
@ljedrz ljedrz deleted the HirIdification_phase_1 branch February 3, 2019 04:10
bors added a commit to rust-lang/rust-clippy that referenced this pull request Feb 3, 2019
Fix breakage due to rust-lang/rust#58079

The rustc change added HirId to a few nodes. As I understand it, the plan is
to remove the NodeId from these nodes eventually. Where the NodeId was
not being matched, I used `..` to try and avoid further breakage. Where it
was, I used `_` to make the fix easier when NodeId is removed.
bors added a commit that referenced this pull request Feb 3, 2019
submodule: update clippy from 6ce78d1 to 3bda548

rust-lang/rust-clippy@6ce78d1...3bda548

Rustup: unused trim result
Auto merge of #3727 - phansch:rustup_unused_trim, r=matthiaskrgr  …
Travis: Don't run integration tests on every PR commit  …
Auto merge of #3726 - phansch:some_renaming, r=oli-obk  …
Fix ICE in vec_box lint and add run-rustfix  …
Make vec_box MachineApplicable
Remove conditionals from base builds  …
Adding lint for too many lines.
Updating number of lines for the failing test to be > 100.  …
Running util/dev to update README/CHANGELOG
Reworking function logic, and adding doc example.  …
Moving tests to ui-toml to make use of clippy.toml
rustfmt
Adding back tests, but also reducing threshold by 1
Updating to just warn for one test.
Fix test broken by removing comment.
Skipping check if in external macro.
Adding lint for too many lines.
Updating number of lines for the failing test to be > 100.  …
Moving tests to ui-toml to make use of clippy.toml
rustfmt
Adding back tests, but also reducing threshold by 1
Updating to just warn for one test.
Fix test broken by removing comment.
Changing single character string to a character match.
Updated readme.
Updating code to ignore rustfmt issue.
phansch and avborhanian
Update clippy_lints/src/types.rs  …
Update clippy_lints/src/types.rs  …
Auto merge of #3732 - phansch:fix_ice_3720, r=oli-obk  …
Auto merge of #3731 - phansch:travis, r=phansch  …
Auto merge of #2857 - avborhanian:master, r=phansch  …
Fix breakage due to #58079  …
Auto merge of #3736 - mikerite:fix-build-20190203, r=phansch  …

related with: #58024
g-bartoszek pushed a commit to g-bartoszek/rust-clippy that referenced this pull request Feb 7, 2019
The rustc change added HirId to a few nodes. As I understand it, the plan is
to remove the NodeId from these nodes eventually. Where the NodeId was
not being matched, I used `..` to try and avoid further breakage. Where it
was, I used `_` to make the fix easier when NodeId is removed.
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.

5 participants