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

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.

2 files changed, 18 insertions, 18 deletions

diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs
index d1a2ddbdb34..e2d291a536d 100644
--- a/compiler/rustc_ast_lowering/src/item.rs
+++ b/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
diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
index 249d1a99d72..4f0368bcb01 100644
--- a/compiler/rustc_ast_lowering/src/lib.rs
+++ b/compiler/rustc_ast_lowering/src/lib.rs
@@ -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 {