user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Rollup merge of #142267 - workingjubilee:debug-assert-less-in-ast-lowering, r=oli-obk

Browse source 
assert more in release in `rustc_ast_lowering`

My understanding of the compiler's architecture is that in the `ast_lowering` crate, we are constructing the HIR as a one-time thing per crate. This is after tokenizing, parsing, resolution, expansion, possible reparsing, reresolution, reexpansion, and so on. In other words, there are many reasons that perf-focused PRs spend a lot of time touching `rustc_parse`, `rustc_expand`, `rustc_ast`, and then `rustc_hir` and "onwards", but `ast_lowering` is a little bit of an odd duck.

In this crate, we have a number of debug assertions. Some are clearly expensive checks that seem like they are prohibitive to run in actual optimized compiler builds, but then there are a number that are simple asserts on integer equalities, `is_empty`, or the like. I believe we should do some of them even in release builds, because the correctness gain is worth the performance cost: almost zero.
This commit is contained in:
Matthias Krüger 2025-06-13 05:19:16 +02:00 • committed by GitHub
commit 6a09b67bdc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 18 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

10
compiler/rustc_ast_lowering/src/item.rs
View file

@ -63,7 +63,7 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> {
for (def_id, info) in lctx.children {
let owner = self.owners.ensure_contains_elem(def_id, || hir::MaybeOwner::Phantom);
debug_assert!(
assert!(
matches!(owner, hir::MaybeOwner::Phantom),
"duplicate copy of {def_id:?} in lctx.children"
);
@ -78,7 +78,7 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> {
match node {
AstOwner::NonOwner => {}
AstOwner::Crate(c) => {
debug_assert_eq!(self.resolver.node_id_to_def_id[&CRATE_NODE_ID], CRATE_DEF_ID);
assert_eq!(self.resolver.node_id_to_def_id[&CRATE_NODE_ID], CRATE_DEF_ID);
self.with_lctx(CRATE_NODE_ID, |lctx| {
let module = lctx.lower_mod(&c.items, &c.spans);
// FIXME(jdonszelman): is dummy span ever a problem here?
@ -1160,7 +1160,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
) -> hir::BodyId {
let body = hir::Body { params, value: self.arena.alloc(value) };
let id = body.id();
debug_assert_eq!(id.hir_id.owner, self.current_hir_id_owner);
assert_eq!(id.hir_id.owner, self.current_hir_id_owner);
self.bodies.push((id.hir_id.local_id, self.arena.alloc(body)));
id
}
@ -1673,8 +1673,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
itctx: ImplTraitContext,
f: impl FnOnce(&mut Self) -> T,
) -> (&'hir hir::Generics<'hir>, T) {
debug_assert!(self.impl_trait_defs.is_empty());
debug_assert!(self.impl_trait_bounds.is_empty());
assert!(self.impl_trait_defs.is_empty());
assert!(self.impl_trait_bounds.is_empty());
// Error if `?Trait` bounds in where clauses don't refer directly to type parameters.
// Note: we used to clone these bounds directly onto the type parameter (and avoid lowering

26
compiler/rustc_ast_lowering/src/lib.rs
View file

@ -523,7 +523,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
span: Span,
) -> LocalDefId {
let parent = self.current_hir_id_owner.def_id;
debug_assert_ne!(node_id, ast::DUMMY_NODE_ID);
assert_ne!(node_id, ast::DUMMY_NODE_ID);
assert!(
self.opt_local_def_id(node_id).is_none(),
"adding a def'n for node-id {:?} and def kind {:?} but a previous def'n exists: {:?}",
@ -607,10 +607,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}
let item = f(self);
debug_assert_eq!(owner_id, item.def_id());
assert_eq!(owner_id, item.def_id());
// `f` should have consumed all the elements in these vectors when constructing `item`.
debug_assert!(self.impl_trait_defs.is_empty());
debug_assert!(self.impl_trait_bounds.is_empty());
assert!(self.impl_trait_defs.is_empty());
assert!(self.impl_trait_bounds.is_empty());
let info = self.make_owner_info(item);
self.attrs = current_attrs;
@ -918,7 +918,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
} else {
let lowered_attrs = self.lower_attrs_vec(attrs, self.lower_span(target_span), id);
debug_assert_eq!(id.owner, self.current_hir_id_owner);
assert_eq!(id.owner, self.current_hir_id_owner);
let ret = self.arena.alloc_from_iter(lowered_attrs);
// this is possible if an item contained syntactical attribute,
@ -956,10 +956,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}
fn alias_attrs(&mut self, id: HirId, target_id: HirId) {
debug_assert_eq!(id.owner, self.current_hir_id_owner);
debug_assert_eq!(target_id.owner, self.current_hir_id_owner);
assert_eq!(id.owner, self.current_hir_id_owner);
assert_eq!(target_id.owner, self.current_hir_id_owner);
if let Some(&a) = self.attrs.get(&target_id.local_id) {
debug_assert!(!a.is_empty());
assert!(!a.is_empty());
self.attrs.insert(id.local_id, a);
}
}
@ -1438,7 +1438,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let id = if let Some(LifetimeRes::ElidedAnchor { start, end }) =
self.resolver.get_lifetime_res(t.id)
{
debug_assert_eq!(start.plus(1), end);
assert_eq!(start.plus(1), end);
start
} else {
self.next_node_id()
@ -1846,16 +1846,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let res = match res {
LifetimeRes::Param { param, .. } => hir::LifetimeKind::Param(param),
LifetimeRes::Fresh { param, .. } => {
debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
assert_eq!(ident.name, kw::UnderscoreLifetime);
let param = self.local_def_id(param);
hir::LifetimeKind::Param(param)
}
LifetimeRes::Infer => {
debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
assert_eq!(ident.name, kw::UnderscoreLifetime);
hir::LifetimeKind::Infer
}
LifetimeRes::Static { .. } => {
debug_assert!(matches!(ident.name, kw::StaticLifetime | kw::UnderscoreLifetime));
assert!(matches!(ident.name, kw::StaticLifetime | kw::UnderscoreLifetime));
hir::LifetimeKind::Static
}
LifetimeRes::Error => hir::LifetimeKind::Error,
@ -2285,7 +2285,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
) -> hir::Stmt<'hir> {
let hir_id = self.next_id();
if let Some(a) = attrs {
debug_assert!(!a.is_empty());
assert!(!a.is_empty());
self.attrs.insert(hir_id.local_id, a);
}
let local = hir::LetStmt {