Skip to content

Store DefId instead of EiiDecl in EiiImplResolution::Known#158235

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
cezarbbb:eii-metadata-opt
Jul 15, 2026
Merged

Store DefId instead of EiiDecl in EiiImplResolution::Known#158235
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
cezarbbb:eii-metadata-opt

Conversation

@cezarbbb

@cezarbbb cezarbbb commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Fixes the FIXME(eii) about removing the extern target from the encoded decl.

Known(EiiDecl) serialized the same EiiDecl N+1 times per entry (once as the declaration, once per Known impl). Changing to Known(DefId) eliminates that redundancy — the EiiDecl is recoverable via tcx.externally_implementable_items() or a local lookup map, consistent with how Rust metadata uses DefId as a reference elsewhere.

I believe there are benefits in both performance and size(full EiiDecl to single DefId, and O(N) to O(1) lookup).

Also fixes a soundness gap(I think?) in check_attr.rs where the Known branch skipped impl_unsafe checking, and replaces an .expect() in codegen_attrs.rs with a let Some pattern.

tracking issues: #125418

r? @jdonszelmann

@rustbot

rustbot commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) 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 Jun 22, 2026
@rustbot

rustbot commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

jdonszelmann is currently at their maximum review capacity.
They may take a while to respond.

Comment thread compiler/rustc_metadata/src/eii.rs Outdated
Comment thread compiler/rustc_codegen_ssa/src/codegen_attrs.rs Outdated
@rust-bors

This comment has been minimized.

@cezarbbb cezarbbb force-pushed the eii-metadata-opt branch from 499975b to c65973d Compare July 6, 2026 02:50
@rustbot

rustbot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Comment thread compiler/rustc_ast_lowering/src/item.rs Outdated
@bjorn3

This comment was marked as resolved.

@bjorn3 bjorn3 added the F-extern_item_impls `#![feature(extern_item_impls)]` label Jul 8, 2026
@bjorn3

bjorn3 commented Jul 8, 2026

Copy link
Copy Markdown
Member

r? bjorn3

@rustbot rustbot assigned bjorn3 and unassigned jdonszelmann Jul 8, 2026
@cezarbbb cezarbbb force-pushed the eii-metadata-opt branch from c65973d to b645391 Compare July 9, 2026 02:51
@cezarbbb

cezarbbb commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

You should be able to simplify this a bit further:

Applied. After this PR the Known branch only reads target.foreign_item, so impl_unsafe/name in the stored EiiDecl are dead. Slimming it to Option<Path> cleans that up. (impl_unsafe in generate_default_impl is still consumed by the impl_safety line, so that one stays.)

Comment thread compiler/rustc_hir/src/attrs/data_structures.rs Outdated
@bjorn3

bjorn3 commented Jul 14, 2026

Copy link
Copy Markdown
Member

@bors r+

@rust-bors

rust-bors Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 48a980d has been approved by bjorn3

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 Jul 14, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jul 14, 2026
Store `DefId` instead of `EiiDecl` in `EiiImplResolution::Known`

Fixes the `FIXME(eii)` about removing the extern target from the encoded decl.

`Known(EiiDecl)` serialized the same `EiiDecl` N+1 times per entry (once as the declaration, once per Known impl). Changing to `Known(DefId)` eliminates that redundancy — the `EiiDecl` is recoverable via `tcx.externally_implementable_items()` or a local lookup map, consistent with how Rust metadata uses `DefId` as a reference elsewhere.

I believe there are benefits in both performance and size(full `EiiDecl` to single `DefId`, and O(N) to O(1) lookup).

Also fixes a soundness gap(I think?) in `check_attr.rs` where the `Known` branch skipped `impl_unsafe` checking, and replaces an `.expect()` in `codegen_attrs.rs` with `a let Some` pattern.

tracking issues: rust-lang#125418

r? @jdonszelmann
rust-bors Bot pushed a commit that referenced this pull request Jul 14, 2026
…uwer

Rollup of 22 pull requests

Successful merges:

 - #156047 (Fix trait method resolution on an adjusted never type)
 - #157824 (Comptime inherent impls)
 - #158235 (Store `DefId` instead of `EiiDecl` in `EiiImplResolution::Known`)
 - #158723 (Support EII on Windows MSVC)
 - #158993 (rerun in original typing mode if we meet any opaques in post analysis)
 - #159160 (Make `HasTokens` a sub-trait of `HasAttrs`.)
 - #159183 (Introduce InstanceKind::LlvmIntrinsic)
 - #159251 (Bump rustc-perf submodule)
 - #155013 (Suggest the `[const] Destruct` bound for type parameters in const functions when missing)
 - #159155 (unstable book: Document `diagnostic_on_unknown` feature)
 - #159235 (Add regression test for #95719)
 - #159243 (inline Once wait and wait_force)
 - #159255 (Replace shortened type with `_` instead of `...` as placeholder)
 - #159259 (Add regression test for #144033)
 - #159265 (bootstrap: skip intrinsic-test when rustfmt is unavailable)
 - #159269 (disable range-len-try-from.rs on s390x)
 - #159272 (slice: make swap delegate to swap_unchecked)
 - #159274 (bootstrap: Rename `std_crates_for_run_make` to `std_crates_for_make_run`)
 - #159275 (remove obsolete comment)
 - #159277 (Construct `tokens` for attrs made by `mk_attr_word` and other variants)
 - #159283 (Remove obsolete verbose flag from deref/ref suggestions)
 - #159290 (rustc-dev-guide subtree update)
