Skip to content

Fix reborrow_info early return skipping field validation#156784

Open
ikow wants to merge 2 commits into
rust-lang:mainfrom
ikow:fix/reborrow-early-return
Open

Fix reborrow_info early return skipping field validation#156784
ikow wants to merge 2 commits into
rust-lang:mainfrom
ikow:fix/reborrow-early-return

Conversation

@ikow
Copy link
Copy Markdown

@ikow ikow commented May 20, 2026

Summary

reborrow_info() in compiler/rustc_hir_analysis/src/coherence/builtin.rs validates that all data fields of a struct implementing Reborrow are either Copy or Reborrow. However, when iterating over data fields, finding a Reborrow field caused an immediate return Ok(()) (line 567), skipping validation of all remaining fields.

This could allow a struct with a non-Copy, non-Reborrow second data field to incorrectly implement Reborrow, potentially leading to unsound bitwise copies of types that should not be copied.

Fix

Change return Ok(()) to continue so the loop validates all fields, not just the first one.

Test plan

  • Added tests/ui/reborrow/reborrow_multi_field_validation.rs — a compile-fail test with a struct that has a Reborrow first field and a String (non-Copy, non-Reborrow) second field
  • Before this fix, the compiler incorrectly accepts this impl; after the fix, it correctly rejects it

Tracking issue: #145612 (reborrow feature)

The `reborrow_info` function validates that all data fields of a struct
implementing `Reborrow` are either `Copy` or themselves `Reborrow`.
However, when iterating over data fields, finding a `Reborrow` field
caused an immediate `return Ok(())`, skipping validation of all
remaining fields. This could allow a struct with a non-Copy,
non-Reborrow second field to incorrectly implement `Reborrow`.

Fix: change `return Ok(())` to `continue` so all fields are checked.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 20, 2026
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 20, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 20, 2026

r? @dingxiangfei2009

rustbot has assigned @dingxiangfei2009.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 20 candidates

@rust-log-analyzer

This comment has been minimized.

@dingxiangfei2009
Copy link
Copy Markdown
Contributor

cc @aapoalas

Ah right, this is indeed a typo.


struct Bad<'a> {
first: &'a mut i32,
second: String, //~ ERROR the trait bound `String: Copy` is not satisfied
Copy link
Copy Markdown
Contributor

@dingxiangfei2009 dingxiangfei2009 May 20, 2026

Choose a reason for hiding this comment

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

Let's ./x t tests/ui/reborrow/reborrow_multi_field_validation.rs --bless once.

View changes since the review

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

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

Oh; well that'd explain a few problems! Thank you <3

View changes since this review

Copy link
Copy Markdown
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

Please strip the commit "co authored by LLM" thing. Either it is your work, or it is not. If it weren't, we could just use the tools ourselves instead of filtering our reviews through you to your LLM.

Similarly do not include "test" sections in your PR description. We do see test changes in the diff.

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 21, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 21, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

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

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants