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

typeck: extract expr type-checking to expr.rs + refactor check_expr_kind #61857

Merged
merged 22 commits into from
Jun 17, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jun 15, 2019

In this PR we:

  • Extract out the bulk of the expression type checking logic from check/mod.rs into a new file check/expr.rs.

  • Refactor fn check_expr_kind into several smaller functions.

More functions should probably be moved but I think this is a reasonable start.

r? @oli-obk
cc @eddyb

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 15, 2019
@rust-highfive

This comment has been minimized.

@Centril Centril force-pushed the typeck-extract-expr branch from c01e37f to 5057552 Compare June 15, 2019 02:01
@@ -68,6 +68,7 @@ This API is completely unstable and subject to change.
#![feature(rustc_diagnostic_macros)]
#![feature(slice_patterns)]
#![feature(never_type)]
#![feature(inner_deref)]
Copy link
Contributor

Choose a reason for hiding this comment

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

what's up with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used in some places, e.g. https://github.com/rust-lang/rust/pull/61857/files#diff-4dca14d9f8d48a6af9ed414a126a9823R243 (just search for .deref() in expr.rs) to make things type-check :)

Copy link
Member

Choose a reason for hiding this comment

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

Is that an addition to Option? Damn, I think it's 1. confusing 2. we should be making progress on the coerce front instead of adding method hacks :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep; there's a PR to rename it to .as_deref(). It's much less of a hack than using .map(...). =P

Copy link
Member

Choose a reason for hiding this comment

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

We should probably never stabilize it and introduce the structural coercion already, hmpf.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 17, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Jun 17, 2019

📌 Commit 5057552 has been approved by oli-obk

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 17, 2019
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 17, 2019
@bors
Copy link
Contributor

bors commented Jun 17, 2019

⌛ Testing commit 5057552 with merge b01a257...

bors added a commit that referenced this pull request Jun 17, 2019
typeck: extract expr type-checking to expr.rs + refactor check_expr_kind

In this PR we:

- Extract out the bulk of the expression type checking logic from `check/mod.rs` into a new file `check/expr.rs`.

- Refactor `fn check_expr_kind` into several smaller functions.

More functions should probably be moved but I think this is a reasonable start.

r? @oli-obk
cc @eddyb
@bors
Copy link
Contributor

bors commented Jun 17, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk
Pushing b01a257 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 17, 2019
@bors bors merged commit 5057552 into rust-lang:master Jun 17, 2019
@Centril Centril deleted the typeck-extract-expr branch June 17, 2019 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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