From 3a7abc37bc2b01473d8290999c95e4750fab498b Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Mon, 12 Apr 2021 11:25:44 -0400 Subject: [PATCH] Squash warnings: remove final use of insert_unchecked (#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 --- .../gotoc/cbmc/goto_program/symbol_table.rs | 5 -- .../rustc_codegen_llvm/src/gotoc/metadata.rs | 55 +++++++++-------- .../rustc_codegen_llvm/src/gotoc/operand.rs | 31 ++++------ .../rustc_codegen_llvm/src/gotoc/rvalue.rs | 59 +++++++++---------- .../rustc_codegen_llvm/src/gotoc/statement.rs | 4 +- 5 files changed, 68 insertions(+), 86 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/gotoc/cbmc/goto_program/symbol_table.rs b/compiler/rustc_codegen_llvm/src/gotoc/cbmc/goto_program/symbol_table.rs index 7eba1a8207213..4c8556ef4678a 100644 --- a/compiler/rustc_codegen_llvm/src/gotoc/cbmc/goto_program/symbol_table.rs +++ b/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) -> bool>( diff --git a/compiler/rustc_codegen_llvm/src/gotoc/metadata.rs b/compiler/rustc_codegen_llvm/src/gotoc/metadata.rs index 2407ffcb71fff..8c2dc68827d7a 100644 --- a/compiler/rustc_codegen_llvm/src/gotoc/metadata.rs +++ b/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, Expr) -> Option>( + &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, &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 { self.symbol_table.lookup(&fname).map(|s| s.to_expr()) } diff --git a/compiler/rustc_codegen_llvm/src/gotoc/operand.rs b/compiler/rustc_codegen_llvm/src/gotoc/operand.rs index 55e9276e529de..9d0fc122c8030 100644 --- a/compiler/rustc_codegen_llvm/src/gotoc/operand.rs +++ b/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); diff --git a/compiler/rustc_codegen_llvm/src/gotoc/rvalue.rs b/compiler/rustc_codegen_llvm/src/gotoc/rvalue.rs index f0510ed7b3f29..331f08399f5a4 100644 --- a/compiler/rustc_codegen_llvm/src/gotoc/rvalue.rs +++ b/compiler/rustc_codegen_llvm/src/gotoc/rvalue.rs @@ -643,7 +643,7 @@ impl<'tcx> GotocCtx<'tcx> { } // Casting to a Box from a Box (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) + }, + ) } } diff --git a/compiler/rustc_codegen_llvm/src/gotoc/statement.rs b/compiler/rustc_codegen_llvm/src/gotoc/statement.rs index de489cb8ec56c..ef3c7e8db2b28 100644 --- a/compiler/rustc_codegen_llvm/src/gotoc/statement.rs +++ b/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 {