Auto merge of #142267 - workingjubilee:debug-assert-less-in-ast-lowering, r=<try>

[EXPERIMENTAL] assert in release builds 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 simple integer equalities. I am curious if the latter introduce a serious problem if we simply *do* them.

r? `@ghost`

Author: rust-bors[bot]

compiler/rustc_ast_lowering/src/index.rs

@@ -73,9 +73,9 @@ pub(super) fn index_hir<'hir>(
 impl<'a, 'hir> NodeCollector<'a, 'hir> {
     #[instrument(level = "debug", skip(self))]
     fn insert(&mut self, span: Span, hir_id: HirId, node: Node<'hir>) {
-        debug_assert_eq!(self.owner, hir_id.owner);
-        debug_assert_ne!(hir_id.local_id.as_u32(), 0);
-        debug_assert_ne!(hir_id.local_id, self.parent_node);
+        assert_eq!(self.owner, hir_id.owner);
+        assert_ne!(hir_id.local_id.as_u32(), 0);
+        assert_ne!(hir_id.local_id, self.parent_node);
 
         // Make sure that the DepNode of some node coincides with the HirId
         // owner of that node.
@@ -110,7 +110,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
     }
 
     fn with_parent<F: FnOnce(&mut Self)>(&mut self, parent_node_id: HirId, f: F) {
-        debug_assert_eq!(parent_node_id.owner, self.owner);
+        assert_eq!(parent_node_id.owner, self.owner);
         let parent_node = self.parent_node;
         self.parent_node = parent_node_id.local_id;
         f(self);
@@ -147,7 +147,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
     }
 
     fn visit_nested_body(&mut self, id: BodyId) {
-        debug_assert_eq!(id.hir_id.owner, self.owner);
+        assert_eq!(id.hir_id.owner, self.owner);
         let body = self.bodies[&id.hir_id.local_id];
         self.visit_body(body);
     }
@@ -162,7 +162,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
 
     #[instrument(level = "debug", skip(self))]
     fn visit_item(&mut self, i: &'hir Item<'hir>) {
-        debug_assert_eq!(i.owner_id, self.owner);
+        assert_eq!(i.owner_id, self.owner);
         self.with_parent(i.hir_id(), |this| {
             if let ItemKind::Struct(_, _, struct_def) = &i.kind {
                 // If this is a tuple or unit-like struct, register the constructor.
@@ -176,7 +176,7 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
 
     #[instrument(level = "debug", skip(self))]
     fn visit_foreign_item(&mut self, fi: &'hir ForeignItem<'hir>) {
-        debug_assert_eq!(fi.owner_id, self.owner);
+        assert_eq!(fi.owner_id, self.owner);
         self.with_parent(fi.hir_id(), |this| {
             intravisit::walk_foreign_item(this, fi);
         });
@@ -195,15 +195,15 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
 
     #[instrument(level = "debug", skip(self))]
     fn visit_trait_item(&mut self, ti: &'hir TraitItem<'hir>) {
-        debug_assert_eq!(ti.owner_id, self.owner);
+        assert_eq!(ti.owner_id, self.owner);
         self.with_parent(ti.hir_id(), |this| {
             intravisit::walk_trait_item(this, ti);
         });
     }
 
     #[instrument(level = "debug", skip(self))]
     fn visit_impl_item(&mut self, ii: &'hir ImplItem<'hir>) {
-        debug_assert_eq!(ii.owner_id, self.owner);
+        assert_eq!(ii.owner_id, self.owner);
         self.with_parent(ii.hir_id(), |this| {
             intravisit::walk_impl_item(this, ii);
         });

compiler/rustc_ast_lowering/src/item.rs

@@ -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

compiler/rustc_ast_lowering/src/lib.rs

@@ -503,7 +503,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: {:?}",
@@ -586,10 +586,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;
@@ -891,7 +891,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         } else {
             let lowered_attrs = self.lower_attrs_vec(attrs, self.lower_span(target_span));
 
-            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,
@@ -915,10 +915,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);
         }
     }
@@ -1397,7 +1397,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()
@@ -1805,16 +1805,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,
@@ -2244,7 +2244,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 {