const_eval: allow function pointer signatures containing &mut T in const contexts #116015

EvanMerlock · 2023-09-21T00:24:25Z

potentially fixes #114994

We utilize a TypeVisitor here in order to more easily handle control flow.

In the event the typekind the Visitor sees is a function pointer, we skip over it
However, otherwise we do one of two things:
- If we find a mutable reference, check it, then continue visiting types
- If we find any other type, continue visiting types

This means we will check if the function pointer itself is mutable, but not if any of the types within are.

rustbot · 2023-09-21T00:24:33Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cjgillot (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

cjgillot · 2023-09-21T06:33:19Z

cc @oli-obk as it touches const-fn checks.

compiler/rustc_const_eval/src/transform/check_consts/check.rs

- uses TypeReader for resolving local/return ty to support case where function pointer is before another type in a compound type

- add new testcase for TypeVisitor on const-eval mutable ref check

oli-obk · 2023-09-25T14:59:25Z

Nominating for T-lang FCP as this enables more code to compile. Specifically this allows mutable references in const fn, as long as they are part of a function pointer signature. We already support function pointers on stable (but not calling them), and we forbid all mutable references. This check was overzealous and also forbade function pointers that had mutable references as arguments, even though there is no risk associated with that at all, as function pointers can't be used at compile-time anyway (except to pass them around).

tmandry · 2023-10-03T18:27:00Z

@rfcbot fcp merge

rfcbot · 2023-10-03T18:27:01Z

Team member @tmandry has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

nikomatsakis · 2023-10-03T18:27:40Z

@oli-obk I'm fine with this PR, but it'd be nice to have examples of other places where &mut is disallowed (and why) so that we can see that those cases don't apply here.

nikomatsakis · 2023-10-03T18:30:35Z

@rfcbot reviewed

compiler-errors · 2023-10-03T18:31:07Z

@nikomatsakis: See #57349 for the remaining blockers for &mut in const, there seems to be some outstanding issue with statics (de)duplication.

rfcbot · 2023-10-03T18:31:12Z

🔔 This is now entering its final comment period, as per the review above. 🔔

RalfJung · 2023-10-03T18:45:24Z

Cc @rust-lang/wg-const-eval

rfcbot · 2023-10-13T18:33:58Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

oli-obk · 2023-10-13T20:17:28Z

@bors r+

bors · 2023-10-13T20:17:32Z

📌 Commit d975ae5 has been approved by oli-obk

It is now in the queue for this repository.

bors · 2023-10-14T04:31:15Z

⌛ Testing commit d975ae5 with merge 2104b0d...

const_eval: allow function pointer signatures containing &mut T in const contexts potentially fixes rust-lang#114994 We utilize a `TypeVisitor` here in order to more easily handle control flow. - In the event the typekind the Visitor sees is a function pointer, we skip over it - However, otherwise we do one of two things: - If we find a mutable reference, check it, then continue visiting types - If we find any other type, continue visiting types This means we will check if the function pointer _itself_ is mutable, but not if any of the types _within_ are.

rust-log-analyzer · 2023-10-14T05:43:14Z

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

tests\ui-toml\undocumented_unsafe_blocks\undocumented_unsafe_blocks.rs (default) ... ok
tests\ui-toml\undocumented_unsafe_blocks\undocumented_unsafe_blocks.rs (disabled) ... ok

tests\ui-toml\doc_valid_idents_append\doc_markdown.rs FAILED:
command: "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\clippy-driver.exe" "--error-format=json" "--emit=metadata" "-Aunused" "-Ainternal_features" "-Zui-testing" "-Dwarnings" "-Ldependency=C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps" "--extern=clippy_lints=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libclippy_lints-5e8d785e054edb28.rlib" "--extern=clippy_utils=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libclippy_utils-0cf228b0c1780fd5.rlib" "--extern=futures=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libfutures-a074388a1bf2e009.rlib" "--extern=if_chain=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libif_chain-ba0253edeba7a8f0.rlib" "--extern=itertools=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libitertools-6894b4f3e8c5523f.rlib" "--extern=parking_lot=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libparking_lot-f87e1900e520ff9b.rlib" "--extern=quote=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libquote-3ebce4a1d5d43cf1.rlib" "--extern=regex=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libregex-3e46ad2dfdd0c657.rlib" "--extern=serde=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libserde-12500b7f802b2beb.rlib" "--extern=serde_derive=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\release\\deps\\serde_derive-67ed154847593e2b.dll" "--extern=syn=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libsyn-bc3ce53f417d339c.rlib" "--extern=tokio=\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\deps\\libtokio-159ee9fbda356223.rlib" "-Ldependency=C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\release\\deps" "--out-dir" "\\\\?\\C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2-tools\\ui_test" "tests\\ui-toml\\doc_valid_idents_append\\doc_markdown.fixed" "--edition" "2021" "--crate-name" "doc_markdown"
called `Result::unwrap()` on an `Err` value: tests failed

Location:
    C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\ui_test-0.20.0\src\lib.rs:362:13
    C:\Users\runneradmin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\ui_test-0.20.0\src\lib.rs:362:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: test failed, to rerun pass `--test compile-test`

rustfix failed with exit code: 1

full stderr:
error: failed to write \\?\C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage2-tools\ui_test\libdoc_markdown.rmeta: Access is denied. (os error 5)
error: aborting due to previous error


full stdout:

bors · 2023-10-14T05:43:40Z

💔 Test failed - checks-actions

oli-obk · 2023-10-14T08:13:22Z

@bors retry

bors · 2023-10-14T09:18:32Z

⌛ Testing commit d975ae5 with merge 139f63a...

bors · 2023-10-14T11:03:57Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 139f63a to master...

rust-timer · 2023-10-14T14:08:50Z

Finished benchmarking commit (139f63a): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.9%	[0.9%, 0.9%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.3%	[1.3%, 1.3%]	1
Regressions ❌ (secondary)	1.6%	[1.3%, 2.1%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.3%	[1.3%, 1.3%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.3%	[1.3%, 1.3%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.3%	[1.3%, 1.3%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 627.527s -> 628.233s (0.11%)
Artifact size: 271.26 MiB -> 271.32 MiB (0.03%)

Pkgsrc changes: * Adjust patches and cargo checksums to new versions. * For an external LLVM, set dependency of llvm >= 16, in accordance with the upstream changes. * Mark that on NetBSD we now need >= 9.0, so 8.x is no longer supported. * On NetBSD/sparc64 10.x, we now need GCC 12 to build the embedded LLVM, which is version 17; apparently GCC 10.4 or 10.5 mis-compiles it, resulting in an illegal instruction fault during the build. Ref. rust-lang/rust#117231 Upstream changes: Version 1.75.0 (2023-12-28) ========================== - [Stabilize `async fn` and return-position `impl Trait` in traits.] (rust-lang/rust#115822) - [Allow function pointer signatures containing `&mut T` in `const` contexts.] (rust-lang/rust#116015) - [Match `usize`/`isize` exhaustively with half-open ranges.] (rust-lang/rust#116692) - [Guarantee that `char` has the same size and alignment as `u32`.] (rust-lang/rust#116894) - [Document that the null pointer has the 0 address.] (rust-lang/rust#116988) - [Allow partially moved values in `match`.] (rust-lang/rust#103208) - [Add notes about non-compliant FP behavior on 32bit x86 targets.] (rust-lang/rust#113053) - [Stabilize ratified RISC-V target features.] (rust-lang/rust#116485) Compiler -------- - [Rework negative coherence to properly consider impls that only partly overlap.] (rust-lang/rust#112875) - [Bump `COINDUCTIVE_OVERLAP_IN_COHERENCE` to deny, and warn in dependencies.] (rust-lang/rust#116493) - [Consider alias bounds when computing liveness in NLL.] (rust-lang/rust#116733) - [Add the V (vector) extension to the `riscv64-linux-android` target spec.] (rust-lang/rust#116618) - [Automatically enable cross-crate inlining for small functions] (rust-lang/rust#116505) - Add several new tier 3 targets: - [`csky-unknown-linux-gnuabiv2hf`] (rust-lang/rust#117049) - [`i586-unknown-netbsd`] (rust-lang/rust#117170) - [`mipsel-unknown-netbsd`] (rust-lang/rust#117356) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Override `Waker::clone_from` to avoid cloning `Waker`s unnecessarily.] (rust-lang/rust#96979) - [Implement `BufRead` for `VecDeque<u8>`.] (rust-lang/rust#110604) - [Implement `FusedIterator` for `DecodeUtf16` when the inner iterator does.] (rust-lang/rust#110729) - [Implement `Not, Bit{And,Or}{,Assign}` for IP addresses.] (rust-lang/rust#113747) - [Implement `Default` for `ExitCode`.] (rust-lang/rust#114589) - [Guarantee representation of None in NPO] (rust-lang/rust#115333) - [Document when atomic loads are guaranteed read-only.] (rust-lang/rust#115577) - [Broaden the consequences of recursive TLS initialization.] (rust-lang/rust#116172) - [Windows: Support sub-millisecond sleep.] (rust-lang/rust#116461) - [Fix generic bound of `str::SplitInclusive`'s `DoubleEndedIterator` impl] (rust-lang/rust#100806) - [Fix exit status / wait status on non-Unix `cfg(unix)` platforms.] (rust-lang/rust#115108) Stabilized APIs --------------- - [`Atomic*::from_ptr`] (https://doc.rust-lang.org/stable/core/sync/atomic/struct.AtomicUsize.html#method.from_ptr) - [`FileTimes`] (https://doc.rust-lang.org/stable/std/fs/struct.FileTimes.html) - [`FileTimesExt`] (https://doc.rust-lang.org/stable/std/os/windows/fs/trait.FileTimesExt.html) - [`File::set_modified`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.set_modified) - [`File::set_times`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.set_times) - [`IpAddr::to_canonical`] (https://doc.rust-lang.org/stable/core/net/enum.IpAddr.html#method.to_canonical) - [`Ipv6Addr::to_canonical`] (https://doc.rust-lang.org/stable/core/net/struct.Ipv6Addr.html#method.to_canonical) - [`Option::as_slice`] (https://doc.rust-lang.org/stable/core/option/enum.Option.html#method.as_slice) - [`Option::as_mut_slice`] (https://doc.rust-lang.org/stable/core/option/enum.Option.html#method.as_mut_slice) - [`pointer::byte_add`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_add) - [`pointer::byte_offset`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_offset) - [`pointer::byte_offset_from`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_offset_from) - [`pointer::byte_sub`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_sub) - [`pointer::wrapping_byte_add`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_add) - [`pointer::wrapping_byte_offset`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_offset) - [`pointer::wrapping_byte_sub`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_sub) These APIs are now stable in const contexts: - [`Ipv6Addr::to_ipv4_mapped`] (https://doc.rust-lang.org/stable/core/net/struct.Ipv6Addr.html#method.to_ipv4_mapped) - [`MaybeUninit::assume_init_read`] (https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#method.assume_init_read) - [`MaybeUninit::zeroed`] (https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#method.zeroed) - [`mem::discriminant`] (https://doc.rust-lang.org/stable/core/mem/fn.discriminant.html) - [`mem::zeroed`] (https://doc.rust-lang.org/stable/core/mem/fn.zeroed.html) Cargo ----- - [Add new packages to `[workspace.members]` automatically.] (rust-lang/cargo#12779) - [Allow version-less `Cargo.toml` manifests.] (rust-lang/cargo#12786) - [Make browser links out of HTML file paths.] (rust-lang/cargo#12889) Rustdoc ------- - [Accept less invalid Rust in rustdoc.] (rust-lang/rust#117450) - [Document lack of object safety on affected traits.] (rust-lang/rust#113241) - [Hide `#[repr(transparent)]` if it isn't part of the public ABI.] (rust-lang/rust#115439) - [Show enum discriminant if it is a C-like variant.] (rust-lang/rust#116142) Compatibility Notes ------------------- - [FreeBSD targets now require at least version 12.] (rust-lang/rust#114521) - [Formally demote tier 2 MIPS targets to tier 3.] (rust-lang/rust#115238) - [Make misalignment a hard error in `const` contexts.] (rust-lang/rust#115524) - [Fix detecting references to packed unsized fields.] (rust-lang/rust#115583) - [Remove support for compiler plugins.] (rust-lang/rust#116412)

rustbot assigned cjgillot Sep 21, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 21, 2023

oli-obk reviewed Sep 21, 2023

View reviewed changes

compiler/rustc_const_eval/src/transform/check_consts/check.rs Outdated Show resolved Hide resolved

EvanMerlock added 2 commits September 22, 2023 19:43

fix(const_eval): correctly check const type for function pointers

fd1ccbc

- uses TypeReader for resolving local/return ty to support case where function pointer is before another type in a compound type

test(const_eval): add test cases for rust-lang#114994

d975ae5

- add new testcase for TypeVisitor on const-eval mutable ref check

EvanMerlock force-pushed the master branch from e0c75f7 to d975ae5 Compare September 23, 2023 00:43

oli-obk added T-lang Relevant to the language team, which will review and decide on the PR/issue. I-lang-nominated Nominated for discussion during a lang team meeting. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 25, 2023

oli-obk marked this pull request as ready for review September 25, 2023 14:55

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 3, 2023

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Oct 3, 2023

tmandry removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Oct 10, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 13, 2023

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 14, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 14, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 14, 2023

bors merged commit 139f63a into rust-lang:master Oct 14, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Oct 14, 2023

Veykril mentioned this pull request Oct 17, 2023

Pull upstream master 2023 10 17 ferrocene/ferrocene#54

Merged

apiraino removed the to-announce Announce this issue on triage meeting label Oct 19, 2023

sorpaas mentioned this pull request Oct 29, 2023

Introduce the concept of Etable in core rust-ethereum/evm#186

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

const_eval: allow function pointer signatures containing &mut T in const contexts #116015

const_eval: allow function pointer signatures containing &mut T in const contexts #116015

EvanMerlock commented Sep 21, 2023 •

edited

Loading

rustbot commented Sep 21, 2023

cjgillot commented Sep 21, 2023

oli-obk commented Sep 25, 2023 •

edited

Loading

tmandry commented Oct 3, 2023

rfcbot commented Oct 3, 2023 •

edited by scottmcm

Loading

nikomatsakis commented Oct 3, 2023

nikomatsakis commented Oct 3, 2023

compiler-errors commented Oct 3, 2023

rfcbot commented Oct 3, 2023

RalfJung commented Oct 3, 2023

rfcbot commented Oct 13, 2023

oli-obk commented Oct 13, 2023

bors commented Oct 13, 2023

bors commented Oct 14, 2023

rust-log-analyzer commented Oct 14, 2023

bors commented Oct 14, 2023

oli-obk commented Oct 14, 2023

bors commented Oct 14, 2023

bors commented Oct 14, 2023

rust-timer commented Oct 14, 2023

const_eval: allow function pointer signatures containing &mut T in const contexts #116015

const_eval: allow function pointer signatures containing &mut T in const contexts #116015

Conversation

EvanMerlock commented Sep 21, 2023 • edited Loading

rustbot commented Sep 21, 2023

cjgillot commented Sep 21, 2023

oli-obk commented Sep 25, 2023 • edited Loading

tmandry commented Oct 3, 2023

rfcbot commented Oct 3, 2023 • edited by scottmcm Loading

nikomatsakis commented Oct 3, 2023

nikomatsakis commented Oct 3, 2023

compiler-errors commented Oct 3, 2023

rfcbot commented Oct 3, 2023

RalfJung commented Oct 3, 2023

rfcbot commented Oct 13, 2023

oli-obk commented Oct 13, 2023

bors commented Oct 13, 2023

bors commented Oct 14, 2023

rust-log-analyzer commented Oct 14, 2023

bors commented Oct 14, 2023

oli-obk commented Oct 14, 2023

bors commented Oct 14, 2023

bors commented Oct 14, 2023

rust-timer commented Oct 14, 2023

Overall result: ❌ regressions - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

EvanMerlock commented Sep 21, 2023 •

edited

Loading

oli-obk commented Sep 25, 2023 •

edited

Loading

rfcbot commented Oct 3, 2023 •

edited by scottmcm

Loading