Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

API: Use a ModItem as the root of a crate #306

Merged
merged 6 commits into from
Nov 4, 2023
Merged

API: Use a ModItem as the root of a crate #306

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4cf8706
API: Use a `ModItem` as the root of a crate
xFrednet Oct 31, 2023
3a082aa
API: Add `LintPass::check_crate` method
xFrednet Oct 31, 2023
3d622aa
Rustc: Fix ICE for compiler generated code
xFrednet Oct 31, 2023
dfbe27c
Doc: Update changelog
xFrednet Oct 31, 2023
9b08562
Rustc: Address PR Review
xFrednet Nov 2, 2023
efdb8bc
API: Update debug text for `SpanSource::Builtin`
xFrednet Nov 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions marker_api/src/span.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ impl<'ast> std::fmt::Debug for Span<'ast> {
fmt_pos(file.try_to_file_pos(self.end))
),
SpanSource::Macro(expn) => format!("[Inside Macro] {:#?}", expn.call_site()),
SpanSource::Buildin(_) => "[From Prelude]".to_string(),
SpanSource::Builtin(_) => "[From Prelude]".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If lints can't observe the prelude, then we may go without the "Builtin" span source? Or is there any other way this "Builtin" span source can be created? The test that was added in this PR no longer contains such a span source. If there is any other way it could be created, it would be good to have a test that showcases how it can be constructed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filter mechanism added in this PR should filter out all spans with a builtin source. However, I'm not 100% sure if that really removes every trace. Before this new SpanSource Marker would ICE when requesting these spans. I'm in favor of keeping it in the API as it will prevent potential ICEs. If we're starting to look at the stabilization of Marker, we can review if this value ever came up again.

I can imagine that also some unstable features might fall back on this source, during initial development. This is just a guess, though.

I'll change the text of this function to say "[Builtin]" instead.

};
f.debug_struct(&name).finish()
}
Expand Down Expand Up @@ -421,7 +421,7 @@ impl<'ast> Span<'ast> {
pub enum SpanSource<'ast> {
File(&'ast FileInfo<'ast>),
Macro(&'ast ExpnInfo<'ast>),
Buildin(&'ast BuildinInfo<'ast>),
Builtin(&'ast BuiltinInfo<'ast>),
}

#[repr(C)]
Expand Down Expand Up @@ -514,7 +514,7 @@ impl<'ast> FilePos<'ast> {
#[repr(C)]
#[derive(Debug, Clone, Copy)]
#[cfg_attr(feature = "driver-api", derive(Default))]
pub struct BuildinInfo<'ast> {
pub struct BuiltinInfo<'ast> {
/// The lifetime is not needed right now, but I want to have it, to potentualy
/// add more behavior to this struct.
_lifetime: PhantomData<&'ast ()>,
Expand Down
33 changes: 14 additions & 19 deletions marker_rustc_driver/src/conversion/marker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ struct MarkerConverterInner<'ast, 'tcx> {
variants: RefCell<FxHashMap<VariantId, &'ast EnumVariant<'ast>>>,

// Cached/Dummy values
buildin_span_source: &'ast marker_api::span::BuildinInfo<'ast>,
builtin_span_source: &'ast marker_api::span::BuiltinInfo<'ast>,
num_symbols: RefCell<FxHashMap<u32, SymbolId>>,

/// Lang-items are weird, and if I'm being honest, I'm uncertain that I
Expand Down Expand Up @@ -226,7 +226,7 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
stmts: RefCell::default(),
fields: RefCell::default(),
variants: RefCell::default(),
buildin_span_source: storage.alloc(marker_api::span::BuildinInfo::default()),
builtin_span_source: storage.alloc(marker_api::span::BuiltinInfo::default()),
num_symbols: RefCell::default(),
lang_item_map: RefCell::default(),
rustc_body: RefCell::default(),
Expand Down Expand Up @@ -308,23 +308,18 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
#[must_use]
fn local_crate(&self) -> &'ast Crate<'ast> {
if let Some(krate) = self.krate.get() {
return krate;
};

let krate = self.alloc(
Crate::builder()
.id(self.to_crate_id(hir::def_id::LOCAL_CRATE))
.root_mod(self.local_crate_mod())
.build(),
);
self.krate
.set(krate)
.expect("this can't be filled, since we checked earlier and this stage is single threaded");

let root_mod = krate.root_mod();
self.items.borrow_mut().insert(root_mod.id(), ItemKind::Mod(root_mod));
krate
self.krate.get_or_init(|| {
let krate = self.alloc(
Crate::builder()
.id(self.to_crate_id(hir::def_id::LOCAL_CRATE))
.root_mod(self.local_crate_mod())
.build(),
);

let root_mod = krate.root_mod();
self.items.borrow_mut().insert(root_mod.id(), ItemKind::Mod(root_mod));
krate
})
}

fn local_crate_mod(&self) -> ModItem<'ast> {
Expand Down
10 changes: 10 additions & 0 deletions marker_rustc_driver/src/conversion/marker/ast/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
return Some(*item);
}

if self.is_compiler_generated(rustc_item.span) {
return None;
}

let ident = self.to_ident(rustc_item.ident);
let data = CommonItemData::new(id, self.to_span_id(rustc_item.span), ident);
let item =
Expand Down Expand Up @@ -164,6 +168,12 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
Some(item)
}

fn is_compiler_generated(&self, span: rustc_span::Span) -> bool {
let ctxt = span.ctxt();

!ctxt.is_root() && matches!(ctxt.outer_expn_data().kind, rustc_span::ExpnKind::AstPass(_))
}

fn to_fn_item(
&self,
data: CommonItemData<'ast>,
Expand Down
4 changes: 2 additions & 2 deletions marker_rustc_driver/src/conversion/marker/span.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
let expn_data = ctxt.outer_expn_data();
return match expn_data.kind {
rustc_span::ExpnKind::Macro(_, _) => SpanSource::Macro(self.alloc(self.to_expn_info(&expn_data))),
rustc_span::ExpnKind::AstPass(_) => SpanSource::Buildin(self.buildin_span_source),
rustc_span::ExpnKind::Desugaring(_) => unreachable!("desugaring spans should never be crated"),
rustc_span::ExpnKind::AstPass(_) => SpanSource::Builtin(self.builtin_span_source),
rustc_span::ExpnKind::Desugaring(_) => unreachable!("desugaring spans should never be created"),
rustc_span::ExpnKind::Root => unreachable!("checked above"),
};
}
Expand Down
59 changes: 0 additions & 59 deletions marker_uilints/tests/ui/item/print_me_root_module.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,65 +12,6 @@ warning: printing item
},
},
items: [
Use(
UseItem {
data: CommonItemData {
id: ItemId(..),
span: SpanId(..),
vis: Visibility {{ /* WIP: See rust-marker/marker#26 */}},
ident: Ident {
name: "",
span: $DIR/print_me_root_module.rs:1:1 - 1:1,
},
},
use_path: AstPath {
segments: [
AstPathSegment {
ident: Ident {
name: "std",
span: [From Prelude],
},
generics: GenericArgs {
args: [],
},
},
AstPathSegment {
ident: Ident {
name: "prelude",
span: [From Prelude],
},
generics: GenericArgs {
args: [],
},
},
AstPathSegment {
ident: Ident {
name: "rust_2021",
span: [From Prelude],
},
generics: GenericArgs {
args: [],
},
},
],
},
use_kind: Glob,
},
),
ExternCrate(
ExternCrateItem {
data: CommonItemData {
id: ItemId(..),
span: SpanId(..),
vis: Visibility {{ /* WIP: See rust-marker/marker#26 */}},
ident: Ident {
name: "std",
span: [From Prelude],
},
},
crate_name: SymbolId(..),
},
),
Fn(
FnItem {
data: CommonItemData {
Expand Down
Loading