Note difference in CTFE timing between associated and free constants #1120

XrXr · 2021-12-24T23:16:12Z

Since const panics are now stabilized, I figure it would be good to
document some of its gotchas. I couldn't really find documents for
the specifics I ran into. I am no const eval expert, and what I
wrote is basically a paraphrase of Ralf's comments¹²,
but I hope to get the ball rolling for adding some docs.

I deliberately chose to not mention the guarantee about syntactically
referenced constants in functions that require code generation as it
seems hard to explain completely without some ambiguity. It also seems
to me that the rules are not completely stable³⁴ yet.

ehuss · 2021-12-31T19:10:41Z

@oli-obk Would you be willing to review this?

oli-obk · 2021-12-31T19:16:29Z

r? @oli-obk

oli-obk · 2022-01-12T15:25:47Z

I'm not sure these docs belong here. This file is specifically about constant evaluation itself. You could add some documentation to associated constants clarifying that they are only evaluated at their use sites.

About the panic section, as it is written right now, it seems like no new information to me, can you elaborate on why you think it would be useful here to mention that panicking during CTFE will cause an error?

XrXr · 2022-01-12T22:33:01Z

You could add some documentation to associated constants clarifying that they are only evaluated at their use sites.

Sounds good, I will move it.

can you elaborate on why you think it would be useful here to mention that panicking during CTFE will cause an error?

I added the panic section because the page had no explicit mention of panicking during CTFE, so I thought I would talk about it to show that it is a possible outcome of CTFE. Maybe a whole section for it is too much. I also put the section for proximity to the eval timing section since timing (whether CTFE happens at all) is more relevant if it could abort compilation.

I found this difference in timing surprising and other may as well.

XrXr · 2022-01-20T03:21:12Z

@oli-obk I've reduced the scope of the change and relocated the additions as suggested. 77387cb

oli-obk · 2022-01-20T07:27:15Z

I don't have merge powers. @ehuss this lgtm

ehuss · 2022-01-24T14:48:32Z

src/items/associated-items.md

