rust-lang / const-eval

home for proposals in and around compile-time function evaluation
Apache License 2.0
105 stars 17 forks source link

Towards dynamic const-qualify #27

Closed RalfJung closed 5 years ago

RalfJung commented 5 years ago

I finally came around to look at https://github.com/rust-rfcs/const-eval/issues/17 again, and updated the documents with what I learned on the way.

@oli-obk @eddyb please check if this all makes sense. :)

RalfJung commented 5 years ago

Also Cc @ecstatic-morse

RalfJung commented 5 years ago

@eddyb in this discussion

Promotion in consts and statics is performed by removing the StorageDead - that's what the bitset returned by mir_const_qualif is for - since that already has to work.

So I was using the term for the mechanism, and you were using for the effect. That would explain the confusion. Nuking StorageDead actually makes sense to me, and that's what I used to think happens in consts -- but then I talked with you and you taught me about the "enclosing scope" rule and since then I was sure it was not done by nuking StorageDead.

I think "promotion" makes more sense as the name for the mechanism of extracting part of the MIR out -- and that's what Promoted already means in the compiler. So I feel like we should come up with another name for the effect of something having a longer lifetime than expected. But we can also do it the other way around, as long as we are able to clearly distinguish the two.

Overall we are looking for three terms:

Finally I feel like we are making progress on this minefield of three different things that are all referred to by the same name.^^

Strawman proposals for the three names:

eddyb commented 5 years ago

Nuking StorageDead isn't enough for non-lifetime things (like [expr; n]), and it may be dubious even for the 'static effect, it's a microoptimization, we should measure how expensive the correct way is.

The reason I was abusing the word "promotion" is mostly historical: the lifting was "merely" the way it was implemented in MIR, but both "qualification" and "promotion" predate MIR, and they were referring to AST expressions: "qualification" was the analysis, and "promotion" was the effect of having 'static lifetime (as observed by borrowck and implemented in AST-based codegen).

The way it was done back then made even something as simple as 1 + 1 behave like *&1 + *&1, with some custom constant-folding for reading from a custom allocation (because otherwise the LLVM IR would've been loading every little literal from some global).

I should've changed the terminology when I implemented the MIR side, but I guess I was trying to make them as close as possible to avoid introducing any bugs (since the old borrowck would still use the information from the AST pass, until we switched to the MIR borrowck, and only recently are we getting close to getting rid of the old one).

For the MIR, I think "const checking" and "const promotion" are pretty okay terms. One thing your nomenclature seems to avoid is that the analysis used to nuke StorageDead (and which should actually be used to do the same extraction as in fns) is the same as the runtime promotion, just more relaxed.

The reason you can be more relaxed is the code must run at compile-time anyway, so there is no risk of a const fn changing behavior from runtime to compile-time (and breaking compilation). That's pretty much it, the rules around drops and frozen memory are the same.

oli-obk commented 5 years ago

The mechanism is called promotion because it is what does the action of promoting.

The unsuspecting-runtime-code-gets-const-evaluated thing should probably focus on the fact that const evaluating arbitrary runtime code will fail, so we need to be careful about what we guarantee. Maybe "guaranteed" is the right keyword?

I like "lifetime extension", because it makes it so much clearer that the type system process could just as well extend the lifetime on &read_from_network() and that would be :boom: out of obvious reasons.

RalfJung commented 5 years ago

Nuking StorageDead isn't enough for non-lifetime things (like [expr; n]), and it may be dubious even for the 'static effect, it's a microoptimization, we should measure how expensive the correct way is.

Fair. The focus in this PR anyway is to document current behavior and clarify terminology.

One thing your nomenclature seems to avoid is that the analysis used to nuke StorageDead (and which should actually be used to do the same extraction as in fns) is the same as the runtime promotion, just more relaxed.

Right, so those would be two different "modes" of the analysis, whatever we end up calling that one.

The mechanism is called promotion because it is what does the action of promoting.

That doesn't exactly explain which of the multiple possible meanings of this verb you mean. ;)

Like, do you consider nuking StorageDead as "how promotion looks like in consts", or as not being promotion?

I think my preferred proposal is to use the term "promotion" in the same sense as StaticKind::Promoted. So that would mean that "promotion" is "messing with MIR to extract some part of it into a separate static". Then we'd have to find a separate term for the analysis, at least right now where it is the case that the analysis does not use promotion for all the things it finds (but uses "nuking StorageDead" for some).

Not sure if a separate name for the analysis still makes sense though if we end up always using the "messing with MIR" mechanism for everything? Still, seems nice to be able to talk about them separately. It's just that someone would have to rename everything promotion-analysis-related in rustc...

ecstatic-morse commented 5 years ago

FWIW, I've been using the following nomenclature in my head while working on rust-lang/rust#64470 and friends.

Using that terminology, the three jobs of the current qualifcation pass would be:

The last step, promotion, has two different subroutines depending on whether the current item is a const/static or fn/const fn.

I suppose I should be referring to the last two steps together as "lifetime extension", although I mostly just say "promotion" when writing about them. Also, I think "lifetime extension" might already have a specific meaning in the context of the borrow checker?

RalfJung commented 5 years ago

@ecstatic-morse thanks a lot, that is very helpful!

by passing it to a function with #[rustc_args_promotable]

Does that attribute have the same effect as being inside the body of a const? Promotability (not just promotion) also works differently there than in the body of a normal fn.

promotable: Whether a temporary is eligible for promotion

What exactly is a "temporary" here? Is it just any MIR-level Local?

I suppose I should be referring to the last two steps together as "lifetime extension", although I mostly just say "promotion" when writing about them. Also, I think "lifetime extension" might already have a specific meaning in the context of the borrow checker?

Call them "promotion" unfortunately overlaps with the third step which is also called "promotion". But what I meant by "lifetime extension" was just what you call "promotability (analysis)".

Maybe using the same name here for both is okay, as long as everyone is aware of the more refined terminology and we switch to it when it seems like confusion is arising.

ecstatic-morse commented 5 years ago

Does that attribute have the same effect as being inside the body of a const? Promotability (not just promotion) also works differently there than in the body of a normal fn.

Ah, that's a good catch. I think this is the place where they diverge? Basically, promotion of #[rustc_args_required_const] arguments is mandatory in fns and const fns--an error is thrown if the argument is not explicitly promotable, but unnecessary in consts and statics--everything inside a const body is also const and there's no need to extend the lifetime of the argument by removing StorageDead.

Said another way, when it comes to #[rustc_args_required_const] arguments (and probably array initializers), the purpose of promotion is not actually lifetime extension since there's no reference involved.

What exactly is a "temporary" here? Is it just any MIR-level Local?

A temporary is a MIR Local that does not correspond to a named variable in the source. We don't promote named variables because it would surprise the user when their drop impl is not called.

Call them "promotion" unfortunately overlaps with the third step which is also called "promotion". But what I meant by "lifetime extension" was just what you call "promotability (analysis)".

I like "promotability analysis" (I didn't have a good noun for the second step). The hierarchy currently looks like this in my head:

Centril commented 5 years ago

Basically, promotion of #[rustc_args_required_const] arguments is mandatory in fns and const fns--an error is thrown if the argument is not explicitly promotable, but unnecessary in consts and statics--everything inside a const body is also const and there's no need to extend the lifetime of the argument by removing StorageDead.

Question: what is the difference between #[rustc_args_required_const] and const A: B generic parameters in terms of what code can be passed as an argument?

oli-obk commented 5 years ago

no difference for the caller except in syntax.

The function body can't use the argument as const, so you can't forward it internally, so const generics are more powerful there.

RalfJung commented 5 years ago

Said another way, when it comes to #[rustc_args_required_const] arguments (and probably array initializers), the purpose of promotion is not actually lifetime extension since there's no reference involved.

I see. That sounds quite hacky. But on the other hand we probably do want the same checks for both cases (as in both cases it is not locally syntactically apparent that things are evaluated at const-time). So maybe this is a hint that the analysis (shared by promotion-in-runtime-fn and rustc_args_required_const) should be called something that does not involve the term "promotion", as it is not exclusive to promotion.

Maybe it makes more sense then to to speak of "explicit-const validation" and "implicit-const validation" or "explicit-const checking" and "implicit-const checking" (I think I prefer the latter) to refer to the analysis that checks if code is amenable to const-eval. And then both rustc_args_required_const and promotion-in-runtime-fn would be using the results "implicit-const checking". (The brackets here are like "(implicit-const) checking" as the check is about testing whether this code can be implicitly treated as being const-code.)

RalfJung commented 5 years ago

It seems like we are mostly bikeshedding terminology here at this point, which IMO does not have to block this PR. So, maybe we can move that to an issue? Is there anything else blocking the PR?

ecstatic-morse commented 5 years ago

I'm still confused about the first part of this comment. Other than that I think it's ready.

oli-obk commented 5 years ago

r=me with @ecstatic-morse's confusion resolved

ecstatic-morse commented 5 years ago

My confusion has been resolved (at least far as this PR is concerned).

RalfJung commented 5 years ago

Great, thanks. :)

eddyb commented 5 years ago

Note to self: review this (eventually).