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 document on consts in patterns #48

Merged
merged 6 commits into from
Aug 10, 2020
Merged

Conversation

RalfJung
Copy link
Member

Write down what I know about consts in patterns. There are some TODO here that I'll need help to fill in -- Cc @oli-obk @lcnr.

Cc #42

patterns.md Show resolved Hide resolved
patterns.md Outdated Show resolved Hide resolved
patterns.md Outdated Show resolved Hide resolved
patterns.md Outdated Show resolved Hide resolved
Co-authored-by: Oliver Scherer <github35764891676564198441@oli-obk.de>
patterns.md Outdated
However, that check has some loopholes, so e.g. `&T` can be used in a pattern no matter the `T`.

Any reference type const-pattern is compiled by [calling `PartialEq::eq`][compile-partial-eq] to compare the subject of the `match` with the constant. Const-patterns with other types (enum, struct, tuple, array) are treated as if the constant was inlined as a pattern (and the usual `match` tree is constructed).
Note that this is in contradiction with what has been RFC'd, which specifies that we should always call `PartialEq::eq`.
Copy link
Member Author

Choose a reason for hiding this comment

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

Would be good to link to the RFC here. I assume that's https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md but so far I have not found where it says that we should use PartialEq.

Copy link
Member Author

Choose a reason for hiding this comment

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

The RFC calls this "semantic equality" and makes some preparation for maybe switching to it in the future, but it dosn't actually say that we want to adopt semantic equality. That's my reading, anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

But later the RFC explicitly says

The goal of this RFC is not to decide between semantic and structural equality. Rather, the goal is to restrict pattern matching to that subset of types where the two variants behave roughly the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's very explicit in the detailed design section

When converting a constant value into a pattern, if the constant is of struct or enum type, we will check whether this attribute is present on the struct -- if so, we will convert the value as we do today. If not, we will report an error that the struct/enum value cannot be used in a pattern.

where "as we do today" is "convert to an equivalent pattern"

Copy link
Member Author

@RalfJung RalfJung Aug 10, 2020

Choose a reason for hiding this comment

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

I don't think that is more explicit than "The goal of this RFC is not to decide between semantic and structural equality".

That text you quoted does not say anywhere that we should use semantic equality for anything.

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 adjusted the text to reflect what I think the RFC says.

@oli-obk oli-obk merged commit f19f101 into rust-lang:master Aug 10, 2020
@RalfJung RalfJung deleted the patterns branch August 10, 2020 13:25
@RalfJung
Copy link
Member Author

Thanks for the help @oli-obk :)

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.

2 participants