Allow constant expressions in [Type * n].

[luqmana]

So this is a partial fix for #3469. Partial because it only works for simple constant expressions like 32/2 and 2+2 and not for any actual constants.

For example:

const FOO: uint = 2+2;
let v: [int * FOO];

results in:

error: expected constant expr for vector length: Non-constant path in constant expr

This seems to be because at the point of error (typeck::astconv) the def_map doesn't contain the constant and thus it can't lookup the actual expression (2+2 in this case).

So, feedback on what I have so far and suggestions for how to address the constant issue?

[sanxiyn]

It seems to me that the problem is resolve does not visit vector length expression. You need to edit visit.rs.

[pcwalton]

So you will need to be careful about situations like this:

const X: int = Y[1024];
const Y: [int * Z] = [0, ..Z];
const Z: int = 1024 * 2;

Here the right order to evaluate the constants—and determine their types—is Z, Y, X. So we will need to actually typecheck "just in time", just before evaluating constants. We also need to intertwine constant evaluation and typechecking. So I think we need to do the following in the collect module in typeck:

1. Build a dependency graph of constants. There is an edge between constants A and B if A contains a lexical reference to B. This can be done from the def_map.
2. Topologically sort the dependency graph.
3. For each constant C in the dependency graph, first astconv its type, then evaluate it.

[nikomatsakis]

Why not intertwine constant eval and type checking? If a constant appears in a type, we start to evaluate it. If that encounters a cyclic situation, report an error. We already do this for type expansion.

[nikomatsakis]

I meant not "type evaluation" but rather "type collection", I guess

[nikomatsakis]

There are still the unresolved questions of precisely what sort of constant expressions to allow. For now we could do a best effort thing where the type checker complains if it is asked to evaluate a constant that includes field projection / dereferencing or anything beyond basic arithmetic.

[pcwalton]

@nikomatsakis Sure, that's basically implicitly doing the topological sort.

I don't know if we need to overly restrict what constant expressions to allow. As long as it's not cyclic I don't see any issues…

[graydon]

We had a discussion about this but I'm not finding it. Anyone have a link? Untangling this needs to happen eventually before 1.0.

[sanxiyn]

Re: previous discussion. #2317 has long comments on constant expression design, from 10 months ago.

[graydon]

I was looking for the discussion in https://github.com/mozilla/rust/wiki/Meeting-weekly-2013-01-15 concerning [type * n]. It goes into some depth about the tensions we've been trying to resolve, but doesn't explicitly settle on a course of action.

[luqmana]

So this actually works now. It also fixes the [0, ..expr] syntax since before it'd fail for anything more complex than a single value. (i.e FOO but not FOO+1).

So now you can do stuff like:

const FOO: int = 2;
let v: [int * FOO * 3] = [0, ..FOO + 2 + 2];

r? @pcwalton

[nikomatsakis]

I think that this PR is a good starting point but does not address some of the more subtle issues, but maybe things work out simpler than I expect. Can you add the following two tests and tell me what happens? These tests are not expected to compile successfully but we want to be sure they report the right sort of error.

Test 1:

const X: uint = Y[0];
const Y: [uint * X] = [3, 2, 1];

Test 2:

struct Foo {f: uint, g: [uint * X]}
const X: uint = Z.f;
const Z: Foo = Foo { f: 3, g: [3, 2, 1] };

[nikomatsakis]

See my comment on #3469: #3469 (comment)

[nikomatsakis]

By the way, I don't want to let the perfect be the enemy of the good---I am not opposed to taking this pull request, it seems like a strict improvement on the current situation! I just don't consider the matter of gracefully handling constants in types fully resolved yet.

[nikomatsakis]

Oh, well, @luqmana one thing that is missing from this PR are some tests. Can you please add files into src/test/run-pass and src/test/compile-fail showing that this works in various situations? For example, you can add @pcwalton's example from above, as well as various other examples.

[luqmana]

@nikomatsakis ah no, your comments definitely make sense. The way constant expressions work now seems a bit delicate in terms of the order in which type checking and const checking happen.

The two tests above would currently fail:

const X: uint = Y[0];
const Y: [uint * X] = [3, 2, 1];
---
error: expected constant expr for vector length: Unsupported constant expr
const Y: [uint * X] = [3, 2, 1];
         ^~~~~~~~~~

struct Foo {f: uint, g: [uint * X]}
const X: uint = Z.f;
const Z: Foo = Foo { f: 3, g: [3, 2, 1] };
---
error: expected constant expr for vector length: Unsupported constant expr
struct Foo {f: uint, g: [uint * X]}
                        ^~~~~~~~~~

I think this should be a separate issue as this pull mostly just addresses using const expressions that evaluate to uint/int to declare fixed length vectors.

[erickt]

I have to say the type [int * FOO * 3] looks really weird to me. I understand what it says, but having * mean two different things right next to each other just feels wrong. This could ultimately lead to someone writing:

type foo = int;
const foo: foo = 3;
type bar = [foo * foo * foo];

Evil.

This feels like a good justification for an alternative syntax. Perhaps [int .. FOO * 3] to match the expression syntax. I'm not crazy about it but at least it'll be clear which portion is the type and which is the number.

[luqmana]

Definitely, but i'm not sure about reception to that kind of syntax change or at least in the scope of this pull request (i think that'd require an RFC).

[graydon]

Expression language doesn't fit in pattern language or type language. I'd prefer the type form only takes an ident.

[luqmana]

@graydon so [Type * FOO] where FOO is either a const or uint?

[brendanzab]

Still no review? This would be super nice to have in glfw-rs, ticking off one of my old issues (#3469).

[graydon]

Sorry, keep hesitating because of the fragility of the approach; the long term fix is to intertwine constant eval and type checking. Not sure what subset is most useful in the short term.

[luqmana]

I think, at least in the short term, this address the useful ability to use a const expr for declaring fixed length vectors. It also fixes the issue where the vector repeat syntax ICE's on expressions more complex than a single term.

[graydon]

Discussed at meeting today, agreed this is good enough for the short term, and we'll implement a longer-term fix intertwining constant evaluation and type checking later.