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 |
|
|
ddb4f2a to
499975b
Compare
This comment has been minimized.
This comment has been minimized.
499975b to
c65973d
Compare
|
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. |
| ) { | ||
| EiiImplResolution::Known(decl) | ||
| && let Some(foreign_item_did) = | ||
| self.lower_path_simple_eii(*node_id, &target.foreign_item) |
There was a problem hiding this comment.
According to the doc comment of the known_eii_macro_resolution field on EiiImpl, it is supposed to bypass name resolution to prevent bugs like #149981. This PR effectively changes it back to resolving paths (target.foreign_item is a path). If bypassing name resolution is no longer necessary, maybe known_eii_macro_resolution could be removed entirely? Or did I misunderstand the exact problem this is supposed to workaround?
Edit: Maybe what actually matters is getting the path to the foreign item rather than to the macro here:
|
You should be able to simplify this a bit further: Author: bjorn3 <17426603+bjorn3@users.noreply.github.com>
Date: Wed Jul 8 11:29:54 2026 +0200
Replace EiiDecl in known_eii_macro_resolution with Path
Only the Path is actually looked at now.
diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs
index bd5ce5d18b8..9b881688a6b 100644
--- a/compiler/rustc_ast/src/ast.rs
+++ b/compiler/rustc_ast/src/ast.rs
@@ -3924,7 +3924,7 @@ pub struct EiiImpl {
///
/// This field is that shortcut: we prefill the extern target to skip a name resolution step,
/// making sure it never fails. It'd be awful UX if we fail name resolution in code invisible to the user.
- pub known_eii_macro_resolution: Option<EiiDecl>,
+ pub known_eii_macro_resolution: Option<Path>,
pub impl_safety: Safety,
pub span: Span,
pub inner_span: Span,
diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs
index 8b129576308..8d4a6f10476 100644
--- a/compiler/rustc_ast_lowering/src/item.rs
+++ b/compiler/rustc_ast_lowering/src/item.rs
@@ -141,8 +141,7 @@ fn lower_eii_impl(
}: &EiiImpl,
) -> hir::attrs::EiiImpl {
let resolution = if let Some(target) = known_eii_macro_resolution
- && let Some(foreign_item_did) =
- self.lower_path_simple_eii(*node_id, &target.foreign_item)
+ && let Some(foreign_item_did) = self.lower_path_simple_eii(*node_id, target)
{
EiiImplResolution::Known(foreign_item_did)
} else if let Some(macro_did) = self.lower_path_simple_eii(*node_id, eii_macro_path) {
diff --git a/compiler/rustc_builtin_macros/src/eii.rs b/compiler/rustc_builtin_macros/src/eii.rs
index 68d5278c85e..66879d5daf7 100644
--- a/compiler/rustc_builtin_macros/src/eii.rs
+++ b/compiler/rustc_builtin_macros/src/eii.rs
@@ -302,15 +302,12 @@ fn generate_default_impl(
},
span: eii_attr_span,
is_default: true,
- known_eii_macro_resolution: Some(ast::EiiDecl {
- foreign_item: ecx.path(
- foreign_item_name.span,
- // prefix self to explicitly escape the const block generated below
- // NOTE: this is why EIIs can't be used on statements
- vec![Ident::from_str_and_span("self", foreign_item_name.span), foreign_item_name],
- ),
- impl_unsafe,
- }),
+ known_eii_macro_resolution: Some(ecx.path(
+ foreign_item_name.span,
+ // prefix self to explicitly escape the const block generated below
+ // NOTE: this is why EIIs can't be used on statements
+ vec![Ident::from_str_and_span("self", foreign_item_name.span), foreign_item_name],
+ )),
};
let mut item_kind = item_kind.clone();
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
index c94f2f3ffb8..6f206aadf0a 100644
--- a/compiler/rustc_resolve/src/late.rs
+++ b/compiler/rustc_resolve/src/late.rs
@@ -5609,12 +5609,7 @@ fn resolve_eii(&mut self, eii_impls: &[EiiImpl]) {
// See docs on the `known_eii_macro_resolution` field:
// if we already know the resolution statically, don't bother resolving it.
if let Some(target) = known_eii_macro_resolution {
- self.smart_resolve_path(
- *node_id,
- &None,
- &target.foreign_item,
- PathSource::ExternItemImpl,
- );
+ self.smart_resolve_path(*node_id, &None, target, PathSource::ExternItemImpl);
} else {
self.smart_resolve_path(*node_id, &None, &eii_macro_path, PathSource::Macro);
} |
|
r? bjorn3 |
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