Store DefId instead of EiiDecl in EiiImplResolution::Known#158235
Store DefId instead of EiiDecl in EiiImplResolution::Known#158235cezarbbb wants to merge 1 commit into
DefId instead of EiiDecl in EiiImplResolution::Known#158235Conversation
|
Some changes occurred in compiler/rustc_passes/src/check_attr.rs cc @jdonszelmann, @JonathanBrouwer Some changes occurred in compiler/rustc_hir/src/attrs |
|
|
| tcx.dcx().span_delayed_bug( | ||
| i.span, | ||
| "EII impl has Known resolution but can't find EiiDeclaration", | ||
| ); |
There was a problem hiding this comment.
How is this possible in error cases? If it also isn't expected to be possible in error cases this should be a bug!().
There was a problem hiding this comment.
It can't happen — Known is only produced by compiler-generated default impls where the foreign item path uses self::, so it always resolves to something in the current crate. Changed to bug!().
| // (this is ok since we generate codegen fn attrs in the local crate) | ||
| // if any of them is *not default* then don't emit the alias. | ||
| && tcx.externally_implementable_items(LOCAL_CRATE).get(&foreign_item).expect("at least one").1.iter().any(|(_, imp)| !imp.is_default) | ||
| && let Some((_, impls)) = tcx.externally_implementable_items(LOCAL_CRATE).get(&foreign_item) |
There was a problem hiding this comment.
Is there a case where it is expected to get None?
There was a problem hiding this comment.
No, it shouldn't return None — the impl we're processing is itself part of that map's entry. Changed to unwrap_or_else(|| bug!(...)).
ddb4f2a to
499975b
Compare
Fixes the
FIXME(eii)about removing the extern target from the encoded decl.Known(EiiDecl)serialized the sameEiiDeclN+1 times per entry (once as the declaration, once per Known impl). Changing toKnown(DefId)eliminates that redundancy — theEiiDeclis recoverable viatcx.externally_implementable_items()or a local lookup map, consistent with how Rust metadata usesDefIdas a reference elsewhere.I believe there are benefits in both performance and size(full
EiiDeclto singleDefId, and O(N) to O(1) lookup).Also fixes a soundness gap(I think?) in
check_attr.rswhere theKnownbranch skippedimpl_unsafechecking, and replaces an.expect()incodegen_attrs.rswitha let Somepattern.tracking issues: #125418
r? @jdonszelmann