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

Refactor match checking to use HAIR #36695

Merged
merged 13 commits into from
Oct 27, 2016
Merged

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Sep 24, 2016

Refactor match checking to use HAIR instead of HIR, fixing quite a few bugs in the process.

r? @eddyb

@@ -28,6 +26,8 @@ use self::cx::Cx;

pub mod cx;

pub use rustc_const_eval::pattern::{BindingMode, Pattern, PatternKind, FieldPattern};
Copy link
Member

Choose a reason for hiding this comment

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

Is moving parts of HAIR to rustc_const_eval a good idea? What about doing the opposite?
That is, move match checking to rustc_mir and perhaps even integrate it with MIR match building.

Copy link
Member

Choose a reason for hiding this comment

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

Another idea would be to move it to rustc_typeck (and maybe have HAIR definitions in rustc).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

match checking uses a completely different algorithm than match building. HAIR patterns are basically independent of the rest of HAIR.

ty::TySlice(_) => match *ctor {
Slice(length) => length,
ConstantValue(_) => {
// TODO: this is utterly wrong, but required for byte arrays
Copy link
Member

Choose a reason for hiding this comment

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

/build/src/librustc_const_eval/_match.rs:477: TODO is deprecated; use FIXME

Copy link
Contributor Author

@arielb1 arielb1 Sep 24, 2016

Choose a reason for hiding this comment

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

The TODO is the entire point of this PR. It will be removed before the merge.

@arielb1 arielb1 force-pushed the literal-match branch 2 times, most recently from 2200ac4 to 81fb5cf Compare September 29, 2016 22:33
@bors
Copy link
Contributor

bors commented Oct 1, 2016

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

@arielb1 arielb1 force-pushed the literal-match branch 4 times, most recently from d03f2d5 to 879cf61 Compare October 4, 2016 16:22
@bors
Copy link
Contributor

bors commented Oct 5, 2016

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

@bors
Copy link
Contributor

bors commented Oct 6, 2016

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

@bors
Copy link
Contributor

bors commented Oct 8, 2016

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

Convert byte literal pattern to byte array patterns when they are both
used together. so matching them is properly handled. I could've done the
conversion eagerly, but that could have caused a bad worst-case for
massive byte-array matches.

Fixes rust-lang#18027.
Fixes rust-lang#25051.
Fixes rust-lang#26510.
nested slice patterns have the same functionality as non-nested
ones, so flatten them in HAIR construction.

Fixes rust-lang#26158.
@arielb1 arielb1 changed the title [WIP] Refactor match checking to use HAIR Refactor match checking to use HAIR Oct 26, 2016
Copy link
Member

@eddyb eddyb left a comment

Choose a reason for hiding this comment

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

r=me with an explanation for the missing diagnostics, or a revert of their removal.



E0003: r##"
/*E0003: r##"
Copy link
Member

Choose a reason for hiding this comment

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

What's with these two commented diagnostics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

E0002 was merged into E0004, and E0003 refers to a feature that does not exist anymore.

[NAN, _] => {},
_ => {},
};
//~^^^ WARNING unmatchable NaN in pattern, use the is_nan method in a guard instead
Copy link
Member

Choose a reason for hiding this comment

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

Why are the NaN warnings gone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because you can't have floating-point patterns anymore.

@arielb1
Copy link
Contributor Author

arielb1 commented Oct 26, 2016

@bors r=eddyb

@bors
Copy link
Contributor

bors commented Oct 26, 2016

📌 Commit 4157850 has been approved by eddyb

@arielb1
Copy link
Contributor Author

arielb1 commented Oct 26, 2016

added back the test for #6804

@bors r=eddyb

@bors
Copy link
Contributor

bors commented Oct 26, 2016

📌 Commit 3f9ebb4 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Oct 27, 2016

⌛ Testing commit 3f9ebb4 with merge 4a58463...

bors added a commit that referenced this pull request Oct 27, 2016
Refactor match checking to use HAIR

Refactor match checking to use HAIR instead of HIR, fixing quite a few bugs in the process.

r? @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants