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

Non-constant repeat count expr causes ICE #4063

Closed
therealgymmy opened this Issue Nov 29, 2012 · 19 comments

Comments

Projects
None yet
7 participants
@therealgymmy
Contributor

therealgymmy commented Nov 29, 2012

Current (minimized) example of breakage:

(courtesy of youknowone; see comments below)

fn main() { let _a = [0, ..1 as uint]; }

Original Report:

The following code causes the crash

enum State { ST_NULL, ST_WHITESPACE }

priv fn SomeFunction () -> StateTable {
    StateTable { entry: ~[mut ~[mut ST_NULL, ..ST_WHITESPACE], ..128] }
}

The error messages are as follows:

error: internal compiler error: unexpected failure
note: the compiler hit an unexpected failure path. this is a bug

It seems that the compiler does not understand entry: ~[mut ~[mut ST_NULL, ..ST_WHITESPACE], ..128].

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Nov 29, 2012

Contributor

The error happens because you're using ST_WHITESPACE as the repeat count; enum variants are never considered constants, and the repeat count has to be constant. It might be reasonable to consider C-like enums to be constants, but in this case I suspect it's not what you intended.

Of course, the error message needs to be improved. Changing the title to reflect that.

Contributor

catamorphism commented Nov 29, 2012

The error happens because you're using ST_WHITESPACE as the repeat count; enum variants are never considered constants, and the repeat count has to be constant. It might be reasonable to consider C-like enums to be constants, but in this case I suspect it's not what you intended.

Of course, the error message needs to be improved. Changing the title to reflect that.

@therealgymmy

This comment has been minimized.

Show comment
Hide comment
@therealgymmy

therealgymmy Nov 29, 2012

Contributor

Hi, as I don't see any documentation on this. Is there a way to use an enum (or a similar thing to C enums) as a repeat count (to force it to be a constant)?

Contributor

therealgymmy commented Nov 29, 2012

Hi, as I don't see any documentation on this. Is there a way to use an enum (or a similar thing to C enums) as a repeat count (to force it to be a constant)?

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Nov 29, 2012

Contributor

I tried doing:

enum State { ST_NULL, ST_WHITESPACE }

struct StateTable {
    entry: ~[mut ~[mut State]],
}

priv fn SomeFunction () -> StateTable {
    StateTable { entry: ~[mut ~[mut ST_NULL, ..(ST_WHITESPACE as uint)], ..128] }
}

(note the addition of as uint) -- but that yielded:

error: internal compiler error: node_id_to_type: no type for node `expr ST_WHITESPACE as uint (id=22)`

Noting this so it can be fixed too.

Contributor

catamorphism commented Nov 29, 2012

I tried doing:

enum State { ST_NULL, ST_WHITESPACE }

struct StateTable {
    entry: ~[mut ~[mut State]],
}

priv fn SomeFunction () -> StateTable {
    StateTable { entry: ~[mut ~[mut ST_NULL, ..(ST_WHITESPACE as uint)], ..128] }
}

(note the addition of as uint) -- but that yielded:

error: internal compiler error: node_id_to_type: no type for node `expr ST_WHITESPACE as uint (id=22)`

Noting this so it can be fixed too.

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Nov 29, 2012

Contributor

I think a better way to do what you're trying to do would be to use the vec::from_elem function.

Contributor

catamorphism commented Nov 29, 2012

I think a better way to do what you're trying to do would be to use the vec::from_elem function.

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Nov 29, 2012

Contributor

However, as a third thing to fix as part of this bug, I'll note that vector repeat expressions should be documented.

Contributor

catamorphism commented Nov 29, 2012

However, as a third thing to fix as part of this bug, I'll note that vector repeat expressions should be documented.

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Nov 29, 2012

Contributor

For the question of using enums as constants, see #3934

Contributor

catamorphism commented Nov 29, 2012

For the question of using enums as constants, see #3934

@therealgymmy

This comment has been minimized.

Show comment
Hide comment
@therealgymmy

therealgymmy Nov 29, 2012

Contributor

Thanks. :-)

Another thing I want to point out (possibly already pointed out by others) is that there seems to be no way to directly compare two enums.
As in, I cannot do something like if ST_NULL == GetSomeEnum().
I have to explicitly cast enum to some comparable type such as uint, so I have to do this instead if ST_NULL as uint == GetSomeEnum() as uint.

I find this not very intuitive, but I may be missing something here. Again, comparison between enum's don't seem to be documented.

Contributor

therealgymmy commented Nov 29, 2012

Thanks. :-)

Another thing I want to point out (possibly already pointed out by others) is that there seems to be no way to directly compare two enums.
As in, I cannot do something like if ST_NULL == GetSomeEnum().
I have to explicitly cast enum to some comparable type such as uint, so I have to do this instead if ST_NULL as uint == GetSomeEnum() as uint.

I find this not very intuitive, but I may be missing something here. Again, comparison between enum's don't seem to be documented.

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Nov 29, 2012

Contributor

At least right now, you have to write an Eq impl if you want equality on a particular enum type. For an example, see src/libcore/option.rs.

Once deriving is fully implemented, this will be easier.

Contributor

catamorphism commented Nov 29, 2012

At least right now, you have to write an Eq impl if you want equality on a particular enum type. For an example, see src/libcore/option.rs.

Once deriving is fully implemented, this will be easier.

@therealgymmy

This comment has been minimized.

Show comment
Hide comment
@therealgymmy

therealgymmy Nov 29, 2012

Contributor

Awesome!
I'm looking forward to the next release :-)

Contributor

therealgymmy commented Nov 29, 2012

Awesome!
I'm looking forward to the next release :-)

@therealgymmy

This comment has been minimized.

Show comment
Hide comment
@therealgymmy

therealgymmy Dec 2, 2012

Contributor

Another internal error related to using enum in an expression expecting constant.

The following code also causes the compiler to emit an internal error

for [0, ..(ST_WHITESPACE as uint + 1)].each |i| {
    // do stuff
}

This is the error message

error: internal compiler error: node_id_to_type:
no type for node `expr ST_WHITESPACE as uint (id=350)`
Contributor

therealgymmy commented Dec 2, 2012

Another internal error related to using enum in an expression expecting constant.

The following code also causes the compiler to emit an internal error

for [0, ..(ST_WHITESPACE as uint + 1)].each |i| {
    // do stuff
}

This is the error message

error: internal compiler error: node_id_to_type:
no type for node `expr ST_WHITESPACE as uint (id=350)`
@youknowone

This comment has been minimized.

Show comment
Hide comment
@youknowone

youknowone Mar 1, 2013

Contributor

Original issue is duplication of #3645

The minimal case of commented new one is:

fn main() { let a = [0, ..1 as uint]; }
Contributor

youknowone commented Mar 1, 2013

Original issue is duplication of #3645

The minimal case of commented new one is:

fn main() { let a = [0, ..1 as uint]; }
@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix Mar 25, 2013

Member

Not critical for 0.6; de-milestoning

Member

pnkfelix commented Mar 25, 2013

Not critical for 0.6; de-milestoning

@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix May 23, 2013

Member

Despite the claim in an earlier comment that this is a duplicate of #3645 (which was closed), it appears to me like the described minimal case readily duplicates in 0.6 (18fca3e 2013-05-22).

The easy workaround for that minimal case is to use 1u instead of 1 as uint; but the point here really is that there are contexts where one wants to use the cast-expression form for a constant (e.g. an identifier).

Member

pnkfelix commented May 23, 2013

Despite the claim in an earlier comment that this is a duplicate of #3645 (which was closed), it appears to me like the described minimal case readily duplicates in 0.6 (18fca3e 2013-05-22).

The easy workaround for that minimal case is to use 1u instead of 1 as uint; but the point here really is that there are contexts where one wants to use the cast-expression form for a constant (e.g. an identifier).

@jdm

This comment has been minimized.

Show comment
Hide comment
@jdm

jdm May 23, 2013

Contributor

One compelling case is that as is necessary for casting an enum to an integer value.

Contributor

jdm commented May 23, 2013

One compelling case is that as is necessary for casting an enum to an integer value.

@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix Jul 30, 2013

Member

Visiting for bug triage, email 2013-07-29

The enumVariant as uint case described by jdm is semi-addressed by ba9c3eb

But this is nonetheless still a bug.

Member

pnkfelix commented Jul 30, 2013

Visiting for bug triage, email 2013-07-29

The enumVariant as uint case described by jdm is semi-addressed by ba9c3eb

But this is nonetheless still a bug.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Oct 14, 2013

Member

The test case of

fn main() { let a = [0, ..1 as uint]; }

now passes, but I was unable to get the test case of

enum State { ST_NULL, ST_WHITESPACE }

fn SomeFunction () {
    ~[ST_NULL, ..(ST_WHITESPACE as uint)];
}

fn main() {}

so this may still be an open issue. The error I'm getting is:

foo.rs:4:17: 4:41 error: expected constant integer for repeat count but found variable
Member

alexcrichton commented Oct 14, 2013

The test case of

fn main() { let a = [0, ..1 as uint]; }

now passes, but I was unable to get the test case of

enum State { ST_NULL, ST_WHITESPACE }

fn SomeFunction () {
    ~[ST_NULL, ..(ST_WHITESPACE as uint)];
}

fn main() {}

so this may still be an open issue. The error I'm getting is:

foo.rs:4:17: 4:41 error: expected constant integer for repeat count but found variable
@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix Oct 14, 2013

Member

If one explicitly initializes the enum values for State, then the code compiles:

enum State { ST_NULL = 0, ST_WHITESPACE=1 }

fn SomeFunction () {
    ~[ST_NULL, ..(ST_WHITESPACE as uint)];
}

fn main() {}

I claim that this strikes the right balance, and thus this issue can probably be closed.

Member

pnkfelix commented Oct 14, 2013

If one explicitly initializes the enum values for State, then the code compiles:

enum State { ST_NULL = 0, ST_WHITESPACE=1 }

fn SomeFunction () {
    ~[ST_NULL, ..(ST_WHITESPACE as uint)];
}

fn main() {}

I claim that this strikes the right balance, and thus this issue can probably be closed.

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Oct 21, 2013

Contributor

This appears to be fixed -- adding needstest label. (I think the "error: expected constant integer for repeat count but found variable" should be a compile-fail test, and Felix's example can be the run-pass test.)

Contributor

catamorphism commented Oct 21, 2013

This appears to be fixed -- adding needstest label. (I think the "error: expected constant integer for repeat count but found variable" should be a compile-fail test, and Felix's example can be the run-pass test.)

@omasanori

This comment has been minimized.

Show comment
Hide comment
@omasanori

omasanori Jan 29, 2014

Contributor

I tried to add test cases for this issue and found that ST_NULL = 0 is not needed to pass the code. The code:

enum State { ST_NULL, ST_WHITESPACE=1 }
// ...

seems enough. Is it correct?

Contributor

omasanori commented Jan 29, 2014

I tried to add test cases for this issue and found that ST_NULL = 0 is not needed to pass the code. The code:

enum State { ST_NULL, ST_WHITESPACE=1 }
// ...

seems enough. Is it correct?

alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 1, 2014

Add test cases for #4063.
Signed-off-by: OGINO Masanori <masanori.ogino@gmail.com>

alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 1, 2014

Add test cases for #4063.
Signed-off-by: OGINO Masanori <masanori.ogino@gmail.com>

@bors bors closed this in 5c5d995 Feb 1, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment