marker
marker copied to clipboard
rust-marker
[Bug]: Maker panics if the trait from `std` is requested via `ctx.ast().item()`
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
- 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());
}
}
- Take this code that is linted
struct Foo;
impl Into<()> for Foo {
fn into(self) {}
}
fn main() {}
- 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
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 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 :thinking:
