Auto merge of #52956 - ljedrz:debug_asserts, r=<try>

Use debug_assert! instead of assert! where possible

We are using a *lot* of `assert!`ions in the compiler. I propose using `debug_assert!`s where possible instead, for the following reasons:
1. `assert!`s are not free - they incur both compilation and runtime performance penalties
2. test builds should be run with debug assertions on; `debug_assert!`s should suffice to notify us when something is wrong
3. the compiler code (besides `core`/`std` etc.) is not user-facing, so we don't have to be prepared e.g. for invalid arguments coming from runtime input
4. regular `assert!`s panic during runtime, so it's not like we can allow such behavior anyway - we need to ensure it doesn't happen before we ship

I didn't alter tests (no runtime penalty), user-facing code (it requires a more thorough analysis; I might do it if this proposal passes) and a few modules I am unfamiliar with.

Author: bors

src/libarena/lib.rs

@@ -166,8 +166,8 @@ impl<T> TypedArena<T> {
     where
         T: Copy,
     {
-        assert!(mem::size_of::<T>() != 0);
-        assert!(slice.len() != 0);
+        debug_assert!(mem::size_of::<T>() != 0);
+        debug_assert!(slice.len() != 0);
 
         let available_capacity_bytes = self.end.get() as usize - self.ptr.get() as usize;
         let at_least_bytes = slice.len() * mem::size_of::<T>();
@@ -318,7 +318,7 @@ impl DroplessArena {
     fn align(&self, align: usize) {
         let final_address = ((self.ptr.get() as usize) + align - 1) & !(align - 1);
         self.ptr.set(final_address as *mut u8);
-        assert!(self.ptr <= self.end);
+        debug_assert!(self.ptr <= self.end);
     }
 
     #[inline(never)]
@@ -357,7 +357,7 @@ impl DroplessArena {
     #[inline]
     pub fn alloc_raw(&self, bytes: usize, align: usize) -> &mut [u8] {
         unsafe {
-            assert!(bytes != 0);
+            debug_assert!(bytes != 0);
 
             self.align(align);
 
@@ -377,7 +377,7 @@ impl DroplessArena {
 
     #[inline]
     pub fn alloc<T>(&self, object: T) -> &mut T {
-        assert!(!mem::needs_drop::<T>());
+        debug_assert!(!mem::needs_drop::<T>());
 
         let mem = self.alloc_raw(
             mem::size_of::<T>(),
@@ -402,9 +402,9 @@ impl DroplessArena {
     where
         T: Copy,
     {
-        assert!(!mem::needs_drop::<T>());
-        assert!(mem::size_of::<T>() != 0);
-        assert!(slice.len() != 0);
+        debug_assert!(!mem::needs_drop::<T>());
+        debug_assert!(mem::size_of::<T>() != 0);
+        debug_assert!(slice.len() != 0);
 
         let mem = self.alloc_raw(
             slice.len() * mem::size_of::<T>(),

src/libpanic_unwind/emcc.rs

@@ -30,7 +30,7 @@ pub fn payload() -> *mut u8 {
 }
 
 pub unsafe fn cleanup(ptr: *mut u8) -> Box<dyn Any + Send> {
-    assert!(!ptr.is_null());
+    debug_assert!(!ptr.is_null());
     let ex = ptr::read(ptr as *mut _);
     __cxa_free_exception(ptr as *mut _);
     ex

src/librustc/cfg/graphviz.rs

@@ -32,7 +32,7 @@ pub struct LabelledCFG<'a, 'tcx: 'a> {
 
 impl<'a, 'tcx> LabelledCFG<'a, 'tcx> {
     fn local_id_to_string(&self, local_id: hir::ItemLocalId) -> String {
-        assert!(self.cfg.owner_def_id.is_local());
+        debug_assert!(self.cfg.owner_def_id.is_local());
         let node_id = self.tcx.hir.hir_to_node_id(hir::HirId {
             owner: self.tcx.hir.def_index_to_hir_id(self.cfg.owner_def_id.index).owner,
             local_id

src/librustc/dep_graph/dep_node.rs

@@ -301,7 +301,7 @@ macro_rules! define_dep_nodes {
             pub fn from_def_path_hash(kind: DepKind,
                                       def_path_hash: DefPathHash)
                                       -> DepNode {
-                assert!(kind.can_reconstruct_query_key() && kind.has_params());
+                debug_assert!(kind.can_reconstruct_query_key() && kind.has_params());
                 DepNode {
                     kind,
                     hash: def_path_hash.0,
@@ -313,7 +313,7 @@ macro_rules! define_dep_nodes {
             /// does not require any parameters.
             #[inline]
             pub fn new_no_params(kind: DepKind) -> DepNode {
-                assert!(!kind.has_params());
+                debug_assert!(!kind.has_params());
                 DepNode {
                     kind,
                     hash: Fingerprint::ZERO,

src/librustc/hir/def_id.rs

@@ -39,7 +39,7 @@ newtype_index!(CrateNum
 
 impl CrateNum {
     pub fn new(x: usize) -> CrateNum {
-        assert!(x < (u32::MAX as usize));
+        debug_assert!(x < (u32::MAX as usize));
         CrateNum(x as u32)
     }
 
@@ -127,7 +127,7 @@ impl DefIndex {
     pub fn from_proc_macro_index(proc_macro_index: usize) -> DefIndex {
         let def_index = DefIndex::from_array_index(proc_macro_index,
                                                    DefIndexAddressSpace::High);
-        assert!(def_index != CRATE_DEF_INDEX);
+        debug_assert!(def_index != CRATE_DEF_INDEX);
         def_index
     }
 
@@ -222,7 +222,7 @@ pub struct LocalDefId(DefIndex);
 impl LocalDefId {
     #[inline]
     pub fn from_def_id(def_id: DefId) -> LocalDefId {
-        assert!(def_id.is_local());
+        debug_assert!(def_id.is_local());
         LocalDefId(def_id.index)
     }
 

src/librustc/hir/lowering.rs

@@ -654,8 +654,8 @@ impl<'a> LoweringContext<'a> {
     where
         F: FnOnce(&mut LoweringContext) -> (Vec<hir::GenericParam>, T),
     {
-        assert!(!self.is_collecting_in_band_lifetimes);
-        assert!(self.lifetimes_to_define.is_empty());
+        debug_assert!(!self.is_collecting_in_band_lifetimes);
+        debug_assert!(self.lifetimes_to_define.is_empty());
         let old_anonymous_lifetime_mode = self.anonymous_lifetime_mode;
 
         self.is_collecting_in_band_lifetimes = self.sess.features_untracked().in_band_lifetimes;
@@ -735,7 +735,7 @@ impl<'a> LoweringContext<'a> {
     /// When we have either an elided or `'_` lifetime in an impl
     /// header, we convert it to
     fn collect_fresh_in_band_lifetime(&mut self, span: Span) -> ParamName {
-        assert!(self.is_collecting_in_band_lifetimes);
+        debug_assert!(self.is_collecting_in_band_lifetimes);
         let index = self.lifetimes_to_define.len();
         let hir_name = ParamName::Fresh(index);
         self.lifetimes_to_define.push((span, hir_name));
@@ -1596,7 +1596,7 @@ impl<'a> LoweringContext<'a> {
                         if let Some(&n) = self.type_def_lifetime_params.get(&def_id) {
                             return n;
                         }
-                        assert!(!def_id.is_local());
+                        debug_assert!(!def_id.is_local());
                         let item_generics =
                             self.cstore.item_generics_cloned_untracked(def_id, self.sess);
                         let n = item_generics.own_counts().lifetimes;
@@ -4369,7 +4369,7 @@ impl<'a> LoweringContext<'a> {
                 has_value: has_value,
             },
             Defaultness::Final => {
-                assert!(has_value);
+                debug_assert!(has_value);
                 hir::Defaultness::Final
             }
         }

src/librustc/hir/map/definitions.rs

@@ -257,7 +257,7 @@ impl DefPath {
             debug!("DefPath::make: key={:?}", key);
             match key.disambiguated_data.data {
                 DefPathData::CrateRoot => {
-                    assert!(key.parent.is_none());
+                    debug_assert!(key.parent.is_none());
                     break;
                 }
                 _ => {
@@ -500,7 +500,7 @@ impl Definitions {
         let address_space = super::ITEM_LIKE_SPACE;
         let root_index = self.table.allocate(key, def_path_hash, address_space);
         assert_eq!(root_index, CRATE_DEF_INDEX);
-        assert!(self.def_index_to_node[address_space.index()].is_empty());
+        debug_assert!(self.def_index_to_node[address_space.index()].is_empty());
         self.def_index_to_node[address_space.index()].push(ast::CRATE_NODE_ID);
         self.node_to_def_index.insert(ast::CRATE_NODE_ID, root_index);
 
@@ -522,14 +522,15 @@ impl Definitions {
         debug!("create_def_with_parent(parent={:?}, node_id={:?}, data={:?})",
                parent, node_id, data);
 
-        assert!(!self.node_to_def_index.contains_key(&node_id),
-                "adding a def'n for node-id {:?} and data {:?} but a previous def'n exists: {:?}",
-                node_id,
-                data,
-                self.table.def_key(self.node_to_def_index[&node_id]));
+        debug_assert!(!self.node_to_def_index.contains_key(&node_id),
+                      "adding a def'n for node-id {:?} and data {:?} \
+                       but a previous def'n exists: {:?}",
+                      node_id,
+                      data,
+                      self.table.def_key(self.node_to_def_index[&node_id]));
 
         // The root node must be created with create_root_def()
-        assert!(data != DefPathData::CrateRoot);
+        debug_assert!(data != DefPathData::CrateRoot);
 
         // Find the next free disambiguator for this key.
         let disambiguator = {
@@ -581,8 +582,8 @@ impl Definitions {
     /// AST to HIR lowering.
     pub fn init_node_id_to_hir_id_mapping(&mut self,
                                           mapping: IndexVec<ast::NodeId, hir::HirId>) {
-        assert!(self.node_to_hir_id.is_empty(),
-                "Trying initialize NodeId -> HirId mapping twice");
+        debug_assert!(self.node_to_hir_id.is_empty(),
+                      "Trying initialize NodeId -> HirId mapping twice");
         self.node_to_hir_id = mapping;
     }
 

src/librustc/hir/map/hir_id_validator.rs

@@ -82,7 +82,7 @@ impl<'a, 'hir: 'a> HirIdValidator<'a, 'hir> {
     fn check<F: FnOnce(&mut HirIdValidator<'a, 'hir>)>(&mut self,
                                                        node_id: NodeId,
                                                        walk: F) {
-        assert!(self.owner_def_index.is_none());
+        debug_assert!(self.owner_def_index.is_none());
         let owner_def_index = self.hir_map.local_def_id(node_id).index;
         self.owner_def_index = Some(owner_def_index);
         walk(self);

src/librustc/hir/map/mod.rs

@@ -350,7 +350,7 @@ impl<'hir> Map<'hir> {
     }
 
     pub fn def_key(&self, def_id: DefId) -> DefKey {
-        assert!(def_id.is_local());
+        debug_assert!(def_id.is_local());
         self.definitions.def_key(def_id.index)
     }
 
@@ -361,7 +361,7 @@ impl<'hir> Map<'hir> {
     }
 
     pub fn def_path(&self, def_id: DefId) -> DefPath {
-        assert!(def_id.is_local());
+        debug_assert!(def_id.is_local());
         self.definitions.def_path(def_id.index)
     }
 
@@ -554,7 +554,7 @@ impl<'hir> Map<'hir> {
     /// item (possibly associated), a closure, or a `hir::AnonConst`.
     pub fn body_owner(&self, BodyId { node_id }: BodyId) -> NodeId {
         let parent = self.get_parent_node(node_id);
-        assert!(self.map[parent.as_usize()].is_body_owner(node_id));
+        debug_assert!(self.map[parent.as_usize()].is_body_owner(node_id));
         parent
     }
 

src/librustc/hir/print.rs

@@ -2001,7 +2001,7 @@ impl<'a> State<'a> {
         self.popen()?;
         let mut i = 0;
         // Make sure we aren't supplied *both* `arg_names` and `body_id`.
-        assert!(arg_names.is_empty() || body_id.is_none());
+        debug_assert!(arg_names.is_empty() || body_id.is_none());
         self.commasep(Inconsistent, &decl.inputs, |s, ty| {
             s.ibox(indent_unit)?;
             if let Some(arg_name) = arg_names.get(i) {

src/librustc/infer/anon_types/mod.rs

@@ -513,15 +513,15 @@ impl<'cx, 'gcx, 'tcx> ReverseMapper<'cx, 'gcx, 'tcx> {
     }
 
     fn fold_kind_mapping_missing_regions_to_empty(&mut self, kind: Kind<'tcx>) -> Kind<'tcx> {
-        assert!(!self.map_missing_regions_to_empty);
+        debug_assert!(!self.map_missing_regions_to_empty);
         self.map_missing_regions_to_empty = true;
         let kind = kind.fold_with(self);
         self.map_missing_regions_to_empty = false;
         kind
     }
 
     fn fold_kind_normally(&mut self, kind: Kind<'tcx>) -> Kind<'tcx> {
-        assert!(!self.map_missing_regions_to_empty);
+        debug_assert!(!self.map_missing_regions_to_empty);
         kind.fold_with(self)
     }
 }

src/librustc/infer/canonical/canonicalizer.rs

@@ -396,7 +396,7 @@ impl<'cx, 'gcx, 'tcx> Canonicalizer<'cx, 'gcx, 'tcx> {
                 // If `var_values` has become big enough to be heap-allocated,
                 // fill up `indices` to facilitate subsequent lookups.
                 if !var_values.is_array() {
-                    assert!(indices.is_empty());
+                    debug_assert!(indices.is_empty());
                     *indices =
                         var_values.iter()
                             .enumerate()

src/librustc/infer/canonical/query_result.rs

@@ -576,8 +576,8 @@ pub fn make_query_outlives<'tcx>(
         givens,
     } = region_constraints;
 
-    assert!(verifys.is_empty());
-    assert!(givens.is_empty());
+    debug_assert!(verifys.is_empty());
+    debug_assert!(givens.is_empty());
 
     let mut outlives: Vec<_> = constraints
             .into_iter()

src/librustc/infer/higher_ranked/mod.rs

@@ -252,7 +252,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
                                              -> ty::Region<'tcx> {
             // Regions that pre-dated the LUB computation stay as they are.
             if !is_var_in_set(new_vars, r0) {
-                assert!(!r0.is_late_bound());
+                debug_assert!(!r0.is_late_bound());
                 debug!("generalize_region(r0={:?}): not new variable", r0);
                 return r0;
             }
@@ -266,7 +266,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
                 debug!("generalize_region(r0={:?}): \
                         non-new-variables found in {:?}",
                        r0, tainted);
-                assert!(!r0.is_late_bound());
+                debug_assert!(!r0.is_late_bound());
                 return r0;
             }
 
@@ -349,7 +349,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
                                              r0: ty::Region<'tcx>)
                                              -> ty::Region<'tcx> {
             if !is_var_in_set(new_vars, r0) {
-                assert!(!r0.is_late_bound());
+                debug_assert!(!r0.is_late_bound());
                 return r0;
             }
 
@@ -402,7 +402,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
                 return rev_lookup(infcx, span, a_map, a_r.unwrap());
             } else if a_r.is_none() && b_r.is_none() {
                 // Not related to bound variables from either fn:
-                assert!(!r0.is_late_bound());
+                debug_assert!(!r0.is_late_bound());
                 return r0;
             } else {
                 // Other:
@@ -468,7 +468,7 @@ fn fold_regions_in<'a, 'gcx, 'tcx, T, F>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
         // we should only be encountering "escaping" late-bound regions here,
         // because the ones at the current level should have been replaced
         // with fresh variables
-        assert!(match *region {
+        debug_assert!(match *region {
             ty::ReLateBound(..) => false,
             _ => true
         });
@@ -746,13 +746,13 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                     // trait checking, and all of the skolemized regions
                     // appear inside predicates, which always have
                     // binders, so this assert is satisfied.
-                    assert!(current_depth > ty::INNERMOST);
+                    debug_assert!(current_depth > ty::INNERMOST);
 
                     // since leak-check passed, this skolemized region
                     // should only have incoming edges from variables
                     // (which ought not to escape the snapshot, but we
                     // don't check that) or itself
-                    assert!(
+                    debug_assert!(
                         match *r {
                             ty::ReVar(_) => true,
                             ty::ReSkolemized(_, ref br1) => br == br1,

src/librustc/infer/mod.rs

@@ -1033,7 +1033,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         outlives_env: &OutlivesEnvironment<'tcx>,
         will_later_be_reported_by_nll: bool,
     ) {
-        assert!(self.is_tainted_by_errors() || self.region_obligations.borrow().is_empty(),
+        debug_assert!(self.is_tainted_by_errors() || self.region_obligations.borrow().is_empty(),
                 "region_obligations not empty: {:#?}",
                 self.region_obligations.borrow());
 
@@ -1049,7 +1049,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             lexical_region_resolve::resolve(region_rels, var_infos, data);
 
         let old_value = self.lexical_region_resolutions.replace(Some(lexical_region_resolutions));
-        assert!(old_value.is_none());
+        debug_assert!(old_value.is_none());
 
         if !self.is_tainted_by_errors() {
             // As a heuristic, just skip reporting region errors
@@ -1072,9 +1072,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     /// translate them into the form that the NLL solver
     /// understands. See the NLL module for mode details.
     pub fn take_and_reset_region_constraints(&self) -> RegionConstraintData<'tcx> {
-        assert!(self.region_obligations.borrow().is_empty(),
-                "region_obligations not empty: {:#?}",
-                self.region_obligations.borrow());
+        debug_assert!(self.region_obligations.borrow().is_empty(),
+                      "region_obligations not empty: {:#?}",
+                      self.region_obligations.borrow());
 
         self.borrow_region_constraints().take_and_reset_data()
     }
@@ -1099,7 +1099,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                                                          .take()
                                                          .expect("regions already resolved")
                                                          .into_infos_and_data();
-        assert!(data.is_empty());
+        debug_assert!(data.is_empty());
         var_infos
     }