10

const_eval: allow function pointer signatures containing &mut T in const con...

 1 year ago
source link: https://github.com/rust-lang/rust/pull/116015
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

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

Conversation

Contributor

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.

stepancheg, c410-f3r, and 0x7CFE reacted with thumbs up emoji

Collaborator

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

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

Contributor

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

oli-obk

added T-lang Relevant to the language team, which will review and decide on the PR/issue. I-lang-nominated Indicates that an issue has been 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

Contributor

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).

Contributor

@rfcbot fcp merge

rfcbot

commented

Oct 3, 2023

•

edited by scottmcm

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.

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

Contributor

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

Contributor

@rfcbot reviewed

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

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

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

Member

Cc @rust-lang/wg-const-eval

tmandry

removed the I-lang-nominated Indicates that an issue has been nominated for discussion during a lang team meeting. label

Oct 10, 2023

rfcbot

added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting

and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised.

labels

Oct 13, 2023

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.

Contributor

@bors r+

Contributor

📌 Commit d975ae5 has been approved by oli-obk

It is now in the queue for this repository.

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

Contributor

⌛ Testing commit d975ae5 with merge 2104b0df847b11a4e8ffa9aeca363838e5cc4830...

Collaborator

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)

Contributor

💔 Test failed - checks-actions

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

Contributor

@bors retry

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

Contributor

⌛ Testing commit d975ae5 with merge 139f63a...

Contributor

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

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

Collaborator

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

Cycles

Results

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%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

oli-obk

oli-obk left review comments
Assignees

cjgillot

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects

None yet

Milestone

1.75.0

Development

Successfully merging this pull request may close these issues.

Function pointer signature containing &mut T should work in const context

12 participants

report

Recommend

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK