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

mention the extra const UB #1273

Merged
merged 1 commit into from
Jul 20, 2023
Merged

mention the extra const UB #1273

merged 1 commit into from
Jul 20, 2023

Conversation

RalfJung
Copy link
Member

Cc @rust-lang/wg-const-eval @rust-lang/wg-unsafe-code-guidelines -- I think that is the only extra const UB but let me know if I missed anything. :)

Havvy
Havvy previously requested changes Sep 26, 2022
Copy link
Contributor

@Havvy Havvy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an editorial review. I have not verified the technical content.

src/behavior-considered-undefined.md Outdated Show resolved Hide resolved
src/behavior-considered-undefined.md Outdated Show resolved Hide resolved
src/behavior-considered-undefined.md Outdated Show resolved Hide resolved
@RalfJung
Copy link
Member Author

@Havvy @ehuss what is needed to move this PR forwards?

@ehuss
Copy link
Contributor

ehuss commented Jul 18, 2023

Sorry for the delay. This looks good to me, but I don't actually know the specifics of what is UB, or how we should decide that. I trust that you are correct, but it might help to get one other person who is familiar with it to approve it.

Perhaps @oli-obk or someone on wg-unsafe-code-guidelines, can you find someone to take a look?

@RalfJung
Copy link
Member Author

@rust-lang/wg-const-eval @rust-lang/opsem please have a look at this PR. :)

@digama0
Copy link

digama0 commented Jul 18, 2023

Is there a reason this needs to be UB and not simply an error? A more advanced implementation of const-eval could potentially handle this in limited situations, and in all the cases where rustc hits this issue, it reliably rejects the pattern.

@CAD97
Copy link

CAD97 commented Jul 18, 2023

I believe this PR is an accurate description of the current state of affairs.

The one thing to pay attention to is that this implicitly confirms that ptr-int transmutes are permitted outside of const contexts (by stating it's UB specifically in const context). I believe we've decided this is both fine and desired (semantics: strip provenance when loading as integer (ptr::addr)), loading a pointer without provenance loads an integer then as casts to pointer (ptr::from_exposed_addr)), but it's probably still a good idea to have an FCP for that.

Is there a reason this needs to be UB and not simply an error?

A not insignificant reason is that we don't really have a way to say / precedent for an operation which is an error at const time but just permitted at runtime. The closest would I guess be panics, but those errors would be better understood as being an unwind, and were catch_unwind to be const, it should probably catch const panics as well.

There's also the fact that it decidedly was UB previously (exposing nondeterminism into const) before it was turned into an error.

The original unsafe in const RFC covers more motivation on why we chose to treat const UB the way we do. (Not the least being continuing confusion between lib and lang UB.) Rustc goes to decent lengths to diagnose unambiguous lang UB in const, and we should expect it and any alternative frontends to continue to do so, but from the opsem perspective const evaluation errors are basically UB by definition (they're raised by operations for which the evaluator defines no semantics).

(IIRC: const UB is permitted to fail compilation and/or compromise the output artifact arbitrarily, but must not result in the compiler doing anything it couldn't otherwise do. Rustc provides an additional QOI guarantee that when no error is produced, compiler behavior is further bound to that reachable by selecting arbitrary byte-valid results for the const context (e.g. won't cause a linked but never mentioned proc macro to be run).)

@digama0
Copy link

digama0 commented Jul 18, 2023

Right, I am aware that our const UB semantics are both much more constrained than regular UB, but also quite nondeterministic, which is why I think if at all possible we should opt for something more well defined than that.

A not insignificant reason is that we don't really have a way to say / precedent for an operation which is an error at const time but just permitted at runtime.

You could say likewise that we don't have a precedent for an operation which is UB at const time and permitted at runtime. If the RFC rust-lang/rfcs#3352 regarding const time / runtime divergence goes through (which I expect to happen), then having an operation that is an error at const time and works at runtime seems quite reasonable and tame to work with, while an operation that is UB at const time (and is allowed to corrupt any constant it is used in) couldn't really be part of a library at all.

@RalfJung
Copy link
Member Author

RalfJung commented Jul 19, 2023 via email

@RalfJung
Copy link
Member Author

Note that this UB went through t-lang FCP here, 2 years ago. We just forgot to add it to the reference. So I think that should be sufficient justification to merge as-is.

@digama0 if you want to change this to a const-eval error and have a good idea for how to realize this, I'm not opposed, but I don't think that should block updating the reference to a consensus that was formed 2 years ago.

@digama0
Copy link

digama0 commented Jul 19, 2023

I'll retract my concern.

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I did not realize this was part of rust-lang/rust#85769, I missed the UB part in #1081. Thanks all for taking a look, if there any other concerns or feedback, feel free to follow up with comments or PRs.

@ehuss ehuss added this pull request to the merge queue Jul 20, 2023
Merged via the queue into rust-lang:master with commit e94fb3d Jul 20, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 1, 2023
Update books

## rust-lang/reference

7 commits in 1ea0178266b3f3f613b0fabdaf16a83961c99cdb..9cd5c5a6ccbd4c07c65ab5c69a53286280308c95
2023-07-29 22:29:51 UTC to 2023-07-16 20:12:46 UTC

- Fix merge queue building twice. (rust-lang/reference#1383)
- Clarify UB around immutability & mutation (rust-lang/reference#1385)
- mention the extra const UB (rust-lang/reference#1273)
- Operator expressions: make the note about division by zero clearer. (rust-lang/reference#1384)
- Make unsafe keyword docs less confusing (rust-lang/reference#1379)
- Say that division by zero for primitive types panics (rust-lang/reference#1382)
- Add CI trigger for merge queues. (rust-lang/reference#1381)

## rust-lang/rust-by-example

3 commits in 8a87926a985ce32ca1fad1be4008ee161a0b91eb..07e0df2f006e59d171c6bf3cafa9d61dbeb520d8
2023-07-24 11:37:55 UTC to 2023-07-24 11:35:36 UTC

- Added attribute unused_labels - fixed warning. (rust-lang/rust-by-example#1729)
- more explanation about panic (rust-lang/rust-by-example#1728)
- chore: add the portuguese version of this project to `readme.md` (rust-lang/rust-by-example#1727)

## rust-lang/rustc-dev-guide

26 commits in b5a12d95e32ae53791cc6ab44417774667ed2ac6..24eebb6df96d037aad285e4f7793bd0393a66878
2023-07-30 11:23:23 UTC to 2023-07-11 06:02:34 UTC

- fix(name-resolution): remove unnecessary closing paranthesis (rust-lang/rustc-dev-guide#1760)
- fix(macro-expansion.md): fix the article `an` to `a` (rust-lang/rustc-dev-guide#1759)
- fix(serialization.md): fix the name of a derive macro (rust-lang/rustc-dev-guide#1756)
- fix(serialization.md): add a necessary plural suffix (rust-lang/rustc-dev-guide#1757)
- fix(salsa.md): add punctuation to prevent confusion (rust-lang/rustc-dev-guide#1754)
- fix(salsa.md): remove duplicate "To Be" verb (rust-lang/rustc-dev-guide#1755)
- feat(fuzzing.md): make `halfempty` word a link (rust-lang/rustc-dev-guide#1750)
- fix(about.md): use `a` instead of `an` (rust-lang/rustc-dev-guide#1751)
- refactor(git.md): make git-scm links clickable (rust-lang/rustc-dev-guide#1747)
- fix(walkthrough.md) add a comma operator to eliminate ambiguity (rust-lang/rustc-dev-guide#1749)
- fix(git.md): remove a confusing end of sentence character (rust-lang/rustc-dev-guide#1748)
- refactor(profiling/with_perf): remove a wrong to be verb (rust-lang/rustc-dev-guide#1746)
- refactor(tests/headers): remove duplicate list item (rust-lang/rustc-dev-guide#1745)
- refactor(test/headers.md): make the meaning more obvious (rust-lang/rustc-dev-guide#1744)
- refactor(tests/ui): remove unnecessary duplicate word (rust-lang/rustc-dev-guide#1743)
- refactor(compiletest): remove unnecessary duplicate word (rust-lang/rustc-dev-guide#1742)
- generic_arguments.md: substs -> GenericArgs (rust-lang/rustc-dev-guide#1741)
- fix(suggested): remove an unnecessary and confusing statement (rust-lang/rustc-dev-guide#1739)
- fix(how-to-build-and-run): fix a typo ("fromer" -> "former") (rust-lang/rustc-dev-guide#1736)
- fix(how-to-build-and-run): remove a wrong paragraph (rust-lang/rustc-dev-guide#1735)
- coverage code has moved (rust-lang/rustc-dev-guide#1728)
- linked issue is closed (rust-lang/rustc-dev-guide#1729)
- remove duplicate reference in about-this-guide.md (rust-lang/rustc-dev-guide#1734)
- Explain more in depth what early and late bound generic parameters are (rust-lang/rustc-dev-guide#1732)
- add section for normalization with the new solver (rust-lang/rustc-dev-guide#1731)
- Improve cleanup-crew.md with an example post (rust-lang/rustc-dev-guide#1730)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 14, 2023
Update books

## rust-lang/book

3 commits in 668c64760b5c7ea654facb4ba5fe9faddfda27cc..72187f5cd0beaaa9c6f584156bcd88f921871e83
2023-08-04 14:42:07 UTC to 2023-08-03 13:36:44 UTC

- redirects: change link for the `#![no_std]` tutorial (rust-lang/book#3705)
- [chpt10.2] - Small wording changes (rust-lang/book#3724)
- Improve sentence (rust-lang/book#3725)

## rust-embedded/book

1 commits in 1e5556dd1b864109985d5871616ae6b9164bcead..99ad2847b865e96d8ae7b333d3ee96963557e621
2023-08-11 06:31:04 UTC to 2023-08-11 06:31:04 UTC

- Fix a small typo in qemu.md (rust-embedded/book#359)

## rust-lang/nomicon

1 commits in 302b995bcb24b70fd883980fd174738c3a10b705..388750b081c0893c275044d37203f97709e058ba
2023-08-10 21:15:21 UTC to 2023-08-10 21:15:21 UTC

- Document thiscall abi (rust-lang/nomicon#311)

## rust-lang/reference

10 commits in 1ea0178266b3f3f613b0fabdaf16a83961c99cdb..d43038932adeb16ada80e206d4c073d851298101
2023-08-12 19:07:28 UTC to 2023-07-16 20:12:46 UTC

- Document thiscall abi (rust-lang/reference#1092)
- add section about implied bounds (rust-lang/reference#1261)
- Clearly specify the `instruction_set` effects (rust-lang/reference#1307)
- Fix merge queue building twice. (rust-lang/reference#1383)
- Clarify UB around immutability & mutation (rust-lang/reference#1385)
- mention the extra const UB (rust-lang/reference#1273)
- Operator expressions: make the note about division by zero clearer. (rust-lang/reference#1384)
- Make unsafe keyword docs less confusing (rust-lang/reference#1379)
- Say that division by zero for primitive types panics (rust-lang/reference#1382)
- Add CI trigger for merge queues. (rust-lang/reference#1381)

## rust-lang/rust-by-example

3 commits in 8a87926a985ce32ca1fad1be4008ee161a0b91eb..07e0df2f006e59d171c6bf3cafa9d61dbeb520d8
2023-07-24 11:37:55 UTC to 2023-07-24 11:35:36 UTC

- Added attribute unused_labels - fixed warning. (rust-lang/rust-by-example#1729)
- more explanation about panic (rust-lang/rust-by-example#1728)
- chore: add the portuguese version of this project to `readme.md` (rust-lang/rust-by-example#1727)

## rust-lang/rustc-dev-guide

31 commits in b5a12d95e32ae53791cc6ab44417774667ed2ac6..b123ab4754127d822ffb38349ce0fbf561f1b2fd
2023-08-14 08:34:59 UTC to 2023-07-11 06:02:34 UTC

- fix: stabilize debugger_visualizer (rust-lang/rustc-dev-guide#1766)
- feat(part-5-intro): make "Part 5" obvious (rust-lang/rustc-dev-guide#1753)
- Update from `#[warn_]` to `#[warning]` diagnostic attributes (rust-lang/rustc-dev-guide#1765)
- Add RPITIT documentation (rust-lang/rustc-dev-guide#1764)
- fix(visitor.md): fix a type name in a code sample (rust-lang/rustc-dev-guide#1762)
- fix(name-resolution): remove unnecessary closing paranthesis (rust-lang/rustc-dev-guide#1760)
- fix(macro-expansion.md): fix the article `an` to `a` (rust-lang/rustc-dev-guide#1759)
- fix(serialization.md): fix the name of a derive macro (rust-lang/rustc-dev-guide#1756)
- fix(serialization.md): add a necessary plural suffix (rust-lang/rustc-dev-guide#1757)
- fix(salsa.md): add punctuation to prevent confusion (rust-lang/rustc-dev-guide#1754)
- fix(salsa.md): remove duplicate "To Be" verb (rust-lang/rustc-dev-guide#1755)
- feat(fuzzing.md): make `halfempty` word a link (rust-lang/rustc-dev-guide#1750)
- fix(about.md): use `a` instead of `an` (rust-lang/rustc-dev-guide#1751)
- refactor(git.md): make git-scm links clickable (rust-lang/rustc-dev-guide#1747)
- fix(walkthrough.md) add a comma operator to eliminate ambiguity (rust-lang/rustc-dev-guide#1749)
- fix(git.md): remove a confusing end of sentence character (rust-lang/rustc-dev-guide#1748)
- refactor(profiling/with_perf): remove a wrong to be verb (rust-lang/rustc-dev-guide#1746)
- refactor(tests/headers): remove duplicate list item (rust-lang/rustc-dev-guide#1745)
- refactor(test/headers.md): make the meaning more obvious (rust-lang/rustc-dev-guide#1744)
- refactor(tests/ui): remove unnecessary duplicate word (rust-lang/rustc-dev-guide#1743)
- refactor(compiletest): remove unnecessary duplicate word (rust-lang/rustc-dev-guide#1742)
- generic_arguments.md: substs -> GenericArgs (rust-lang/rustc-dev-guide#1741)
- fix(suggested): remove an unnecessary and confusing statement (rust-lang/rustc-dev-guide#1739)
- fix(how-to-build-and-run): fix a typo ("fromer" -> "former") (rust-lang/rustc-dev-guide#1736)
- fix(how-to-build-and-run): remove a wrong paragraph (rust-lang/rustc-dev-guide#1735)
- coverage code has moved (rust-lang/rustc-dev-guide#1728)
- linked issue is closed (rust-lang/rustc-dev-guide#1729)
- remove duplicate reference in about-this-guide.md (rust-lang/rustc-dev-guide#1734)
- Explain more in depth what early and late bound generic parameters are (rust-lang/rustc-dev-guide#1732)
- add section for normalization with the new solver (rust-lang/rustc-dev-guide#1731)
- Improve cleanup-crew.md with an example post (rust-lang/rustc-dev-guide#1730)
@RalfJung RalfJung deleted the const-ub branch August 22, 2023 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants