Skip to content

Fix several bugs in ferrocene::prevalidated#2336

Merged
bors-ferrocene[bot] merged 9 commits intomainfrom
jyn/assoc-items-lint
May 1, 2026
Merged

Fix several bugs in ferrocene::prevalidated#2336
bors-ferrocene[bot] merged 9 commits intomainfrom
jyn/assoc-items-lint

Conversation

@jyn514
Copy link
Copy Markdown
Contributor

@jyn514 jyn514 commented Apr 22, 2026

Thanks to @Urhengulas for catching these.

  1. The THIR pass didn't check associated items at all. This is fixed in the first commit and tested in tests/ui/ferrocene/lint-prevalidated/thir-impl-item.rs. The second commit adds the missing items to libcore.
  2. The post-mono pass didn't check dynamic casts. I had correctly "failed closed" here -- the THIR pass would ICE if it wasn't able to check a cast — but because of 1. there were several casts in libcore that weren't caught, including in libcore. The ICE is fixed in the third commit and the post-mono pass is extended in the fourth. This is tested in tests/ui/ferrocene/lint-prevalidated/dyn-cast-post-mono.rs.
  3. The post-mono pass can't handle MIR inlining. I tried to get this working (see https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/SourceScopeData.20-.3E.20SourceScope/with/590200295), but wasn't able to figure out how to do it reliably. Instead I just disable mir inlining in the 5th commit. I got this working, and added a test in tests/ui/ferrocene/lint-prevalidated/mir-inlining.rs.

Comment thread compiler/rustc_lint/src/ferrocene/mod.rs Outdated
@jyn514 jyn514 force-pushed the jyn/assoc-items-lint branch from c24f42b to 86ce85e Compare April 22, 2026 14:13
Comment thread compiler/rustc_lint/src/ferrocene/dynamic_casts.rs Outdated
Copy link
Copy Markdown
Member

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

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

Overall looking good, we discussed internally that we'd like to investigate more the safety of disabling the inline_mir option.

Comment thread ferrocene/doc/symbol-report.csv
@jyn514 jyn514 force-pushed the jyn/assoc-items-lint branch from 86ce85e to db6fc38 Compare April 23, 2026 14:55
@jyn514
Copy link
Copy Markdown
Contributor Author

jyn514 commented Apr 23, 2026

we discussed internally that we'd like to investigate more the safety of disabling the inline_mir option.

I've investigated this. I've found that disabling -Z inline-mir actually fixes several unsound known-bug tests, which ... is good for us but concerning in general. My opinion is that we should have disabled inline-mir several releases ago; the timing is unfortunate but I stand by this change.

Copy link
Copy Markdown
Member

@Urhengulas Urhengulas left a comment

Choose a reason for hiding this comment

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

Seems good, although I cannot fully assess if the compiler/lint code is correct. I also tested it on the library I was using it and it now successfully detects the unvalidated functions that it did not detect before.

Comment thread compiler/rustc_lint/src/ferrocene/diagnostics.rs
Comment thread compiler/rustc_lint/src/ferrocene/dynamic_casts.rs
Comment thread compiler/rustc_lint/src/ferrocene/mod.rs Outdated
@Urhengulas
Copy link
Copy Markdown
Member

This PR affects coverage like following:

Executing ./x test library/core library/alloc --coverage=library --tests on this branch and comparing it to the coverage from the last nightly build shows that this PR changes coverage (with "count annotated lines as tested") like following:

last nightly this PR Diff
Lines 99.05% (32824/33138 lines) 96.07% (33816/35198 lines) -2.98%
Fully Tested 7676 8069 +390
Partially tested 30 12 -18
Fully untested 17 421 +404
Fully ignored 314 314 +0
Total 8037 8816 +779

The lower number for partially tested is odd...

Copy link
Copy Markdown
Member

@Urhengulas Urhengulas left a comment

Choose a reason for hiding this comment

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

bors merge

@bors-ferrocene
Copy link
Copy Markdown
Contributor

🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

bors-ferrocene Bot added a commit that referenced this pull request Apr 24, 2026
2291: backport: squash and use `cherry-pick`, not rebase r=Urhengulas a=jyn514

This hits many fewer conflicts when backporting. Now, instead of requiring every commit to backport cleanly, we only require the diff between `main` and the PR to backport cleanly. This decreases the fraction of PRs that need to be submitted manually, and also reduces the manual work needed to backport PRs when a manual backport is needed.

There are also various other changes to make the backporting scripts more maintainable and testable; see each individual commit for details. Unlike before, this does not mess with the current git state; running `git cherry-pick --abort` puts you back in exactly the same state you were in before you ran `one.py`.

See #2301 for an example PR opened by this automation, and https://github.com/ferrocene/ferrocene/actions/runs/24351997960/job/71108663815 for a sample job output.

---

## Testing

Here is a sample session using `one.py` to do a manual backport for the upcoming release:
```console
$ git checkout origin/release/1.95
HEAD is now at a31dd5c Merge #2258
$ git checkout jyn/backport-automation ferrocene/tools/backport ferrocene/tools/automations-common
$ ferrocene/tools/backport/one.py 2245
ERROR: this script cannot be run when you have staged changes; try running `git reset`
$ git reset
Unstaged changes after reset:
M	ferrocene/tools/automations-common/src/automations_common/automated_prs.py
M	ferrocene/tools/backport/all.py
M	ferrocene/tools/backport/one.py
$ ferrocene/tools/backport/one.py 2245                                                                                                                                                                                                         17:45
warning: if no API token is set in the GITHUB_TOKEN env var, requests may be rate-limited
INFO: backporting #2245 to a31dd5c (please do not CTRL-C)

Auto-merging .circleci/workflows.yml
Auto-merging ferrocene/ci/scripts/setup-uv.sh
CONFLICT (content): Merge conflict in ferrocene/ci/scripts/setup-uv.sh
error: could not apply d3faae633305... Backport #2245

ERROR: Encountered a merge conflict backporting #2245

INFO: attempting to auto-solve conflicts with mergiraf
WARN Mergiraf: Could not find a supported language for ferrocene/ci/scripts/setup-uv.sh
HELP: Fix the conflicts, run 'git add/rm <filepath>', then run 'git cherry-pick --continue'.
NOTE: Running 'git cherry-pick --abort' will put you back in your original git state.

NOTE: conflicts exist in the following locations:
ferrocene/ci/scripts/setup-uv.sh:7-17
NOTE: 'HEAD' in the conflicted files refers to your current branch, a31dd5c

$ git diff --name-status --cached
M       .circleci/workflows.yml
U       ferrocene/ci/scripts/setup-uv.sh
$ git status | head -n5
HEAD detached at origin/release/1.95
You are currently cherry-picking commit d3faae633305.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)
```

(After this lands on beta, the `git checkout ferrocene/tools && git reset` lines will no longer be needed.)

2336: Fix several bugs in `ferrocene::prevalidated` r=Urhengulas a=jyn514

Thanks to `@Urhengulas` for catching these.

1. The THIR pass didn't check associated items at all. This is fixed in the first commit and tested in `tests/ui/ferrocene/lint-prevalidated/thir-impl-item.rs`. The second commit adds the missing items to libcore.
2. The post-mono pass didn't check dynamic casts. I had correctly "failed closed" here -- the THIR pass would ICE if it wasn't able to check a cast — but because of 1. there were several casts in libcore that weren't caught, including in libcore. The ICE is fixed in the third commit and the post-mono pass is extended in the fourth. This is tested in `tests/ui/ferrocene/lint-prevalidated/dyn-cast-post-mono.rs`.
3. The post-mono pass can't handle MIR inlining. I tried to get this working (see https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/SourceScopeData.20-.3E.20SourceScope/with/590200295), but wasn't able to figure out how to do it reliably. Instead I just disable mir inlining in the 5th commit. This is tested in `tests/ui/ferrocene/lint-prevalidated/mir-inlining.rs`.

Co-authored-by: Jynn Nelson <[email protected]>
Co-authored-by: Urhengulas <[email protected]>
@bors-ferrocene
Copy link
Copy Markdown
Contributor

Build failed (retrying...):

bors-ferrocene Bot added a commit that referenced this pull request Apr 24, 2026
2336: Fix several bugs in `ferrocene::prevalidated` r=Urhengulas a=jyn514

Thanks to `@Urhengulas` for catching these.

1. The THIR pass didn't check associated items at all. This is fixed in the first commit and tested in `tests/ui/ferrocene/lint-prevalidated/thir-impl-item.rs`. The second commit adds the missing items to libcore.
2. The post-mono pass didn't check dynamic casts. I had correctly "failed closed" here -- the THIR pass would ICE if it wasn't able to check a cast — but because of 1. there were several casts in libcore that weren't caught, including in libcore. The ICE is fixed in the third commit and the post-mono pass is extended in the fourth. This is tested in `tests/ui/ferrocene/lint-prevalidated/dyn-cast-post-mono.rs`.
3. The post-mono pass can't handle MIR inlining. I tried to get this working (see https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/SourceScopeData.20-.3E.20SourceScope/with/590200295), but wasn't able to figure out how to do it reliably. Instead I just disable mir inlining in the 5th commit. This is tested in `tests/ui/ferrocene/lint-prevalidated/mir-inlining.rs`.