@@ -335,6 +338,24 @@ fn main() {
 }
 ```

+[Constant evaluation] timing:


When I immediately read this, I didn't know what it meant by "timing".

Instead of placing this down in the "examples" section, can this code block be moved up to the paragraph that you added above? Generally in the reference, when there is an example demonstrating some concept, we try to write that example immediately after the paragraph that introduces that concept.

Thanks, that makes sense. Moved in be49428

rust-lang#1120 (comment)

ehuss

Thanks!

RalfJung · 2022-01-25T16:57:38Z

src/items/associated-items.md

@@ -293,6 +293,25 @@ type that the definition has to implement.
 An *associated constant definition* defines a constant associated with a
 type. It is written the same as a [constant item].

+Unlike [free] constants, associated constant definitions undergo
+[constant evaluation] only when referenced.


This seems odd, to make a guarantee by stating its absence on associated consts. Do we say explicitly anywhere that free constants are always evaluated, even when they are not used?

Do we say explicitly anywhere that free constants are always evaluated, even when they are not used?

I went through Constant Evaluation and Constant items and could not find it explicitly stated. Some possible ways forward:

reword this line to say "Associated constant definitions undergo constant evaluation only when referenced.", removing the reference to free constants

mention that unused free constants always undergo CTFE in Constant items and reword this sentence to link to it.

Do you have any preferences or recommendations?

2 sounds like something we will eventually want to do, but will probably require T-lang FCP to ensure that we are willing to commit to this as a guarantee.

@RalfJung

@RalfJung pointed out in a [comment] that the previous phrasing of the sentence can read like it is giving guarantees about free constant definitions always undergoing CTFE, even when unused. That seems to be how the compiler behaves right now, but it's unclear whether it's intentional. Be more precise and don't talk about free constants at all. [comment]: rust-lang#1120 (comment)

Update books ## nomicon 4 commits in 66d097d3d80e8f88c288c6879c7c2b909ecf8ad4..9493715a6280a1f74be759c7e1ef9999b5d13e6f 2022-01-05 05:45:21 +0900 to 2022-01-27 19:00:32 -0800 - send-and-sync: it's -> its (rust-lang/nomicon#332) - Clarify the HRTB chapter (rust-lang/nomicon#330) - Clarify repr(transparent) in other-reprs (rust-lang/nomicon#329) - Make C code more recognizably C (rust-lang/nomicon#331) ## reference 10 commits in 4dee6eb63d728ffb9e7a2ed443e9ada9275c69d2..411c2f0d5cebf48453ae2d136ad0c5e611d39aec 2022-01-18 09:26:33 -0800 to 2022-01-30 12:46:37 -0800 - paths.md: update comments of `Canoical paths` section (rust-lang/reference#1146) - Add undocumented outer attributes above StructExpr fields (rust-lang/reference#1150) - (rust-lang/reference#1148) - Fix micro typo in async/unsafe function docs (rust-lang/reference#1145) - Note difference in CTFE timing between associated and free constants (rust-lang/reference#1120) - Update the Preludes chapter for the 2021 edition changes to the standard library prelude (rust-lang/reference#1136) - Link to associated constants section rather than glossary (rust-lang/reference#1141) - functions.md: replace `argument` with `parameter` (rust-lang/reference#1142) - Improve rendering (rust-lang/reference#1143) - (minor) link references and replace wording by syntax definition (rust-lang/reference#1139) ## book 24 commits in f17df27fc14696912c48b8b7a7a8fa49e648088d..98904efaa4fc968db8ff59cf2744d9f7ed158166 2022-01-18 17:46:28 -0500 to 2022-01-29 21:22:31 -0500 - Snapshot of chapter 17 for nostarch - Remove the section on object safety. - Don't put a hyphen in 'object safe'. Fixes rust-lang/book#2960. - Clarify that add_text on Post will work in any state. Fixes rust-lang/book#2159. - Fix incorrect descriptions of what the code is doing. Fixes rust-lang/book#2745. - Fix link style and inclusion in print - Snapshot of ch16 for nostarch - Cut discussion of threading models Rust *doesn't* support. - Update a quote of compiler output - Move transfers between threads, not shares. Fixes rust-lang/book#2843. - Ch20-02 Remove reference to a long-gone "trick" - Clarify translations a bit - Added a mention to the translations appendix - Fix listing number from `8-5` to `9-5` in `ch09-02` - Moving example into blockquote means it can't be extracted to a listing project - Move a link to the end with all the other links - Propagate edits back to ch 9 - Responding to edits in chapter 9 - Update to 1.58 - Snapshot of chapter 15 for nostarch - Change 'only difference' to 'main difference'. Fixes rust-lang/book#1581. - Add a back reference to tuple struct syntax. Fixes rust-lang/book#1916 - Add a link to a section reference - Remove an outdated example that says it won't compile but it does ## rustc-dev-guide 2 commits in 78dd6a4..8763adb 2022-01-18 14:44:26 -0300 to 2022-01-26 14:01:40 -0800 - git.md: Expanded a note to try to stress what you need to do if you're playing - Clarify that r? works in comments. ## embedded-book 1 commits in 8c395bdd8073deb20ca67e1ed4b14a3a7e315a37..d5fc1bce3f8eb398f9c25f1b15e0257d7537cd41 2021-11-14 11:38:31 +0000 to 2022-01-24 07:13:31 +0000 - Add link to Japanese translation (rust-embedded/book#311)

ehuss assigned oli-obk Dec 31, 2021

Note difference in CTFE timing between associated and free constants

77387cb

I found this difference in timing surprising and other may as well.

XrXr force-pushed the const-eval-panics branch from bd9a46e to 77387cb Compare January 20, 2022 03:18

XrXr changed the title ~~Add sections about panics during constant evaluation~~ Note difference in CTFE timing between associated and free constants Jan 20, 2022

oli-obk approved these changes Jan 20, 2022

View reviewed changes

ehuss reviewed Jan 24, 2022

View reviewed changes

Move example closer to explaination

be49428

rust-lang#1120 (comment)

ehuss approved these changes Jan 24, 2022

View reviewed changes

ehuss merged commit e67f679 into rust-lang:master Jan 24, 2022

XrXr deleted the const-eval-panics branch January 24, 2022 17:10

XrXr mentioned this pull request Jan 25, 2022

Const in trait impl block not evaluated unless used, skipping panics. rust-lang/rust#91877

Closed

RalfJung reviewed Jan 25, 2022

View reviewed changes

XrXr mentioned this pull request Jan 27, 2022

Improve associated constant item CTFE timing section #1147

Merged

ehuss mentioned this pull request Feb 1, 2022

Update books rust-lang/rust#93532

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Note difference in CTFE timing between associated and free constants #1120

Note difference in CTFE timing between associated and free constants #1120

XrXr commented Dec 24, 2021

ehuss commented Dec 31, 2021

oli-obk commented Dec 31, 2021

oli-obk commented Jan 12, 2022

XrXr commented Jan 12, 2022

XrXr commented Jan 20, 2022

oli-obk commented Jan 20, 2022

ehuss Jan 24, 2022

XrXr Jan 24, 2022 •

edited

Loading

ehuss left a comment

RalfJung Jan 25, 2022

XrXr Jan 25, 2022

RalfJung Jan 26, 2022

Note difference in CTFE timing between associated and free constants #1120

Note difference in CTFE timing between associated and free constants #1120

Conversation

XrXr commented Dec 24, 2021

Footnotes

ehuss commented Dec 31, 2021

oli-obk commented Dec 31, 2021

oli-obk commented Jan 12, 2022

XrXr commented Jan 12, 2022

XrXr commented Jan 20, 2022

oli-obk commented Jan 20, 2022

ehuss Jan 24, 2022

Choose a reason for hiding this comment

XrXr Jan 24, 2022 • edited Loading

Choose a reason for hiding this comment

ehuss left a comment

Choose a reason for hiding this comment

RalfJung Jan 25, 2022

Choose a reason for hiding this comment

XrXr Jan 25, 2022

Choose a reason for hiding this comment

RalfJung Jan 26, 2022

Choose a reason for hiding this comment

XrXr Jan 24, 2022 •

edited

Loading