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

Extend [expr; N] syntax to allow any expr that can appear in a constant expression #1169

Closed
bstrie opened this issue Jun 23, 2015 · 6 comments
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.

Comments

@bstrie
Copy link
Contributor

bstrie commented Jun 23, 2015

Reminder to either myself or @eddyb to actually propose this.

@eddyb
Copy link
Member

eddyb commented Jun 23, 2015

Implementation: move the typeof(expr): Copy check to check_const and only perform it if expr is not a constant rvalue.

@bstrie
Copy link
Contributor Author

bstrie commented Jun 23, 2015

Two things occur to me:

  1. We'd want to make sure that this policy meshes with whatever future direction that const expressions will want to take. I presume that the reason that we don't want want [Box::new(Foo); 4] to work is to avoid implicit clones and also to avoid implicit side effects that can occur from within clone constructors. I don't foresee that const expressions will ever pose a problem in these ways, but I also don't have any thoughts on where const expressions will be going. Also, I don't know if the two reasons given for constraining the expr here are exhaustive.
  2. For symmetry we'd want to apply this to vec![expr; N] as well.

@Kimundi
Copy link
Member

Kimundi commented Jun 23, 2015

@bstrie: vec![expr; N] is already as general as can be in that regard. (Or do you mean that it should support non-clonable expressions?)

@bstrie
Copy link
Contributor Author

bstrie commented Jun 24, 2015

@Kimundi At the time of that comment I didn't realize that vec![expr; N] worked for Clone exprs. And after talking with @eddyb about his idea for the direction of constexprs I'm no longer concerned about either of those points.

@nrc nrc added the T-lang Relevant to the language team, which will review and decide on the RFC. label Aug 29, 2016
@eddyb
Copy link
Member

eddyb commented Sep 6, 2016

It has also occurred to me lately (cc @nikomatsakis @pnkfelix) that a MIR-based borrowck
would be able to do this correctly: [x; N] would count as N uses of x, so [x; 1] always works if you can move x and [x; N] (with N > 1) works if you can copy x, while [CONST; N] has no restrictions.

However, [None; N] has an unfaithful MIR representation of tmp = None; [tmp; N] which would behave as a variable for all intents and purposes.
There's also the fact that [x; N] requires a loop, so perhaps it should be a custom terminator, but I don't think that would help here.
We might need a new Lvalue, i.e. dst[*] = None. This actually composes well with other things working on lvalues, i.e. SetDiscriminant(dst[*], None), but you can't generally split an ADT into multiple statements on dst[*] because it could result in multiple passes, each pass filing in another field.

If it were a fully explicit loop, then it would require a way to express that all the elements have been initialized, but the number of uses of the element value would be encoded in the CFG.
OTOH, a nested constant expression, such as Some(None), would have to be inside the loop, but that's not the same as the general case (which copies a temporary initialized before entering the loop).

So at the end of the day, perhaps special-casing [x; N] for known-to-be-constant x is best.
The checker now lives in librustc_mir/transform/qualify_consts.rs, for the record.

@Centril
Copy link
Contributor

Centril commented Feb 23, 2018

Triage: Closing as there exists an RFC for this now.

@Centril Centril closed this as completed Feb 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

5 participants