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

Constants involving dereferencing in pattern matching #25574

Closed
IvanUkhov opened this issue May 18, 2015 · 27 comments · Fixed by #46882
Closed

Constants involving dereferencing in pattern matching #25574

IvanUkhov opened this issue May 18, 2015 · 27 comments · Fixed by #46882
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@IvanUkhov
Copy link
Contributor

Constants that dereference string literals cannot be currently used for pattern matching as show below:

const A: [u8; 4] = *b"fooo";

fn main() {
    match *b"xxxx" {
        A => {},
        _ => {}
    }
}

Such constants can be used in other contexts without any problem. It is inconsistent, and a very prominent use case is missing.

@steveklabnik steveklabnik added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label May 18, 2015
@ghost ghost added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label May 19, 2015
@ghost
Copy link

ghost commented May 19, 2015

A rather easy task for a new compiler hacker (const_eval::const_expr_to_pat).

@barca
Copy link

barca commented May 20, 2015

I would love to try take a work on this, plus I would qualify as a new compiler hacker. I've been going through the code linked above, and I have some idea of what to do, although I will undoubtedly have questions later.

@ghost
Copy link

ghost commented May 20, 2015

@barca Happy to mentor, let me know how it goes! Thanks!

@barca
Copy link

barca commented May 24, 2015

@jakub- While I'm fairly confident about the code I've written to solve this issue, However I'm running into the issue of compiling the language from source. Its extremely slow to compile, and quite frustrating when a serious error occurs(which has unfortunately happened quite frequently). I was wondering if you had any tricks for speeding up this overall process. Thanks

@barca
Copy link

barca commented Jun 7, 2015

@jakub- So, I've been struggling with this issue, but have been actively working on it. up until now I have been trying to dereference the u8 literals or the vec that holds them, however have been unsuccessful in this. While it is more than possible that I have made a mistake in my implementation, I was wondering if you had an idea of how to approach this issue.

@brson
Copy link
Contributor

brson commented Jun 11, 2015

@pnkfelix @eddyb @nrc Any suggestions on how to proceed here? @barca posting your patch would probably help.

@nrc
Copy link
Member

nrc commented Jun 11, 2015

