[Bug]: Maker panics if the trait from std is requested via ctx.ast().item()

[Veetaha]

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 fixed ctx.ast().item() will return None because the trait is outside of the compiled crate? Then how could I check which trait the impl 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 bummer

Details

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

1. Take this lint crate code

use marker_api::prelude::*;
use marker_api::{LintPass, LintPassInfo, LintPassInfoBuilder};

#[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()
    }

    fn check_item<'ast>(&mut self, ctx: &'ast MarkerContext<'ast>, item: ast::ItemKind<'ast>) {
        let ItemKind::Impl(impl_) = item else { return; };

        if item.span().is_from_expansion() {
            return;
        }

       ctx.ast().item(impl_.trait_ref().unwrap().trait_id());
    }
}

2. Take this code that is linted

struct Foo;

impl Into<()> for Foo {
    fn into(self) {}
}

fn main() {}

3. Run cargo marker

Version

cargo-marker 0.3.0

Logs

Marker compiling lints
   Compiling veetaha-lints v0.1.0 (/home/veetaha/sandbox/rust/crates/veetaha-lints)
    Finished release [optimized] target(s) in 0.23s

      Marker linting
    Checking scratch v0.1.0 (/home/veetaha/sandbox/rust/crates/scratch)
thread 'rustc' panicked at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/compiler/rustc_hir/src/definitions.rs:78:40:
index out of bounds: the len is 8 but the index is 2520
stack backtrace:
   0:     0x7f909f62933c - std::backtrace_rs::backtrace::libunwind::trace::h910709f6ac8bdc9f
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x7f909f62933c - std::backtrace_rs::backtrace::trace_unsynchronized::h66c1b9aae6144841
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x7f909f62933c - std::sys_common::backtrace::_print_fmt::h225f965e4a6dd062
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/sys_common/backtrace.rs:67:5
   3:     0x7f909f62933c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h4f4e7c60db66a770
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x7f909f68f4cc - core::fmt::rt::Argument::fmt::h87caa0a583b068c8
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/core/src/fmt/rt.rs:138:9
   5:     0x7f909f68f4cc - core::fmt::write::h3b600a18a82b19f5
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/core/src/fmt/mod.rs:1094:21
   6:     0x7f909f61bd6e - std::io::Write::write_fmt::h02208d7956f2653b
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/io/mod.rs:1714:15
   7:     0x7f909f629124 - std::sys_common::backtrace::_print::h3aea4dd9a94d323a
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x7f909f629124 - std::sys_common::backtrace::print::hbf8b71196d492872
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x7f909f62c21a - std::panicking::panic_hook_with_disk_dump::{{closure}}::h6a8880f6e8234529
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/panicking.rs:278:22
  10:     0x7f909f62bf07 - std::panicking::panic_hook_with_disk_dump::h8ea3bdb613c8c8a5
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/panicking.rs:312:9
  11:     0x7f90a27eb969 - <rustc_driver_impl[8d3f86e83538be5d]::install_ice_hook::{closure#0} as core[328660574c6e17ab]::ops::function::FnOnce<(&core[328660574c6e17ab]::panic::panic_info::PanicInfo,)>>::call_once::{shim:vtable#0}
  12:     0x7f909f62cac0 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h21e710b40303a14c
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/alloc/src/boxed.rs:2021:9
  13:     0x7f909f62cac0 - std::panicking::rust_panic_with_hook::h006994873154b18b
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/panicking.rs:733:13
  14:     0x7f909f62c847 - std::panicking::begin_panic_handler::{{closure}}::hcca9a8f2a8a2254b
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/panicking.rs:621:13
  15:     0x7f909f629866 - std::sys_common::backtrace::__rust_end_short_backtrace::ha419d4c6c0af0a51
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/sys_common/backtrace.rs:170:18
  16:     0x7f909f62c592 - rust_begin_unwind
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/panicking.rs:617:5
  17:     0x7f909f68b8d3 - core::panicking::panic_fmt::h1243a42fc81a0e60
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/core/src/panicking.rs:67:14
  18:     0x7f909f68ba29 - core::panicking::panic_bounds_check::hb8c9004610e31c8a
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/core/src/panicking.rs:162:5
  19:     0x7f90a1b9975e - rustc_query_system[c8323b2e3373b6b6]::query::plumbing::try_execute_query::<rustc_query_impl[54fb8ce76ffe27d7]::DynamicConfig<rustc_query_system[c8323b2e3373b6b6]::query::caches::VecCache<rustc_hir[dd5c45a8227164a5]::hir_id::OwnerId, rustc_middle[4d497bf664395237]::query::erase::Erased<[u8; 16usize]>>, false, false, false>, rustc_query_impl[54fb8ce76ffe27d7]::plumbing::QueryCtxt, true>
  20:     0x7f90a1b980e1 - rustc_query_impl[54fb8ce76ffe27d7]::query_impl::hir_owner::get_query_incr::__rust_end_short_backtrace
  21:     0x7f90a0ed27d5 - <rustc_middle[4d497bf664395237]::hir::map::Map>::item
  22:     0x55a064b28b3b - marker_rustc_driver::context::map::<impl marker_adapter::context::map::AstMapDriver for marker_rustc_driver::context::RustcContext>::item::he2ed795182a3b0b7
  23:     0x55a064b44321 - marker_adapter::context::map::item::hfa438a98111811a2
  24:     0x7f9094f872d3 - marker_api::context::map::AstMap::item::h46b77d5c0bba56f0
  25:     0x7f9094f87033 - veetaha_lints::__marker_todo::marker_lint_crate_bindings::check_item::hc3fd13d42da5365d
  26:     0x55a064b449fe - marker_utils::visitor::traverse_item::h6e6864935b44afca
  27:     0x55a064b3e360 - <marker_rustc_driver::lint_pass::RustcLintPass as rustc_lint::passes::LateLintPass>::check_crate::hc24059d54fd29dcb
  28:     0x7f90a1a00cc7 - <rustc_session[b6932d4e5e79a3c2]::session::Session>::time::<(), rustc_lint[e0df64bfbaf459ee]::late::check_crate::{closure#0}::{closure#0}>
  29:     0x7f90a19ff999 - <core[328660574c6e17ab]::panic::unwind_safe::AssertUnwindSafe<rustc_interface[5749929743a7739d]::passes::analysis::{closure#5}::{closure#1}::{closure#2}> as core[328660574c6e17ab]::ops::function::FnOnce<()>>::call_once
  30:     0x7f90a19ff70f - <core[328660574c6e17ab]::panic::unwind_safe::AssertUnwindSafe<rustc_interface[5749929743a7739d]::passes::analysis::{closure#5}::{closure#1}> as core[328660574c6e17ab]::ops::function::FnOnce<()>>::call_once
  31:     0x7f90a19ff138 - <rustc_session[b6932d4e5e79a3c2]::session::Session>::time::<(), rustc_interface[5749929743a7739d]::passes::analysis::{closure#5}>
  32:     0x7f90a19fceb7 - rustc_interface[5749929743a7739d]::passes::analysis
  33:     0x7f90a1d081fa - rustc_query_impl[54fb8ce76ffe27d7]::plumbing::__rust_begin_short_backtrace::<rustc_query_impl[54fb8ce76ffe27d7]::query_impl::analysis::dynamic_query::{closure#2}::{closure#0}, rustc_middle[4d497bf664395237]::query::erase::Erased<[u8; 1usize]>>
  34:     0x7f90a1d081e9 - <rustc_query_impl[54fb8ce76ffe27d7]::query_impl::analysis::dynamic_query::{closure#2} as core[328660574c6e17ab]::ops::function::FnOnce<(rustc_middle[4d497bf664395237]::ty::context::TyCtxt, ())>>::call_once
  35:     0x7f90a1fe9018 - rustc_query_system[c8323b2e3373b6b6]::query::plumbing::try_execute_query::<rustc_query_impl[54fb8ce76ffe27d7]::DynamicConfig<rustc_query_system[c8323b2e3373b6b6]::query::caches::SingleCache<rustc_middle[4d497bf664395237]::query::erase::Erased<[u8; 1usize]>>, false, false, false>, rustc_query_impl[54fb8ce76ffe27d7]::plumbing::QueryCtxt, true>
  36:     0x7f90a1fe8b74 - rustc_query_impl[54fb8ce76ffe27d7]::query_impl::analysis::get_query_incr::__rust_end_short_backtrace
  37:     0x7f90a1aa61e3 - <rustc_interface[5749929743a7739d]::queries::QueryResult<&rustc_middle[4d497bf664395237]::ty::context::GlobalCtxt>>::enter::<core[328660574c6e17ab]::result::Result<(), rustc_span[498346d9655021fc]::ErrorGuaranteed>, rustc_driver_impl[8d3f86e83538be5d]::run_compiler::{closure#1}::{closure#2}::{closure#6}>
  38:     0x7f90a1aa51ba - <rustc_interface[5749929743a7739d]::interface::Compiler>::enter::<rustc_driver_impl[8d3f86e83538be5d]::run_compiler::{closure#1}::{closure#2}, core[328660574c6e17ab]::result::Result<core[328660574c6e17ab]::option::Option<rustc_interface[5749929743a7739d]::queries::Linker>, rustc_span[498346d9655021fc]::ErrorGuaranteed>>
  39:     0x7f90a1aa24e8 - std[3de2780fbf87294b]::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface[5749929743a7739d]::util::run_in_thread_pool_with_globals<rustc_interface[5749929743a7739d]::interface::run_compiler<core[328660574c6e17ab]::result::Result<(), rustc_span[498346d9655021fc]::ErrorGuaranteed>, rustc_driver_impl[8d3f86e83538be5d]::run_compiler::{closure#1}>::{closure#0}, core[328660574c6e17ab]::result::Result<(), rustc_span[498346d9655021fc]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[328660574c6e17ab]::result::Result<(), rustc_span[498346d9655021fc]::ErrorGuaranteed>>
  40:     0x7f90a1aa1c75 - <<std[3de2780fbf87294b]::thread::Builder>::spawn_unchecked_<rustc_interface[5749929743a7739d]::util::run_in_thread_pool_with_globals<rustc_interface[5749929743a7739d]::interface::run_compiler<core[328660574c6e17ab]::result::Result<(), rustc_span[498346d9655021fc]::ErrorGuaranteed>, rustc_driver_impl[8d3f86e83538be5d]::run_compiler::{closure#1}>::{closure#0}, core[328660574c6e17ab]::result::Result<(), rustc_span[498346d9655021fc]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[328660574c6e17ab]::result::Result<(), rustc_span[498346d9655021fc]::ErrorGuaranteed>>::{closure#1} as core[328660574c6e17ab]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
  41:     0x7f909f637425 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h07273d00f835f9f4
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/alloc/src/boxed.rs:2007:9
  42:     0x7f909f637425 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hfd47bb1abc348520
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/alloc/src/boxed.rs:2007:9
  43:     0x7f909f637425 - std::sys::unix::thread::Thread::new::thread_start::h98e1ddafb85f3672
                               at /rustc/249595b7523fc07a99c1adee90b1947739ca0e5b/library/std/src/sys/unix/thread.rs:108:17
  44:     0x7f909f362ac3 - start_thread
                               at ./nptl/pthread_create.c:442:8
  45:     0x7f909f3f4a40 - clone3
                               at ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
  46:                0x0 - <unknown>

error: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-marker/marker/issues/new?template=panic.yml

note: please attach the file at `/home/veetaha/sandbox/rust/rustc-ice-2023-10-22T01:38:54.573452304Z-2179216.txt` to your bug report

note: compiler flags: --crate-type bin -C embed-bitcode=no -C debuginfo=2 -C linker=clang -C incremental=[REDACTED] -C link-arg=-fuse-ld=/home/veetaha/apps/bin/mold

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
thread panicked while processing panic. aborting.
error: could not compile `scratch` (bin "scratch")

Caused by:
  process didn't exit successfully: `/home/veetaha/.rustup/toolchains/nightly-2023-08-24-x86_64-unknown-linux-gnu/bin/marker_rustc_driver /home/veetaha/.rustup/toolchains/nightly-2023-08-24-x86_64-unknown-linux-gnu/bin/rustc --crate-name scratch --edition=2021 crates/scratch/src/main.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=134 --crate-type bin --emit=dep-info,metadata -C embed-bitcode=no -C debuginfo=2 -C metadata=6ca4360278f7b380 -C extra-filename=-6ca4360278f7b380 --out-dir /home/veetaha/sandbox/rust/target/debug/deps -C linker=clang -C incremental=/home/veetaha/sandbox/rust/target/debug/incremental -L dependency=/home/veetaha/sandbox/rust/target/debug/deps -C link-arg=-fuse-ld=/home/veetaha/apps/bin/mold` (signal: 6, SIGABRT: process abort signal)
  × linting finished with an error

[Veetaha]

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

marker/marker_rustc_driver/src/conversion/marker/common.rs

Line 414 in 73835de

self.to_item_id(rustc_id)

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).

marker/marker_rustc_driver/src/conversion/rustc/common.rs

Lines 73 to 82 in 73835de

	pub fn to_item_id(&self, api_id: ItemId) -> hir::ItemId {
	let layout = transmute_id!(ItemId as DefIdLayout = api_id);
	hir::ItemId {
	owner_id: hir::OwnerId {
	def_id: hir::def_id::LocalDefId {
	local_def_index: hir::def_id::DefIndex::from_u32(layout.index),
	},
	},
	}
	}

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.

[xFrednet]

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 🤔 Users can still retrieve ItemIds when AstPaths 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 🤔