Co-authored-by: Jynn Nelson <[email protected]>
Co-authored-by: Urhengulas <[email protected]>
@bors-ferrocene
Copy link
Copy Markdown
Contributor

Build failed:

  • full

@jyn514 jyn514 force-pushed the jyn/assoc-items-lint branch from f264a88 to 565fa40 Compare April 27, 2026 10:00
jyn514 and others added 7 commits April 27, 2026 12:07
Previously this wouldn't get caught until the post-mono pass, which made
it annoying to check whether generic inline functions were validated.
- UnresolvedGenericImpl is unreachable outside THIR
- fix typo
plus other minor fixes
@jyn514 jyn514 force-pushed the jyn/assoc-items-lint branch from 565fa40 to 3a7e4b8 Compare April 27, 2026 10:10
@Hoverbear
Copy link
Copy Markdown
Member

bors try

bors-ferrocene Bot added a commit that referenced this pull request Apr 27, 2026
@Hoverbear
Copy link
Copy Markdown
Member

image @jyn514 I think this is unintentional?

@bors-ferrocene
Copy link
Copy Markdown
Contributor

try

Build failed:

Comment thread src/tools/cargo
@jyn514 jyn514 force-pushed the jyn/assoc-items-lint branch from a80d0e1 to c27f4d1 Compare April 28, 2026 08:45
@jyn514
Copy link
Copy Markdown
Contributor Author

jyn514 commented Apr 28, 2026

bors try

bors-ferrocene Bot added a commit that referenced this pull request Apr 28, 2026
@bors-ferrocene
Copy link
Copy Markdown
Contributor

try

Build failed:

These weren't tested before, since we only generated our report for
x86_64-linux-gnu.
@jyn514
Copy link
Copy Markdown
Contributor Author

jyn514 commented Apr 29, 2026

bors try

bors-ferrocene Bot added a commit that referenced this pull request Apr 29, 2026
@bors-ferrocene
Copy link
Copy Markdown
Contributor

try

Build failed:

@jyn514 jyn514 force-pushed the jyn/assoc-items-lint branch from c27f4d1 to 2592e12 Compare May 1, 2026 09:40
@jyn514
Copy link
Copy Markdown
Contributor Author

jyn514 commented May 1, 2026

bors try

bors-ferrocene Bot added a commit that referenced this pull request May 1, 2026
@bors-ferrocene
Copy link
Copy Markdown
Contributor

bors-ferrocene Bot commented May 1, 2026

try

Build failed:

@jyn514
Copy link
Copy Markdown
Contributor Author

jyn514 commented May 1, 2026

Copy link
Copy Markdown
Member

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

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

bors merge

@bors-ferrocene
Copy link
Copy Markdown
Contributor

bors-ferrocene Bot commented May 1, 2026

Build succeeded:

@bors-ferrocene bors-ferrocene Bot merged commit 066d818 into main May 1, 2026
5 of 6 checks passed
@jyn514 jyn514 deleted the jyn/assoc-items-lint branch May 1, 2026 16:52
@ferrocene-automations ferrocene-automations Bot added the backport:manual PR requiring manual intervention to backport label May 4, 2026
Hoverbear pushed a commit that referenced this pull request May 5, 2026
Fix several bugs in `ferrocene::prevalidated`

Thanks to @Urhengulas for catching these.

1. The THIR pass didn't check associated items at all. This is fixed in the first commit and tested in `tests/ui/ferrocene/lint-prevalidated/thir-impl-item.rs`. The second commit adds the missing items to libcore.
2. The post-mono pass didn't check dynamic casts. I had correctly "failed closed" here -- the THIR pass would ICE if it wasn't able to check a cast — but because of 1. there were several casts in libcore that weren't caught, including in libcore. The ICE is fixed in the third commit and the post-mono pass is extended in the fourth. This is tested in `tests/ui/ferrocene/lint-prevalidated/dyn-cast-post-mono.rs`.
3. The post-mono pass can't handle MIR inlining. ~~I tried to get this working (see https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/SourceScopeData.20-.3E.20SourceScope/with/590200295), but wasn't able to figure out how to do it reliably. Instead I just disable mir inlining in the 5th commit.~~ I got this working, and added a test in `tests/ui/ferrocene/lint-prevalidated/mir-inlining.rs`.Co-authored-by: {author}

Ferrocene-backport-of: #2336
Ferrocene-backported-commits: 2592e12 fdea74a 3a7e4b8 5a3e3bd 0c9007d dcfd52b 383e22d 4c7293c 9d9681c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:manual PR requiring manual intervention to backport backport:1.95 merged-in:1.97

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants