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

enum variants aren't considered to be const #5873

Closed
doy opened this issue Apr 13, 2013 · 27 comments
Closed

enum variants aren't considered to be const #5873

doy opened this issue Apr 13, 2013 · 27 comments
Labels
A-typesystem Area: The type system P-medium Medium priority

Comments

@doy
Copy link
Contributor

doy commented Apr 13, 2013

This compiles:

static i: int = 4;

fn main () {
    let x: [int, ..i] = [1, 2, 3, 4];
    println(fmt!("%?", x));
}

But this doesn't:

enum Foo {
    Bar = 4,
}
static i: int = Bar as int;

fn main () {
    let x: [int, ..i] = [1, 2, 3, 4];
    println(fmt!("%?", x));
}
test2.rs:7:11: 7:21 error: expected constant expr for vector length: Can't cast str to int
test2.rs:7     let x: [int, ..i] = [1, 2, 3, 4];
                      ^~~~~~~~~~

That error message doesn't seem to make sense, but that may be because it's just confused - I also get this error message in other circumstances, and I don't understand what the difference is:

src/info/builtin.rs:79:11: 79:52 error: expected constant expr for vector length: Non-constant path in constant expr
src/info/builtin.rs:79 static db: [[Option<&'static str>, ..ncap], ..nterm] = [
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
youknowone added a commit to youknowone/rust that referenced this issue Apr 15, 2013
This will help not to meet confusing errors.
In issue rust-lang#5873, the error was "expected constant expr for vector length: Can't cast str to int".
It was originally "expected constant expr for vector length: Non-constant path in constant expr" (though still invalid error).
This patch make the original error to be printed.
youknowone added a commit to youknowone/rust that referenced this issue Apr 15, 2013
This will help not to meet confusing errors.
In issue rust-lang#5873, the error was "expected constant expr for vector length: Can't cast str to int".
It was originally "expected constant expr for vector length: Non-constant path in constant expr"
This patch make the original error to be printed.
bors added a commit that referenced this issue Apr 16, 2013
This will help not to meet confusing errors.
In issue #5873, the error was "expected constant expr for vector length: Can't cast str to int".
It was originally "expected constant expr for vector length: Non-constant path in constant expr" (though still invalid error).
This patch make the original error to be printed.
graydon pushed a commit to graydon/rust that referenced this issue Apr 17, 2013
This will help not to meet confusing errors.
In issue rust-lang#5873, the error was "expected constant expr for vector length: Can't cast str to int".
It was originally "expected constant expr for vector length: Non-constant path in constant expr"
This patch make the original error to be printed.
@catamorphism
Copy link
Contributor

It seems like the constant checker believes that Bar is a constant, whereas the constant evaluator does not. They should agree. I'm not sure which behavior is right. Also, it seems counter-intuitive that you can't refer to a constant (i) in other constants.

Nominating for milestone 1, welll-defined.

@huonw
Copy link
Member

huonw commented Jul 30, 2013

Works now. Fixed by 877bba9 (possibly; at the very least, it includes a testcase).

@huonw huonw closed this as completed Jul 30, 2013
@tikue
Copy link
Contributor

tikue commented Aug 28, 2013

This doesn't seem to work now.

enum Category {
    A,
    B,
    C,
    NumCategories,
}
static NUM_CATEGORIES: uint = NumCategories as uint;

fn main() {
    let the_categories: [Category, ..NUM_CATEGORIES] = [A, B, C];
    printfln!(the_categories);
}

$ x86_64-apple-darwin/stage2/bin/rust run static_test.rs
static_test.rs:10:24: 10:52 error: expected constant expr for vector length: Non-constant path in constant expr
static_test.rs:10 let the_categories: [Category, ..NUM_CATEGORIES] = [A, B, C];

@bblum bblum reopened this Aug 28, 2013
@jdm
Copy link
Contributor

jdm commented Aug 28, 2013

I'm doubtful that the earlier testcase ever worked; 877bba9 explicitly calls out in the commit message that C style enums don't work in type signatures, only values.

@pnkfelix
Copy link
Member

Part of #5551; inherits its maturity milestone implicitly.

@jdm
Copy link
Contributor

jdm commented Aug 29, 2013

Just for the record, having integral enum constant support in type signatures is a complicated problem, since we end up needing to differentiate between types that can be checked without constant evaluation and types that require constant evaluation before they can be checked. Interleaving these two things looked exceedingly awkward when I attempted to tackle this before, hence why I punted on it.

@flaper87
Copy link
Contributor

triage bump.

The original example seems to work now:

enum Category {
    Bar = 4,
}
static i: int = Bar as int;

fn main () {
    let x: [int, ..i] = [1, 2, 3, 4];
}

This still fails, here's an updated example:

enum Category {
    A,
    B,
    C,
    NumCategories,
}
static NUM_CATEGORIES: uint = NumCategories as uint;

fn main() {
    let the_categories: [Category, ..NUM_CATEGORIES] = [A, B, C];
    println!("{}", the_categories);
}

@pnkfelix
Copy link
Member

I don't think the latter example (that attempts to const-eval NumCategories as uint should work anyway; I believe our policy has been that we reserve the right to change enum encoding if not otherwise provided a la C-style enums, so it would be ... risky at best to support the code as written.

(I do admit, it is a useful coding pattern. Though perhaps I might be able to whip up a macro that expands into the C-style enum. I will try to remember to try that tomorrow morning.)

@nikomatsakis
Copy link
Contributor

I consider the last case to be an anti-pattern in Rust. It creates an entry in the enum that doesn't exist and fouls up match statements. If we want to support this (which perhaps we should), we ought to add some way to get the number of variants in an enum for a constant -- but not by adding a fictitious entry to the enum.

@jaredly
Copy link

jaredly commented Mar 24, 2014

+1 for getting the number of variants in an enum

@thestinger thestinger added the A-typesystem Area: The type system label Sep 16, 2014
@frewsxcv
Copy link
Member

Visiting for triage

Updated the example to compile with the latest Rust

enum Category {
    A,
    B,
    C,
    NumCategories,
}
static NUM_CATEGORIES: uint = Category::NumCategories as uint;

fn main() {
    let the_categories: [Category; NUM_CATEGORIES] = [Category::A, Category::B, Category::C];
    println!("{}", the_categories);
}

Playpen

Can confirm it still doesn't compile



<anon>:7:24: 7:28 warning: the `uint` type is deprecated; use `usize` or a fixed-sized integer
<anon>:7 static NUM_CATEGORIES: uint = Category::NumCategories as uint;
                                ^~~~
<anon>:7:24: 7:28 help: add #![feature(int_uint)] to the crate attributes to silence this warning
<anon>:7 static NUM_CATEGORIES: uint = Category::NumCategories as uint;
                                ^~~~
<anon>:7:58: 7:62 warning: the `uint` type is deprecated; use `usize` or a fixed-sized integer
<anon>:7 static NUM_CATEGORIES: uint = Category::NumCategories as uint;
                                                                  ^~~~
<anon>:7:58: 7:62 help: add #![feature(int_uint)] to the crate attributes to silence this warning
<anon>:7 static NUM_CATEGORIES: uint = Category::NumCategories as uint;
                                                                  ^~~~
<anon>:10:25: 10:51 error: expected constant expr for array length: non-constant path in constant expr [E0250]
<anon>:10     let the_categories: [Category; NUM_CATEGORIES] = [Category::A, Category::B, Category::C];
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~
playpen: application terminated with error code 101
Program ended.

@pnkfelix
Copy link
Member

So, just to review the situation, and repeat what @nikomatsakis and I stated earlier:

  1. The original bug from the description has been fixed. Namely, these two programs both run:

    First program:

    const I1: usize = 4;
    
    fn main () {
        let x: [i32; I1] = [1, 2, 3, 4];
        println!("{:?}", x);
    }

    Second program:

    enum Foo {
        Bar = 4,
    }
    const I2: usize = Foo::Bar as usize;
    
    fn main () {
        let x: [i32; I2] = [1, 2, 3, 4];
        println!("{:?}", x);
    }
  2. There is another program from a comment above that does not work, but I do not think it was ever part of Rust's design that it would work as written:

#[derive(Debug)]
enum Category {
    A,
    B,
    C,
    NumCategories,
}
const NUM_CATEGORIES: usize = Category::NumCategories as usize;

use Category::*;

fn main() {
    let the_categories: [Category; NUM_CATEGORIES] = [A, B, C];
    println!("{:?}", the_categories);
}

This is the case where you get the following error:

<anon>:13:25: 13:51 error: expected constant expr for array length: non-constant path in constant expr [E0250]
<anon>:13     let the_categories: [Category; NUM_CATEGORIES] = [A, B, C];
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~

Footnote 1: both @pnkfelix and @nikomatsakis think the last program above is using an anti-pattern; that is, it is not idiomatic Rust to encode meta-information like "the number of enum variants" as a member itself of the enum. (We may extend the language in the future with some sort of associated constant that provides such information, but that is not part of the current design.)

Footnote 2: there is a straight-forward workaround, if one wants to insist on using the anti-pattern: Put an explicit value in for the NumCategories variant:

#[derive(Debug)]
enum Category {
    A,
    B,
    C,
    NumCategories = 3,
}
const NUM_CATEGORIES: usize = Category::NumCategories as usize;

use Category::*;

fn main() {
    let the_categories: [Category; NUM_CATEGORIES] = [A, B, C];
    println!("{:?}", the_categories);
}

Note also that the Rust compiler will catch if you put too low a value in for `NumCategories above, as long as you have not assigned "out-of-band" values to the earlier variants. In other words, this code will fail to compile:

#[derive(Debug)]
enum Category {
    A,
    B,
    C,
    NumCategories = 2, // this is too low!
}
const NUM_CATEGORIES: usize = Category::NumCategories as usize;

use Category::*;

fn main() {
    let the_categories: [Category; NUM_CATEGORIES] = [A, B];
    println!("{:?}", the_categories);
}

This yields the compile-time error:

<anon>:27:5: 27:22 error: discriminant value `2` already exists [E0081]
<anon>:27     NumCategories = 2, // this is too low!
              ^~~~~~~~~~~~~~~~~
<anon>:26:5: 26:6 note: conflicting discriminant here
<anon>:26     C,
              ^

(I suspect there may even be a straight-forward macro one could write that creates a static array of all the values, to ensure that the provided number is also not too large. Demonstrated here: http://is.gd/rGTANu Though such a macro would benefit from extensions like rust-lang/rfcs#407 for counting.)

@pnkfelix
Copy link
Member

I think we should close this ticket. Nominating for closure.

@pnkfelix
Copy link
Member

(closing)

@ghost
Copy link

ghost commented Mar 7, 2015

Let me get this straight. Rust doesn't allow using c-like enum variants in compile-time evaluation contexts in general, unless the user opts-in to this workaround you've pointed out. Library writers don't know which ones of their public c-like enum variants might be needed to be used as compile-time constants, so they'd have to explicitly specify the values for each variant.

To me this is clearly a compiler bug, and as far as I can tell, you refuse to fix it just because the person reporting it happened to be using an anti-pattern in demonstrating it.

@tikue
Copy link
Contributor

tikue commented Mar 7, 2015

It's backwards compatible to add implicit const ordinals, no? Likewise for a way to get const number of variants.

@pnkfelix
Copy link
Member

pnkfelix commented Mar 8, 2015

@tommit compiler bug != language feature


At one point I had a further notes here trying to further explain the above, but I think the comment thread is essentially self-explanatory, apart from one detail:

I believe the phrase "C-like enum variant" is meant to refer to variants that have been given an explicit value. The example that we have been calling an anti-pattern does not use any explicit values in the variants; therefore I do not think it qualifies as a C-like enum variant.

@pnkfelix
Copy link
Member

pnkfelix commented Mar 8, 2015

I believe the phrase "C-like enum variant" is meant to refer to variants that have been given an explicit value.

I might be wrong on this point; I will check more carefully when I am in front of a computer again

@ghost
Copy link

ghost commented Mar 8, 2015

The Reference claims the following:

Enums have a discriminant. You can assign them explicitly: enum Foo { Bar = 123 }

If a discriminant isn't assigned, they start at zero, and add one for each variant, in order.
You can cast an enum to get this value: let x = Foo::Bar as u32; // x is now 123u32

This only works as long as none of the variants have data attached. If it were Bar(i32), this is disallowed.

From that quote I gather, that given the enum E { A, B }, Rust guarantees that E::A as usize == 0_usize and E::B as usize == 1_usize. If Rust can make those guarantees in its reference manual now, then why can't it make the same guarantees at compile-time?

@pnkfelix You say that it's a language feature that these unassigned nullary enum variants (which belong to an enum whose all variants are nullary) aren't compile-time constants. What is the language design reasoning behind this feature?

@pnkfelix
Copy link
Member

pnkfelix commented Mar 8, 2015

@tommit I was not attempting to claim that it was a language feature that these aren't compile-time constants. (I do concede that my earlier response can be interpreted that way.)

I was trying to claim that changing the current language to treat them as compile-time constants would be a language feature, and thus should go through the RFC process. (After all, a ticket like this really isn't the appropriate venue to debate the value of such a change.)


Here's the essence of what I was thinking when I wrote that response: I have been long under the impression that we were reserving the right to change how discriminant values are assigned by the compiler if not explicitly assigned by programmer.

(Of course actually allowing such a change would probably require restricting the current language; i.e. Variant as Type when Variant would need to be outlawed during dynamic execution in addition to it being disallowed at compile time. And I imagine we could not do that unless we simultaneously accepted rust-lang/rfcs#639 and probably provided a stable API to that RFC as well.)

Its quite possible that my mental model of what we were planning to permit in the future was simply over-aggressive; i.e. that no one else on the team was planning on ever changing the assignment of discriminant values, at least not for enum variants that do not have a data payload attached.

@pnkfelix
Copy link
Member

pnkfelix commented Mar 8, 2015

reopening and putting on agenda for next meeting; this deserves discussion at the team meeting, if only to get clarification about what guarantees we plan with respect to discriminant value assignment.

@pnkfelix pnkfelix reopened this Mar 8, 2015
@ghost
Copy link

ghost commented Mar 8, 2015

@pnkfelix Thank you for getting this thing to get sorted out and for clearing up my earlier confusion.

@nikomatsakis
Copy link
Contributor

Hmm, I tend to agree with @tommit that it should work for both explicitly assigned and unassigned values, if it works at all.

@nikomatsakis
Copy link
Contributor

(I didn't fully appreciate when commenting before that it worked only in one case and not the other.)

@pnkfelix
Copy link
Member

So after discussing with @nikomatsakis i concluded that my mental model is simply wrong, and that there is not sufficient value in trying to future-proof our future discriminant value assignment. In particular @nikomatsakis pointed out to me multiple times that the chosen discriminant value need not be directly represented as the numeric value chosen; thus various tricks I was anticipating (usually revolving around ensuring that e.g. bit patterns are non-zero) should be able to be adopted in the future.

Anyway, the bugs been reopened; we should fix this, for consistency's sake.

@pnkfelix
Copy link
Member

triage: P-high

(In some sense this is a feature request, or at least a request for consistently deploying a particular feature. Fixing this may be important when/if const fn comes, but its hard to tell right now.)

@pnkfelix
Copy link
Member

Decided to file a fresh issue, #23898, to ease triage. (I.e. right now the main item left on this bug is hard to actually extract from the thread of conversation as presented.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-typesystem Area: The type system P-medium Medium priority
Projects
None yet
Development

No branches or pull requests