Towards dynamic const-qualify #27

RalfJung · 2019-08-25T18:19:26Z

I finally came around to look at #17 again, and updated the documents with what I learned on the way.

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

RalfJung · 2019-08-26T07:02:25Z

const.md

ecstatic-morse · 2019-08-26T16:51:41Z

promotion.md

+In `EMPTY_BYTES`, the reference obtains the lifetime of the "enclosing scope",
+similar to how `let x = &mut x;` creates a reference whose lifetime lasts for
+the enclosing scope. This is decided during MIR building already, and does not
+involve promotion.


This "enclosing scope" rule doesn't make sense to me. I don't know how it should extend to arbitrary cases. Is it the braces that cause the problem or the assignment to x? It also might be good to mention #[rustc_promotable] here, since without it calls to const fns can't get promoted.

I don't know how it should extend to arbitrary cases. Is it the braces that cause the problem or the assignment to x?

I don't know. I am paraphrasing @eddyb here.
@eddyb are these rules explained anywhere?

It also might be good to mention #[rustc_promotable] here, since without it calls to const fns can't get promoted.

Good point, will do.

Hm I guess I should edit this in the light of our unresolved terminology.

Is the entire "Promotion is not involved" incorrect, or is there something there that makes sense? I was trying to explain why const EMPTY_BYTES: &Vec<u8> = &Vec::new(); works even though this stuff does not get promoted. @ecstatic-morse since my explanation confused you, could you try to explain this in your own words?

See my latest comment for what is actually confusing me here. The "enclosing scope" rule makes sense intuitively, but the details are still not clear to me. That's my problem though, not this PR's.

I have done some edits. @ecstatic-morse does this resolve your confusion?

The enclosing scope rule is the same one that makes these two different:

{ let r = &foo(); // compiles bar(r); }

{ let r = id(&foo()); // doesn't compile bar(r); }

Except instead of let it's const/static and instead of "enclosing block" it's 'static.

I don't remember the exact name of these semantics, maybe "rvalue/temporary lifetime/scope"? @nikomatsakis would be more helpful here, sorry.

I call these "temporary lifetimes" -- I've been meaning to writeup some text about them for the rust reference.

const.md

oli-obk · 2019-08-27T08:22:05Z

promotion.md

+};
+```
+
+However, since this is in explicit const context, we could be less strict about


We are less strict. You can promote any const fn call inside a constant

Ah, good to know! Are there any other differences? We could in principle be even more aggressive.

iirc we are running just the regular const checks on promoteds inside constants and const fns. So there's no third system, it's exactly as if you wrote an explicit constant

Isnt promotion also about identifying which parts of the MIR to promote, and then actually patch the MIR to replace uses of the data by a promoted? Somehow this still needs to do something extra inside constants I think, or will it just trigger at every &?

So implementation wise the difference is IsPromotable vs IsImplicitlyPromotable, where iirc IsImplicitlyPromotable only makes a difference in non-const fns

So IsPromotable is not about the kind of promotion I am talking about here? :(
What is it about then?

I asked @eddyb about this terminology but must have misunderstood what he said.

So IsPromotable is not about the kind of promotion I am talking about here? :(

I'm confused.

So IsPromotable and IsImplicitlyPromotable are required in normal functions, while const fn and constants only check IsPromotable

I guess I am confused by the "implicit" part. The promotion to a separate static is also implicit inside a const body.

If the thing inside a normal fn is "implicit promotion", I would expect the thing inside a const body to be "explicit promotion", but that does not make sense, does it?

yea... it's not the best naming scheme... Maybe we should call the thing in non-const fns "restricted promotion" and the other cases just "promotion" or "unrestricted promotion"

That doesn't convey why it is restricted, though. "implicit" did that nicely, but was not precise enough.

"promotion of run-time code" is verbose but informative. "run-time promotion" is somewhat misleading... "run-time-code promotion"?

RalfJung · 2019-08-29T08:26:34Z

@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:

The mechanism that extracts parts of a MIR and moves it out (the kind of thing promoted_mir gives access to, from what I understand).
The specific case of using that mechanism on unsuspecting run-time code, which has the effect of moving it to compile-time, leading to a very restrictive analysis (this is what promoted.md in this repo talks about).
The analysis that determines which things get 'static lifetime even though they usually wouldn't. The result of this analysis is used to nuke StorageDead in const (and static?), and to extract part of a MIR and cut it out elsewhere.

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:

Mechanism: promotion; unsuspecting run-time case: run-time-code promotion; analysis: lifetime extension
Mechanism: MIR extraction; unsuspecting run-time case: run-time-code promotion; analysis: promotion
Mechanism: MIR extraction; unsuspecting run-time case: restricted promotion; analysis: promotion

eddyb · 2019-09-16T05:35:34Z

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 · 2019-09-16T11:05:32Z

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 💥 out of obvious reasons.

RalfJung · 2019-09-16T19:45:03Z

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 · 2019-09-16T20:52:02Z

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

lifetime extension: The ultimate goal of RFC #1414; make a certain class of temporary values 'static and prevent them from being dropped.
promotion: the act of mutating a mir::Body to extend the lifetime of a temporary. This is narrower than the way this term was being used in the discussion around RFC #1414, since it is merely the mechanism by which lifetime extension is achieved.
fn promotion: The specific method of promotion used for fn and const fns. This is the typical method of promotion.
StorageDead suppression: A more efficient method of promotion, used in consts and statics but not fns, that requires fewer modifications to the MIR.
promotable: Whether a temporary is eligible for promotion, and thus to have its lifetime extended. Among other things, this requires that the temporary be derived only from const-safe operations.
implicitly promotable: Whether a temporary is eligible for promotion if the user did not explicitly ask for its lifetime to be extended by passing it to a function with #[rustc_args_required_const].
explicitly promotable: Whether a temporary is eligible for promotion if the user explicitly asked for its lifetime to be extended by passing it to a function with #[rustc_args_required_const]. This is all implicitly promotable temporaries as well as the return value of const fn calls (which are not implicitly promotable).

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

validation (or checking) that all operations in an item are allowed in that context.
promotable temp identification.
promotion.

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 · 2019-09-17T07:14:09Z

@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 · 2019-09-17T16:23:29Z

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:

const validation
lifetime extension (if we can agree on this term)
- promotability analysis
- promotion

promotion.md

Centril · 2019-09-18T10:31:49Z

static.md

+* The compiler checks that statics are `Sync`, justifying sharing their address across threads.
+* [Constants](const.md) and [promoteds](promotion.md) are not allowed to read from statics,
+  so their final value does not have have to be [const-valid](const_safety.md#const-safety-check-on-values) in any meaningful way.
+  As of 2019-08, we do check them for validity anyway, to be conservative; and indeed constants could be allowed to read from frozen statics.


I think this would be nice personally; less times to invent an ephemeral constant just to please the check and you also get to use exported statics from other crates.

Any future compat hazards with some new feature?

Discovered a problem... If we want to have a scheme like rust-lang/rfcs#2492 but centered around statics instead, allowing statics in const contexts will give us all the problems re. separate compilation.

@Centril sorry, I do not follow... why is it harder for separate compilation to support const referring to static than const referring to const (and the latter we already support)?

In this case it would be a static for which the value isn't known yet; that is something like pub extern static Foo: u8; <-- no value. That would be similar to rust-lang/rfcs#2492 in that it is matched up with something in the crate graph. If you can then refer to Foo in types then that will create all the same problems as with extern existential type.

As long as whatever instantiates the static checks them for validity, we are sill good. The important part is the invariant that all statics have been checked. The order doesn't matter.

Centril · 2019-09-18T12:05:18Z

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 · 2019-09-18T12:42:16Z

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 · 2019-10-09T11:53:42Z

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 · 2019-10-10T20:03:14Z

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 · 2019-10-10T20:04:51Z

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

ecstatic-morse · 2019-10-10T20:52:54Z

promotion.md

+However, since this is in explicit const context, we are less strict about
+promotion in this situation: all function calls are promoted, not just
+`#[rustc_promotable]` functions.


@RalfJung I think my confusion really occurs because of this paragraph. It seems to apply that both consts should compile in the following example, which is not true.

Ah yes. That's because of the Drop check, which still applies.

~~It also fails on types which are not Drop. I think @oli-obk's comment above was incorrect?~~ Missing a const. Ignore me plz.

That example works if you make new a const fn.

oli-obk · 2019-10-11T08:22:45Z

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

ecstatic-morse · 2019-10-11T21:43:08Z

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

RalfJung · 2019-10-11T21:47:29Z

Great, thanks. :)

eddyb · 2019-10-12T17:43:49Z

Note to self: review this (eventually).

RalfJung added 3 commits August 25, 2019 20:12

note on consts

a18aafc

notes on dynamic checks

a9c7047

link to the must-not-err RFC

50a1829

ecstatic-morse reviewed Aug 26, 2019

View reviewed changes

const.md Show resolved Hide resolved

ecstatic-morse reviewed Aug 26, 2019

View reviewed changes

RalfJung added 3 commits August 27, 2019 09:28

clarify

cbd7e7e

link const -> const_safety

53d0200

mention rustc_promotable attribute

7f8c81c

oli-obk reviewed Aug 27, 2019

View reviewed changes

const.md Outdated Show resolved Hide resolved

oli-obk reviewed Aug 27, 2019

View reviewed changes

RalfJung added 2 commits August 27, 2019 18:02

we are less strict in const bodies

87c3c87

update status of interning-time const check

95acdf2

RalfJung mentioned this pull request Aug 29, 2019

Make sure interned constants are immutable rust-lang/rust#63955

Merged

RalfJung mentioned this pull request Aug 30, 2019

Const validation rejects references that might be dangling (but could actually be valid at run-time) rust-lang/rust#63197

Open

RalfJung mentioned this pull request Sep 16, 2019

Deduplicate some code between miri and const prop rust-lang/rust#64419

Merged

Centril reviewed Sep 18, 2019

View reviewed changes

const_vec_new has been stabilized

a29f147

ecstatic-morse reviewed Oct 10, 2019

View reviewed changes

ecstatic-morse mentioned this pull request Oct 11, 2019

Define promotion contexts and promotability #28

Merged

clarify promotion-in-consts

1d2b7b1

RalfJung merged commit 4c15a11 into rust-lang:master Oct 11, 2019

RalfJung deleted the const-qualif branch October 11, 2019 21:47

Towards dynamic const-qualify #27

Towards dynamic const-qualify #27

Conversation

RalfJung commented Aug 25, 2019

RalfJung commented Aug 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecstatic-morse Oct 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung Aug 28, 2019 • edited Loading

Choose a reason for hiding this comment

RalfJung commented Aug 29, 2019 • edited Loading

eddyb commented Sep 16, 2019

oli-obk commented Sep 16, 2019

RalfJung commented Sep 16, 2019

ecstatic-morse commented Sep 16, 2019 • edited Loading

RalfJung commented Sep 17, 2019

ecstatic-morse commented Sep 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Centril commented Sep 18, 2019

oli-obk commented Sep 18, 2019

RalfJung commented Oct 9, 2019

RalfJung commented Oct 10, 2019

ecstatic-morse commented Oct 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ecstatic-morse Oct 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oli-obk commented Oct 11, 2019

ecstatic-morse commented Oct 11, 2019

RalfJung commented Oct 11, 2019

eddyb commented Oct 12, 2019

ecstatic-morse Oct 10, 2019 •

edited

Loading

RalfJung Aug 28, 2019 •

edited

Loading

RalfJung commented Aug 29, 2019 •

edited

Loading

ecstatic-morse commented Sep 16, 2019 •

edited

Loading

ecstatic-morse commented Sep 17, 2019 •

edited

Loading

ecstatic-morse Oct 11, 2019 •

edited

Loading