@barca on slow compiles, there's a few things you can do, make sure you're building with -j8 (you'll have to experiment with the exact number of jobs to find the optimal number for your machine) and only build the stage1 compiler (make rustc-stage1) rather than the full bootstrap. You can then test using the stage1 build and only do a full bootstrap when you are happy with your work. Other than that, long build times are just part of life for rust compiler hackers, sorry.

@nrc
Copy link
Member

nrc commented Jun 11, 2015

@barca please post your patch and I'll take a look, hard for me to know exactly what the problem is without seeing the code.

@barca
Copy link

barca commented Jun 11, 2015

@nrc thanks, I was able to find some blogs, I've been doing 4 jobs seems to work fast enough.
here's my code. I could be very wrong in my approach, and would appreciate any advice.

@nrc
Copy link
Member

nrc commented Jun 11, 2015

@barca I think you're pretty much on the right track. The pattern you end up with should be a PatLit wrapping an ExprVec, then you just need to convert your Vec of u8s into a Vec of LitInts (wrapped in ExprInts). There will be a little bit of a dance with P(..) to do that. Does that get you unstuck?

@barca
Copy link

barca commented Jun 11, 2015

@nrc thats very helpful, thanks so much. people like you and @brson make me love rust!

@nrc
Copy link
Member

nrc commented Jun 11, 2015

❤️

@barca ping me on irc if you need any more help

@istaheev
Copy link

I came accross this issue and got stuck on it as an inexperienced Rust user. From my understanding the piece of code mentioned in the issue should be equal to the next one where I substitute the constant's value directly into the pattern:

fn main() {
    match *b"xxxx" {
        *b"fooo" => {},
        _ => {}
    }
}

It fails with

<anon>:3:9: 3:10 error: unexpected token: `*`
<anon>:3         *b"fooo" => {},

which means (as far as I understand) that it is not enough to just add constant-to-pattern expansion but also modify match to support matching of dereferenced literals. Is it so or I missed something?

@eddyb
Copy link
Member

eddyb commented Jun 14, 2015

@istaheev it is not, as literals in patterns are more limited than constant expressions.
@jakub- maybe it is time to have a serious solution for this and transform patterns into a structure specific to rustc::middle. const_eval::const_expr_to_pat is such a hack...

That said, I believe what @nrc is suggesting (turning *b"foo" into [b'f', b'o', b'o']) would "just" work, although it might not be as efficient as just matching against b"foo".

@barca
Copy link

barca commented Jun 14, 2015

I'm essentially done with the fix, should I continue to work on it/ push it if you are going to change the structure completely?

@eddyb
Copy link
Member

eddyb commented Jun 14, 2015

@barca such restructuring may not happen for another couple months (or even years), so don't mind me and feel free to open a PR.

@nrc
Copy link
Member

nrc commented Jun 15, 2015

@barca: yes please!

@ghost
Copy link

ghost commented Jun 15, 2015

@barca My apologies for ignoring your messages, the last month's been merciless for me time wise!

@code-ape
Copy link

I was wondering what the current state was for the issue. I'd be interested in contributing if more work needs to be done!

@barca
Copy link

barca commented Jul 14, 2015

@code-ape actually still working on the issue, although haven't had time to do it in the last few days since I was moving. I believe I am fairly close to solution, although am eternally optimistic regarding such issues.

@solson
Copy link
Member

solson commented Sep 25, 2015

I've been looking through E-easy issues for something to do. It sounds like this was close to done, but it's been a few months now.

@barca Any updates?

@eddyb Is the restructuring you mentioned before any closer to happening (or already happened)?

@eddyb
Copy link
Member

eddyb commented Sep 25, 2015

@tsion The MIR has landed (and it has its own pattern lowering), though I don't know if it supports this usecase correctly.
Also, nothing uses the MIR yet, that will happen in the following months.
cc @nikomatsakis

@nikomatsakis
Copy link
Contributor

In general the treatment of constants in pattern checking is somewhat broken, actually (at least in my opinion). I've been meaning to write up something on this, will do so soon, but the TL;DR is that we always use structural equality, even if (e.g.) the Eq trait is not implemented, or is implemented differently.

That said, I imagine this use case ought to work, but there are many others that don't which I don't really think ought to work. I'm not sure I'd call this an E-easy bug, though, but I guess that's because I tend to think that we need to do more overhaul of our constant treatment (and especially in patterns).

@cramertj
Copy link
Member

cramertj commented May 5, 2016

@eddyb @nikomatsakis Is this still E-easy or similar? I'd be interested in contributing if possible.

@eddyb
Copy link
Member

eddyb commented May 5, 2016

@cramertj The transition to MIR-based constant evaluation should fix this.

@eddyb eddyb added A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html and removed E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels May 5, 2016
@cramertj
Copy link
Member

cramertj commented May 5, 2016

Ah, okay. Thanks.

@steveklabnik
Copy link
Member

Triage: as far as I know, const evaluation is still undergoing an overhaul, I don't know if it's far enough along to work on this yet.

bors added a commit that referenced this issue Jan 18, 2018
Replace all const evaluation with miri

* error reporting in constants prints a stacktrace through all called const fns
* Trivial constant propagation and folding in MIR (always active, irrelevant of the optimization level)
* can now use floating constants in patterns (previously only floating point literals were allowed)
    * the future compat lint is still produced for both cases
* can index into constant arrays during const eval (previously feature gated)
* can create a constant union value with field `a` and read from field `b`
* can dereference references into constants
* can create references inside constants (`const X: &u32 = &22`)
* Tuple struct constructors can be used in constants
* regression in const eval errors spans (some of these need improvements in mir debug info)
* can cast floats to ints and vice versa (in constants, and even nan/inf constants)
* Mir dump prints false/true instead of 0u8/1u8
* `1i8 >> [8][0]` does not lint about exceeding bitshifts anymore.
    * Needs const propagation across projections
* `foo[I]` produces a const eval lint if `foo: [T; N]` and `N < I`
    * Essentially all builtin panics produce lints if they can be statically proven to trigger at runtime. This is on a best effort basis, so there might be some complex cases that don't trigger. (The runtime panic stays there, irrelevant of whether the lint is produced or not)

fixes #34997 (stack overflow with many constants)
fixes #25574 (deref byte strings in patterns)
fixes #27918 (broken mir ICE)
fixes #46114 (ICE on struct constructors in patterns)
fixes #37448 (`SomeStruct { foo } as SomeStruct`)
fixes #43754 (`return` in const fn)
fixes #41898 (tuple struct constructors)
fixes #31364 (infinite recursion with const fn, fixed by miri's recursion limit)
closes #29947 (const indexing stabilization)

r? @eddyb
bors added a commit that referenced this issue Jan 30, 2018
Replace all const evaluation with miri

* error reporting in constants prints a stacktrace through all called const fns
* Trivial constant propagation and folding in MIR (always active, irrelevant of the optimization level)
* can now use floating constants in patterns (previously only floating point literals were allowed)
    * the future compat lint is still produced for both cases
* can index into constant arrays during const eval (previously feature gated)
* can create a constant union value with field `a` and read from field `b`
* can dereference references into constants
* can create references inside constants (`const X: &u32 = &22`)
* Tuple struct constructors can be used in constants
* regression in const eval errors spans (some of these need improvements in mir debug info)
* can cast floats to ints and vice versa (in constants, and even nan/inf constants)
* Mir dump prints false/true instead of 0u8/1u8
* `1i8 >> [8][0]` does not lint about exceeding bitshifts anymore.
    * Needs const propagation across projections
* `foo[I]` produces a const eval lint if `foo: [T; N]` and `N < I`
    * Essentially all builtin panics produce lints if they can be statically proven to trigger at runtime. This is on a best effort basis, so there might be some complex cases that don't trigger. (The runtime panic stays there, irrelevant of whether the lint is produced or not)

fixes #34997 (stack overflow with many constants)
fixes #25574 (deref byte strings in patterns)
fixes #27918 (broken mir ICE)
fixes #46114 (ICE on struct constructors in patterns)
fixes #37448 (`SomeStruct { foo } as SomeStruct`)
fixes #43754 (`return` in const fn)
fixes #41898 (tuple struct constructors)
fixes #31364 (infinite recursion with const fn, fixed by miri's recursion limit)
closes #29947 (const indexing stabilization)
fixes #45044 (pattern matching repeat expressions)

r? @eddyb
bors added a commit that referenced this issue Feb 6, 2018
Replace all const evaluation with miri

* error reporting in constants prints a stacktrace through all called const fns
* Trivial constant propagation and folding in MIR (always active, irrelevant of the optimization level)
* can now use floating constants in patterns (previously only floating point literals were allowed)
    * the future compat lint is still produced for both cases
* can index into constant arrays during const eval (previously feature gated)
* can create a constant union value with field `a` and read from field `b`
* can dereference references into constants
* can create references inside constants (`const X: &u32 = &22`)
* Tuple struct constructors can be used in constants
* regression in const eval errors spans (some of these need improvements in mir debug info)
* can cast floats to ints and vice versa (in constants, and even nan/inf constants)
* Mir dump prints false/true instead of 0u8/1u8
* `1i8 >> [8][0]` does not lint about exceeding bitshifts anymore.
    * Needs const propagation across projections
* `foo[I]` produces a const eval lint if `foo: [T; N]` and `N < I`
    * Essentially all builtin panics produce lints if they can be statically proven to trigger at runtime. This is on a best effort basis, so there might be some complex cases that don't trigger. (The runtime panic stays there, irrelevant of whether the lint is produced or not)
* can use `union`s to implement `transmute` for `Copy` types in constants without a feature gate. With all the greatness and nasal demons that come with this.

fixes #34997 (stack overflow with many constants)
fixes #25574 (deref byte strings in patterns)
fixes #27918 (broken mir ICE)
fixes #46114 (ICE on struct constructors in patterns)
fixes #37448 (`SomeStruct { foo } as SomeStruct`)
fixes #43754 (`return` in const fn)
fixes #41898 (tuple struct constructors)
fixes #31364 (infinite recursion with const fn, fixed by miri's recursion limit)
closes #29947 (const indexing stabilization)
fixes #45044 (pattern matching repeat expressions)
fixes #47971 (ICE on const fn + references)

r? @eddyb
bors added a commit that referenced this issue Mar 7, 2018
Replace all const evaluation with miri

* error reporting in constants prints a stacktrace through all called const fns
* Trivial constant propagation and folding in MIR (always active, irrelevant of the optimization level)
* can now use floating constants in patterns (previously only floating point literals were allowed)
    * the future compat lint is still produced for both cases
* can index into constant arrays during const eval (previously feature gated)
* can create a constant union value with field `a` and read from field `b`
* can dereference references into constants
* can create references inside constants (`const X: &u32 = &22`)
* Tuple struct constructors can be used in constants
* regression in const eval errors spans (some of these need improvements in mir debug info)
* can cast floats to ints and vice versa (in constants, and even nan/inf constants)
* Mir dump prints false/true instead of 0u8/1u8
* `1i8 >> [8][0]` does not lint about exceeding bitshifts anymore.
    * Needs const propagation across projections
* `foo[I]` produces a const eval lint if `foo: [T; N]` and `N < I`
    * Essentially all builtin panics produce lints if they can be statically proven to trigger at runtime. This is on a best effort basis, so there might be some complex cases that don't trigger. (The runtime panic stays there, irrelevant of whether the lint is produced or not)
* can use `union`s to implement `transmute` for `Copy` types in constants without a feature gate. With all the greatness and nasal demons that come with this.
* can convert integers to `&'static T` in constants (useful for embedded)

fixes #34997 (stack overflow with many constants)
fixes #25574 (deref byte strings in patterns)
fixes #27918 (broken mir ICE)
fixes #46114 (ICE on struct constructors in patterns)
fixes #37448 (`SomeStruct { foo } as SomeStruct`)
fixes #43754 (`return` in const fn)
fixes #41898 (tuple struct constructors)
fixes #31364 (infinite recursion with const fn, fixed by miri's recursion limit)
closes #29947 (const indexing stabilization)
fixes #45044 (pattern matching repeat expressions)
fixes #47971 (ICE on const fn + references)
fixes #48081 (ICE on cyclic assoc const error)
fixes #48746 (nonhelpful error message with unions)

r? @eddyb

even though 1k loc are added in tests, this PR reduces the loc in this repository by 700
bors added a commit that referenced this issue Mar 8, 2018
Replace all const evaluation with miri

* error reporting in constants prints a stacktrace through all called const fns
* Trivial constant propagation and folding in MIR (always active, irrelevant of the optimization level)
* can now use floating constants in patterns (previously only floating point literals were allowed)
    * the future compat lint is still produced for both cases
* can index into constant arrays during const eval (previously feature gated)
* can create a constant union value with field `a` and read from field `b`
* can dereference references into constants
* can create references inside constants (`const X: &u32 = &22`)
* Tuple struct constructors can be used in constants
* regression in const eval errors spans (some of these need improvements in mir debug info)
* can cast floats to ints and vice versa (in constants, and even nan/inf constants)
* Mir dump prints false/true instead of 0u8/1u8
* `1i8 >> [8][0]` does not lint about exceeding bitshifts anymore.
    * Needs const propagation across projections
* `foo[I]` produces a const eval lint if `foo: [T; N]` and `N < I`
    * Essentially all builtin panics produce lints if they can be statically proven to trigger at runtime. This is on a best effort basis, so there might be some complex cases that don't trigger. (The runtime panic stays there, irrelevant of whether the lint is produced or not)
* can use `union`s to implement `transmute` for `Copy` types in constants without a feature gate. With all the greatness and nasal demons that come with this.
* can convert integers to `&'static T` in constants (useful for embedded)

fixes #34997 (stack overflow with many constants)
fixes #25574 (deref byte strings in patterns)
fixes #27918 (broken mir ICE)
fixes #46114 (ICE on struct constructors in patterns)
fixes #37448 (`SomeStruct { foo } as SomeStruct`)
fixes #43754 (`return` in const fn)
fixes #41898 (tuple struct constructors)
fixes #31364 (infinite recursion with const fn, fixed by miri's recursion limit)
closes #29947 (const indexing stabilization)
fixes #45044 (pattern matching repeat expressions)
fixes #47971 (ICE on const fn + references)
fixes #48081 (ICE on cyclic assoc const error)
fixes #48746 (nonhelpful error message with unions)

r? @eddyb

even though 1k loc are added in tests, this PR reduces the loc in this repository by 700
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.