rust-bors Bot pushed a commit that referenced this pull request Jul 15, 2026
…uwer

Rollup of 22 pull requests

Successful merges:

 - #156047 (Fix trait method resolution on an adjusted never type)
 - #157824 (Comptime inherent impls)
 - #158235 (Store `DefId` instead of `EiiDecl` in `EiiImplResolution::Known`)
 - #158723 (Support EII on Windows MSVC)
 - #158993 (rerun in original typing mode if we meet any opaques in post analysis)
 - #159160 (Make `HasTokens` a sub-trait of `HasAttrs`.)
 - #159183 (Introduce InstanceKind::LlvmIntrinsic)
 - #159251 (Bump rustc-perf submodule)
 - #155013 (Suggest the `[const] Destruct` bound for type parameters in const functions when missing)
 - #159155 (unstable book: Document `diagnostic_on_unknown` feature)
 - #159235 (Add regression test for #95719)
 - #159243 (inline Once wait and wait_force)
 - #159255 (Replace shortened type with `_` instead of `...` as placeholder)
 - #159259 (Add regression test for #144033)
 - #159265 (bootstrap: skip intrinsic-test when rustfmt is unavailable)
 - #159269 (disable range-len-try-from.rs on s390x)
 - #159272 (slice: make swap delegate to swap_unchecked)
 - #159274 (bootstrap: Rename `std_crates_for_run_make` to `std_crates_for_make_run`)
 - #159275 (remove obsolete comment)
 - #159277 (Construct `tokens` for attrs made by `mk_attr_word` and other variants)
 - #159283 (Remove obsolete verbose flag from deref/ref suggestions)
 - #159290 (rustc-dev-guide subtree update)
@rust-bors rust-bors Bot merged commit 9f5052b into rust-lang:main Jul 15, 2026
13 checks passed
@rustbot rustbot added this to the 1.99.0 milestone Jul 15, 2026
rust-timer added a commit that referenced this pull request Jul 15, 2026
Rollup merge of #158235 - cezarbbb:eii-metadata-opt, r=bjorn3

Store `DefId` instead of `EiiDecl` in `EiiImplResolution::Known`

Fixes the `FIXME(eii)` about removing the extern target from the encoded decl.

`Known(EiiDecl)` serialized the same `EiiDecl` N+1 times per entry (once as the declaration, once per Known impl). Changing to `Known(DefId)` eliminates that redundancy — the `EiiDecl` is recoverable via `tcx.externally_implementable_items()` or a local lookup map, consistent with how Rust metadata uses `DefId` as a reference elsewhere.

I believe there are benefits in both performance and size(full `EiiDecl` to single `DefId`, and O(N) to O(1) lookup).

Also fixes a soundness gap(I think?) in `check_attr.rs` where the `Known` branch skipped `impl_unsafe` checking, and replaces an `.expect()` in `codegen_attrs.rs` with `a let Some` pattern.

tracking issues: #125418

r? @jdonszelmann
@cezarbbb cezarbbb deleted the eii-metadata-opt branch July 15, 2026 03:37
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Jul 15, 2026
…uwer

Rollup of 22 pull requests

Successful merges:

 - rust-lang/rust#156047 (Fix trait method resolution on an adjusted never type)
 - rust-lang/rust#157824 (Comptime inherent impls)
 - rust-lang/rust#158235 (Store `DefId` instead of `EiiDecl` in `EiiImplResolution::Known`)
 - rust-lang/rust#158723 (Support EII on Windows MSVC)
 - rust-lang/rust#158993 (rerun in original typing mode if we meet any opaques in post analysis)
 - rust-lang/rust#159160 (Make `HasTokens` a sub-trait of `HasAttrs`.)
 - rust-lang/rust#159183 (Introduce InstanceKind::LlvmIntrinsic)
 - rust-lang/rust#159251 (Bump rustc-perf submodule)
 - rust-lang/rust#155013 (Suggest the `[const] Destruct` bound for type parameters in const functions when missing)
 - rust-lang/rust#159155 (unstable book: Document `diagnostic_on_unknown` feature)
 - rust-lang/rust#159235 (Add regression test for rust-lang/rust#95719)
 - rust-lang/rust#159243 (inline Once wait and wait_force)
 - rust-lang/rust#159255 (Replace shortened type with `_` instead of `...` as placeholder)
 - rust-lang/rust#159259 (Add regression test for rust-lang/rust#144033)
 - rust-lang/rust#159265 (bootstrap: skip intrinsic-test when rustfmt is unavailable)
 - rust-lang/rust#159269 (disable range-len-try-from.rs on s390x)
 - rust-lang/rust#159272 (slice: make swap delegate to swap_unchecked)
 - rust-lang/rust#159274 (bootstrap: Rename `std_crates_for_run_make` to `std_crates_for_make_run`)
 - rust-lang/rust#159275 (remove obsolete comment)
 - rust-lang/rust#159277 (Construct `tokens` for attrs made by `mk_attr_word` and other variants)
 - rust-lang/rust#159283 (Remove obsolete verbose flag from deref/ref suggestions)
 - rust-lang/rust#159290 (rustc-dev-guide subtree update)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) F-extern_item_impls `#![feature(extern_item_impls)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

4 participants