Squash warnings: remove final use of insert_unchecked (rust-lang#29)

* Squash warnings: remove final use of insert_unchecked

* wWe only use ensure_global_var as an expr, so make it return that

* cleanup how codegen_alloc_in_memory generates global vars

Author: danielsn

compiler/rustc_codegen_llvm/src/gotoc/cbmc/goto_program/symbol_table.rs

@@ -48,11 +48,6 @@ impl SymbolTable {
         self.symbol_table.insert(symbol.name.to_string(), symbol);
     }
 
-    #[deprecated(note = "Instead, use the `insert()` function.")]
-    pub fn insert_unchecked(&mut self, symbol: Symbol) {
-        self.symbol_table.insert(symbol.name.to_string(), symbol);
-    }
-
     /// Validates the previous value of the symbol using the validator function, then replaces it.
     /// Useful to replace declarations with the actual definition.
     pub fn replace<F: FnOnce(Option<&Symbol>) -> bool>(

compiler/rustc_codegen_llvm/src/gotoc/metadata.rs

@@ -264,6 +264,32 @@ impl<'tcx> GotocCtx<'tcx> {
         Type::union_tag(union_name)
     }
 
+    /// Ensures that a global variable `name` appears in the Symbol table.
+    /// If it doesn't, inserts it.
+    /// If `init_fn` returns `Some(body)`, creates an initializer for the variable using `body`.
+    /// Otherwise, leaves the variable uninitialized .
+    pub fn ensure_global_var<F: FnOnce(&mut GotocCtx<'tcx>, Expr) -> Option<Stmt>>(
+        &mut self,
+        name: &str,
+        is_file_local: bool,
+        t: Type,
+        loc: Location,
+        init_fn: F,
+    ) -> Expr {
+        if !self.symbol_table.contains(name) {
+            let sym = Symbol::variable(name.to_string(), name.to_string(), t.clone(), loc)
+                .with_is_file_local(is_file_local)
+                .with_is_thread_local(false)
+                .with_is_static_lifetime(true);
+            let var = sym.to_expr();
+            self.symbol_table.insert(sym);
+            if let Some(body) = init_fn(self, var) {
+                self.register_initializer(name, body);
+            }
+        }
+        self.symbol_table.lookup(name).unwrap().to_expr()
+    }
+
     /// Ensures that the `name` appears in the Symbol table.
     /// If it doesn't, inserts it using `f`.
     pub fn ensure<F: FnOnce(&mut GotocCtx<'tcx>, &str) -> Symbol>(
@@ -273,8 +299,7 @@ impl<'tcx> GotocCtx<'tcx> {
     ) -> &Symbol {
         if !self.symbol_table.contains(name) {
             let sym = f(self, name);
-            // TODO, using `insert` here causes regression failures.
-            self.symbol_table.insert_unchecked(sym);
+            self.symbol_table.insert(sym);
         }
         self.symbol_table.lookup(name).unwrap()
     }
@@ -400,32 +425,6 @@ impl<'tcx> GotocCtx<'tcx> {
         self.gen_stack_variable(c, &self.fname(), "temp", t, loc)
     }
 
-    /// Generate a global variable if it doens't already exist.
-    /// Otherwise, returns the existing variable.
-    ///
-    pub fn gen_global_variable(
-        &mut self,
-        name: &str,
-        is_file_local: bool,
-        t: Type,
-        loc: Location,
-    ) -> Symbol {
-        debug!(
-            "gen_global_variable\n\tname:\t{}\n\tis_file_local\t{}\n\tt\t{:?}\n\tloc\t{:?}",
-            name, is_file_local, t, loc
-        );
-
-        let sym = self.ensure(name, |_ctx, _name| {
-            Symbol::variable(name.to_string(), name.to_string(), t.clone(), loc)
-                .with_is_file_local(is_file_local)
-                .with_is_thread_local(false)
-                .with_is_static_lifetime(true)
-        });
-        debug!("{}\n{:?}\n{:?}\n", name, sym.typ, t);
-        assert!(sym.typ == t);
-        sym.to_owned()
-    }
-
     pub fn find_function(&mut self, fname: &str) -> Option<Expr> {
         self.symbol_table.lookup(&fname).map(|s| s.to_expr())
     }

compiler/rustc_codegen_llvm/src/gotoc/operand.rs

@@ -439,29 +439,22 @@ impl<'tcx> GotocCtx<'tcx> {
                 .collect()
         });
 
-        // The type of the global static variable may not be in the symbol table if we are dealing
+        // The global static variable may not be in the symbol table if we are dealing
         // with a literal that can be statically allocated.
-        let typ = match self.symbol_table.lookup(&name) {
-            Some(x) => x.typ.clone(),
-            None => {
-                debug!(
-                    "Could not find {} in symbol_table; inserting alloc type ref {:?}",
-                    &name, alloc_typ_ref
-                );
-                self.symbol_table.insert(Symbol::variable(
-                    name.clone(),
-                    name.clone(),
-                    alloc_typ_ref.clone(),
-                    Location::none(),
-                ));
-                alloc_typ_ref.clone()
-            }
-        };
+        // We need to make a constructor whether it was in the table or not, so we can't use the
+        // closure argument to ensure_global_var to do that here.
+        let var = self.ensure_global_var(
+            &name,
+            false, //TODO is this correct?
+            alloc_typ_ref.clone(),
+            Location::none(),
+            |_, _| None,
+        );
+        let var_typ = var.typ().clone();
 
         // Assign the initial value `val` to `var` via an intermediate `temp_var` to allow for
         // transmuting the allocation type to the global static variable type.
         let alloc_data = self.codegen_allocation_data(alloc);
-        let var = self.gen_global_variable(&name, false, typ.clone(), Location::none());
         let val = Expr::struct_expr_from_values(
             alloc_typ_ref.clone(),
             alloc_data
@@ -483,7 +476,7 @@ impl<'tcx> GotocCtx<'tcx> {
         let temp_var = self.gen_function_local_variable(0, &fn_name, alloc_typ_ref).to_expr();
         let body = Stmt::block(vec![
             Stmt::decl(temp_var.clone(), Some(val), Location::none()),
-            var.to_expr().assign(temp_var.transmute_to(var.typ.clone(), &self.symbol_table)),
+            var.assign(temp_var.transmute_to(var_typ, &self.symbol_table)),
         ]);
         self.register_initializer(&name, body);
 

compiler/rustc_codegen_llvm/src/gotoc/rvalue.rs

@@ -643,7 +643,7 @@ impl<'tcx> GotocCtx<'tcx> {
                             }
                             // Casting to a Box<dyn Trait> from a Box<Adt>
                             (ty::Dynamic(..), ty::Adt(..)) => {
-                                let vtable = self.codegen_vtable(o, t).to_expr();
+                                let vtable = self.codegen_vtable(o, t);
                                 let codegened_operand = self.codegen_operand(o);
                                 let box_inner_data =
                                     self.deref_box(codegened_operand).cast_to(Type::void_pointer());
@@ -668,7 +668,7 @@ impl<'tcx> GotocCtx<'tcx> {
                         dynamic_fat_ptr(
                             self.codegen_ty(t),
                             self.codegen_operand(o).cast_to(Type::void_pointer()),
-                            self.codegen_vtable(o, t).to_expr().address_of(),
+                            self.codegen_vtable(o, t).address_of(),
                             &self.symbol_table,
                         )
                     }
@@ -787,7 +787,7 @@ impl<'tcx> GotocCtx<'tcx> {
         (vt_size, vt_align)
     }
 
-    fn codegen_vtable(&mut self, operand: &Operand<'tcx>, dst_mir_type: Ty<'tcx>) -> &Symbol {
+    fn codegen_vtable(&mut self, operand: &Operand<'tcx>, dst_mir_type: Ty<'tcx>) -> Expr {
         let src_mir_type = self.monomorphize(self.operand_ty(operand));
         return self.codegen_vtable_from_types(src_mir_type, dst_mir_type);
     }
@@ -796,7 +796,7 @@ impl<'tcx> GotocCtx<'tcx> {
         &mut self,
         src_mir_type: Ty<'tcx>,
         dst_mir_type: Ty<'tcx>,
-    ) -> &Symbol {
+    ) -> Expr {
         let trait_type = match dst_mir_type.kind() {
             // dst is pointer type
             ty::Ref(_, pointee_type, ..) => pointee_type,
@@ -819,33 +819,28 @@ impl<'tcx> GotocCtx<'tcx> {
         let vtable_name = self.vtable_name(trait_type);
         let vtable_impl_name = format!("{}_impl_for_{}", vtable_name, src_name);
 
-        self.ensure(&vtable_impl_name, |ctx, _| {
-            // Build the vtable
-            let drop_irep = ctx.codegen_vtable_drop_in_place();
-            let (vt_size, vt_align) = ctx.codegen_vtable_size_and_align(&src_mir_type);
-            let mut vtable_fields = vec![drop_irep, vt_size, vt_align];
-            let concrete_type = binders.principal().unwrap().with_self_ty(ctx.tcx, src_mir_type);
-            let mut methods = ctx.codegen_vtable_methods(concrete_type, trait_type);
-            vtable_fields.append(&mut methods);
-            let vtable = Expr::struct_expr_from_values(
-                Type::struct_tag(&vtable_name),
-                vtable_fields,
-                &ctx.symbol_table,
-            );
-
-            // Store vtable in a static variable (compare codegen_alloc_in_memory)
-            let vtable_var = ctx.gen_global_variable(
-                &vtable_impl_name,
-                true, // REVISIT: static-scope https://github.com/model-checking/rmc/issues/10
-                Type::struct_tag(&vtable_name),
-                Location::none(),
-            );
-
-            // Add the code initializing vtable variable
-            ctx.register_initializer(&vtable_impl_name, vtable_var.to_expr().assign(vtable));
-
-            // Return the vtable variable
-            vtable_var
-        })
+        self.ensure_global_var(
+            &vtable_impl_name,
+            true, // REVISIT: static-scope https://github.com/model-checking/rmc/issues/10
+            Type::struct_tag(&vtable_name),
+            Location::none(),
+            |ctx, var| {
+                // Build the vtable
+                let drop_irep = ctx.codegen_vtable_drop_in_place();
+                let (vt_size, vt_align) = ctx.codegen_vtable_size_and_align(&src_mir_type);
+                let mut vtable_fields = vec![drop_irep, vt_size, vt_align];
+                let concrete_type =
+                    binders.principal().unwrap().with_self_ty(ctx.tcx, src_mir_type);
+                let mut methods = ctx.codegen_vtable_methods(concrete_type, trait_type);
+                vtable_fields.append(&mut methods);
+                let vtable = Expr::struct_expr_from_values(
+                    Type::struct_tag(&vtable_name),
+                    vtable_fields,
+                    &ctx.symbol_table,
+                );
+                let body = var.assign(vtable);
+                Some(body)
+            },
+        )
     }
 }

compiler/rustc_codegen_llvm/src/gotoc/statement.rs

@@ -20,8 +20,8 @@ impl<'tcx> GotocCtx<'tcx> {
         let name = &FN_RETURN_VOID_VAR_NAME.to_string();
         let is_file_local = false;
         let ty = self.codegen_ty_unit();
-        let var = self.gen_global_variable(name, is_file_local, ty, Location::none());
-        Stmt::ret(Some(var.to_expr()), Location::none())
+        let var = self.ensure_global_var(name, is_file_local, ty, Location::none(), |_, _| None);
+        Stmt::ret(Some(var), Location::none())
     }
 
     pub fn codegen_terminator(&mut self, term: &Terminator<'tcx>) -> Stmt {