Open Veetaha opened 1 year ago
The problem occurs because the code tries to look up the definition of the item in HIR, but that item comes from a foreign crate.
When the TraitRef
's ItemId
is created here https://github.com/rust-marker/marker/blob/73835de0a82ebd33b959df200f66e5a3d7bf007a/marker_rustc_driver/src/conversion/marker/common.rs#L414
The type of the ID that is transmuted into ItemId
is rustc_span::def_id::DefId
. But when the ctx.ast().item()
is called the ItemId
is converted into rustc_hir::hir::ItemId
. You may notice that the method in the snippet below doesn't use the layout.krate
field at all. I guess it assumes that this method is always called with local items (of the current crate).
https://github.com/rust-marker/marker/blob/73835de0a82ebd33b959df200f66e5a3d7bf007a/marker_rustc_driver/src/conversion/rustc/common.rs#L73-L82
So this results in an invalid rustc_hir::hir::ItemId
. You'll get a panic even if you try to debug-print this ID.
I'm not sure what the best solution here should be. Maybe the to_item_id(ItemId) -> hir::ItemId
method should check if layout.krate
is not a local crate. I suppose there is a special sentinel value for krate
that identifies it as the current crate. Then if the given item is not from the current crate it should return None
.
But how could users query the info about which trait is implemented? Maybe if there was at least the path to the implemented trait in the TraitRef
that would allow to check the syntactic path to the trait.
Ahh, I think the actual problem is the type. TraitRef
should hold a TyDefId
instead, and Marker should probably have a function to convert the TyDefId
into an ItemId
if the item is available.
Thinking about it, this will not resolve all issues though :thinking: Users can still retrieve ItemId
s when AstPath
s are resolved. So this case definitely need to be handled on Marker's side.
Rustc splits these IDs into a general DefId
which can be used across crate boundaries, but is not guaranteed to have an ast item, and a LocalDefId
which is only defined for items from the current crate, where an ast representation exists. I wonder if it would make sense to adapt a similar model :thinking:
Summary
I've been trying to implement a lint that prohibits implementing
Into/TryInto
traits directly and I've run into multiple issues with it. This is one of them. However, I suppose even if this is fixedctx.ast().item()
will returnNone
because the trait is outside of the compiled crate? Then how could I check which trait theimpl
item tries to implement?Btw. if I output the impl item of the
Into
trait impl specified in the reproducer below I get this. You can see that there is no implemented trait identifier anywhere in the data. This is bummerDetails
```rust impl_ = ImplItem { data: CommonItemData { id: ItemId(..), span: SpanId(..), vis: Visibility {{ /* WIP: See rust-marker/marker#26 */}}, ident: Ident { name: "", span: crates/scratch/src/main.rs:1:1 - 1:1, }, }, is_unsafe: false, is_negated: true, trait_ref: Some( TraitRef { item_id: ItemId(..), generics: GenericArgs { args: [ Ty( TyArg { ty: Tuple( TupleTy { data: CommonSynTyData { _lifetime: PhantomData<&()>, span: SpanId(..), }, types: [], }, ), }, ), ], }, }, ), generics: GenericParams { params: [], clauses: [], }, ty: Path( PathTy { data: CommonSynTyData { _lifetime: PhantomData<&()>, span: SpanId(..), }, path: AstQPath { self_ty: None, path_ty: None, path: AstPath { segments: [ AstPathSegment { ident: Ident { name: "Foo", span: crates/scratch/src/main.rs:3:19 - 3:22, }, generics: GenericArgs { args: [], }, }, ], }, target: Item( ItemId(..), ), }, }, ), items: [ Fn( FnItem { data: CommonItemData { id: ItemId(..), span: SpanId(..), vis: Visibility {{ /* WIP: See rust-marker/marker#26 */}}, ident: Ident { name: "into", span: crates/scratch/src/main.rs:4:8 - 4:12, }, }, generics: GenericParams { params: [], clauses: [], }, constness: NotConst, syncness: Sync, safety: Safe, is_extern: false, has_self: true, abi: Default, params: [ FnParam { span: SpanId(..), pat: Ident( IdentPat { data: CommonPatData { _lifetime: PhantomData<&()>, span: SpanId(..), }, name: SymbolId(..), var: VarId(..), mutability: Unmut, is_ref: false, binding_pat: None, }, ), ty: Path( PathTy { data: CommonSynTyData { _lifetime: PhantomData<&()>, span: SpanId(..), }, path: AstQPath { self_ty: None, path_ty: None, path: AstPath { segments: [ AstPathSegment { ident: Ident { name: "Self", span: crates/scratch/src/main.rs:1:1 - 1:1, }, generics: GenericArgs { args: [], }, }, ], }, target: SelfTy( ItemId(..), ), }, }, ), }, ], return_ty: None, body_id: Some( BodyId(..), ), }, .., ), ], } ```Reproducer
[derive(Default)]
struct MyLintPass {} marker_api::export_lint_pass!(MyLintPass);
marker_api::declare_lint! { /// # What it does /// Breaks marker. MY_LINT, Deny, }
impl LintPass for MyLintPass { fn info(&self) -> LintPassInfo { LintPassInfoBuilder::new(Box::new([MY_LINT])).build() }
}
cargo marker
Version
Logs