Introduce trans::declare

We provide tools to tell what exact symbols to emit for any fn or static, but
don’t quite check if that won’t cause any issues later on. Some of the issues
include LLVM mangling our names again and our names pointing to wrong locations,
us generating dumb foreign call wrappers, linker errors, extern functions
resolving to different symbols altogether (extern {fn fail();} fail(); in some
cases calling fail1()), etc.

Before the commit we had a function called note_unique_llvm_symbol, so it is
clear somebody was aware of the issue at some point, but the function was barely
used, mostly in irrelevant locations.

Along with working on it I took liberty to start refactoring trans/base into
a few smaller modules. The refactoring is incomplete and I hope I will find some
motivation to carry on with it.

This is possibly a [breaking-change] because it makes dumbly written code
properly invalid.

Author: nagisa

src/librustc_trans/trans/base.rs

@@ -41,6 +41,7 @@ use trans::common::{self, Block, Result, NodeIdAndSpan, ExprId, CrateContext,
 use trans::consts;
 use trans::datum::*;
 use trans::debuginfo::{DebugLoc, ToDebugLoc};
+use trans::declare;
 use trans::expr;
 use trans::glue;
 use trans::inline;
@@ -314,13 +315,9 @@ pub fn trans_fn_pointer_shim<'a, 'tcx>(
     debug!("tuple_fn_ty: {}", tuple_fn_ty.repr(tcx));
 
     //
-    let function_name =
-        link::mangle_internal_name_by_type_and_seq(ccx, bare_fn_ty,
-                                                   "fn_pointer_shim");
-    let llfn =
-        decl_internal_rust_fn(ccx,
-                              tuple_fn_ty,
-                              &function_name[..]);
+    let function_name = link::mangle_internal_name_by_type_and_seq(ccx, bare_fn_ty,
+                                                                   "fn_pointer_shim");
+    let llfn = declare::declare_internal_rust_fn(ccx, &function_name[..], tuple_fn_ty);
 
     //
     let empty_substs = tcx.mk_substs(Substs::trans_empty());

src/librustc_trans/trans/callee.rs

@@ -126,6 +126,7 @@ use trans::callee;
 use trans::common;
 use trans::common::{Block, FunctionContext, ExprId, NodeIdAndSpan};
 use trans::debuginfo::{DebugLoc, ToDebugLoc};
+use trans::declare;
 use trans::glue;
 use middle::region;
 use trans::type_::Type;
@@ -844,10 +845,8 @@ impl<'blk, 'tcx> CleanupHelperMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx
                     Some(llpersonality) => llpersonality,
                     None => {
                         let fty = Type::variadic_func(&[], &Type::i32(self.ccx));
-                        let f = base::decl_cdecl_fn(self.ccx,
-                                                    "rust_eh_personality",
-                                                    fty,
-                                                    self.ccx.tcx().types.i32);
+                        let f = declare::declare_cfn(self.ccx, "rust_eh_personality", fty,
+                                                     self.ccx.tcx().types.i32);
                         *personality = Some(f);
                         f
                     }

src/librustc_trans/trans/cleanup.rs

@@ -20,6 +20,7 @@ use trans::common::*;
 use trans::datum::{Datum, rvalue_scratch_datum};
 use trans::datum::{Rvalue, ByValue};
 use trans::debuginfo;
+use trans::declare;
 use trans::expr;
 use trans::monomorphize::{self, MonoId};
 use trans::type_of::*;
@@ -159,7 +160,11 @@ pub fn get_or_create_declaration_if_closure<'a, 'tcx>(ccx: &CrateContext<'a, 'tc
         mangle_internal_name_by_path_and_seq(path, "closure")
     });
 
-    let llfn = decl_internal_rust_fn(ccx, function_type, &symbol[..]);
+    // Currently there’s only a single user of get_or_create_declaration_if_closure and it
+    // unconditionally defines the function, therefore we use define_* here.
+    let llfn = declare::define_internal_rust_fn(ccx, &symbol[..], function_type).unwrap_or_else(||{
+        ccx.sess().bug(&format!("symbol `{}` already defined", symbol));
+    });
 
     // set an inline hint for all closures
     attributes::inline(llfn, attributes::InlineAttr::Hint);

src/librustc_trans/trans/closure.rs

@@ -31,6 +31,7 @@ use trans::cleanup;
 use trans::consts;
 use trans::datum;
 use trans::debuginfo::{self, DebugLoc};
+use trans::declare;
 use trans::machine;
 use trans::monomorphize;
 use trans::type_::Type;
@@ -880,9 +881,10 @@ pub fn C_cstr(cx: &CrateContext, s: InternedString, null_terminated: bool) -> Va
                                                 !null_terminated as Bool);
 
         let gsym = token::gensym("str");
-        let buf = CString::new(format!("str{}", gsym.usize()));
-        let buf = buf.unwrap();
-        let g = llvm::LLVMAddGlobal(cx.llmod(), val_ty(sc).to_ref(), buf.as_ptr());
+        let sym = format!("str{}", gsym.usize());
+        let g = declare::define_global(cx, &sym[..], val_ty(sc)).unwrap_or_else(||{
+            cx.sess().bug(&format!("symbol `{}` is already defined", sym));
+        });
         llvm::LLVMSetInitializer(g, sc);
         llvm::LLVMSetGlobalConstant(g, True);
         llvm::SetLinkage(g, llvm::InternalLinkage);

src/librustc_trans/trans/common.rs

@@ -17,6 +17,7 @@ use middle::{check_const, const_eval, def};
 use trans::{adt, closure, debuginfo, expr, inline, machine};
 use trans::base::{self, push_ctxt};
 use trans::common::*;
+use trans::declare;
 use trans::monomorphize;
 use trans::type_::Type;
 use trans::type_of;
@@ -27,6 +28,7 @@ use util::ppaux::{Repr, ty_to_string};
 use std::iter::repeat;
 use libc::c_uint;
 use syntax::{ast, ast_util};
+use syntax::parse::token;
 use syntax::ptr::P;
 
 pub fn const_lit(cx: &CrateContext, e: &ast::Expr, lit: &ast::Lit)
@@ -75,7 +77,7 @@ pub fn const_lit(cx: &CrateContext, e: &ast::Expr, lit: &ast::Lit)
         ast::LitBool(b) => C_bool(cx, b),
         ast::LitStr(ref s, _) => C_str_slice(cx, (*s).clone()),
         ast::LitBinary(ref data) => {
-            let g = addr_of(cx, C_bytes(cx, &data[..]), "binary", e.id);
+            let g = addr_of(cx, C_bytes(cx, &data[..]), "binary");
             let base = ptrcast(g, Type::i8p(cx));
             let prev_const = cx.const_unsized().borrow_mut()
                                .insert(base, g);
@@ -95,13 +97,16 @@ pub fn ptrcast(val: ValueRef, ty: Type) -> ValueRef {
 
 fn addr_of_mut(ccx: &CrateContext,
                cv: ValueRef,
-               kind: &str,
-               id: ast::NodeId)
+               kind: &str)
                -> ValueRef {
     unsafe {
-        let name = format!("{}{}\0", kind, id);
-        let gv = llvm::LLVMAddGlobal(ccx.llmod(), val_ty(cv).to_ref(),
-                                     name.as_ptr() as *const _);
+        // FIXME: this totally needs a better name generation scheme, perhaps a simple global
+        // counter? Also most other uses of gensym in trans.
+        let gsym = token::gensym("_");
+        let name = format!("{}{}", kind, gsym.usize());
+        let gv = declare::define_global(ccx, &name[..], val_ty(cv)).unwrap_or_else(||{
+            ccx.sess().bug(&format!("symbol `{}` is already defined", name));
+        });
         llvm::LLVMSetInitializer(gv, cv);
         SetLinkage(gv, InternalLinkage);
         SetUnnamedAddr(gv, true);
@@ -111,14 +116,13 @@ fn addr_of_mut(ccx: &CrateContext,
 
 pub fn addr_of(ccx: &CrateContext,
                cv: ValueRef,
-               kind: &str,
-               id: ast::NodeId)
+               kind: &str)
                -> ValueRef {
     match ccx.const_globals().borrow().get(&cv) {
         Some(&gv) => return gv,
         None => {}
     }
-    let gv = addr_of_mut(ccx, cv, kind, id);
+    let gv = addr_of_mut(ccx, cv, kind);
     unsafe {
         llvm::LLVMSetGlobalConstant(gv, True);
     }
@@ -232,7 +236,7 @@ pub fn get_const_expr_as_global<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
         }
     };
 
-    let lvalue = addr_of(ccx, val, "const", expr.id);
+    let lvalue = addr_of(ccx, val, "const");
     ccx.const_values().borrow_mut().insert(key, lvalue);
     lvalue
 }
@@ -280,7 +284,7 @@ pub fn const_expr<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                     if adj.autoderefs == 0 {
                         // Don't copy data to do a deref+ref
                         // (i.e., skip the last auto-deref).
-                        llconst = addr_of(cx, llconst, "autoref", e.id);
+                        llconst = addr_of(cx, llconst, "autoref");
                     } else {
                         // Seeing as we are deref'ing here and take a reference
                         // again to make the pointer part of the far pointer below,
@@ -308,7 +312,7 @@ pub fn const_expr<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                 None => {}
                 Some(box ty::AutoUnsafe(_, None)) |
                 Some(box ty::AutoPtr(_, _, None)) => {
-                    llconst = addr_of(cx, llconst, "autoref", e.id);
+                    llconst = addr_of(cx, llconst, "autoref");
                 }
                 Some(box ty::AutoUnsize(ref k)) => {
                     let info = expr::unsized_info(cx, k, e.id, ty, param_substs,
@@ -591,12 +595,12 @@ fn const_expr_unadjusted<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                   // If this isn't the address of a static, then keep going through
                   // normal constant evaluation.
                   let (v, _) = const_expr(cx, &**sub, param_substs);
-                  addr_of(cx, v, "ref", e.id)
+                  addr_of(cx, v, "ref")
               }
           }
           ast::ExprAddrOf(ast::MutMutable, ref sub) => {
               let (v, _) = const_expr(cx, &**sub, param_substs);
-              addr_of_mut(cx, v, "ref_mut_slice", e.id)
+              addr_of_mut(cx, v, "ref_mut_slice")
           }
           ast::ExprTup(ref es) => {
               let repr = adt::represent_type(cx, ety);
@@ -746,7 +750,7 @@ fn const_expr_unadjusted<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
     }
 }
 
-pub fn trans_static(ccx: &CrateContext, m: ast::Mutability, id: ast::NodeId) {
+pub fn trans_static(ccx: &CrateContext, m: ast::Mutability, id: ast::NodeId) -> ValueRef {
     unsafe {
         let _icx = push_ctxt("trans_static");
         let g = base::get_item_val(ccx, id);
@@ -772,6 +776,7 @@ pub fn trans_static(ccx: &CrateContext, m: ast::Mutability, id: ast::NodeId) {
             }
         }
         debuginfo::create_global_var_metadata(ccx, id, g);
+        g
     }
 }
 

src/librustc_trans/trans/consts.rs

@@ -20,6 +20,7 @@ use trans::base;
 use trans::builder::Builder;
 use trans::common::{ExternMap,tydesc_info,BuilderRef_res};
 use trans::debuginfo;
+use trans::declare;
 use trans::monomorphize::MonoId;
 use trans::type_::{Type, TypeNames};
 use middle::subst::Substs;
@@ -137,7 +138,6 @@ pub struct LocalCrateContext<'tcx> {
     llsizingtypes: RefCell<FnvHashMap<Ty<'tcx>, Type>>,
     adt_reprs: RefCell<FnvHashMap<Ty<'tcx>, Rc<adt::Repr<'tcx>>>>,
     type_hashcodes: RefCell<FnvHashMap<Ty<'tcx>, String>>,
-    all_llvm_symbols: RefCell<FnvHashSet<String>>,
     int_type: Type,
     opaque_vec_type: Type,
     builder: BuilderRef_res,
@@ -418,7 +418,6 @@ impl<'tcx> LocalCrateContext<'tcx> {
                 llsizingtypes: RefCell::new(FnvHashMap()),
                 adt_reprs: RefCell::new(FnvHashMap()),
                 type_hashcodes: RefCell::new(FnvHashMap()),
-                all_llvm_symbols: RefCell::new(FnvHashSet()),
                 int_type: Type::from_ref(ptr::null_mut()),
                 opaque_vec_type: Type::from_ref(ptr::null_mut()),
                 builder: BuilderRef_res(llvm::LLVMCreateBuilderInContext(llcx)),
@@ -673,10 +672,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
         &self.local.type_hashcodes
     }
 
-    pub fn all_llvm_symbols<'a>(&'a self) -> &'a RefCell<FnvHashSet<String>> {
-        &self.local.all_llvm_symbols
-    }
-
     pub fn stats<'a>(&'a self) -> &'a Stats {
         &self.shared.stats
     }
@@ -756,17 +751,16 @@ fn declare_intrinsic(ccx: &CrateContext, key: & &'static str) -> Option<ValueRef
     macro_rules! ifn {
         ($name:expr, fn() -> $ret:expr) => (
             if *key == $name {
-                let f = base::decl_cdecl_fn(
-                    ccx, $name, Type::func(&[], &$ret),
-                    ty::mk_nil(ccx.tcx()));
+                let f = declare::declare_cfn(ccx, $name, Type::func(&[], &$ret),
+                                             ty::mk_nil(ccx.tcx()));
                 ccx.intrinsics().borrow_mut().insert($name, f.clone());
                 return Some(f);
             }
         );
         ($name:expr, fn($($arg:expr),*) -> $ret:expr) => (
             if *key == $name {
-                let f = base::decl_cdecl_fn(ccx, $name,
-                                  Type::func(&[$($arg),*], &$ret), ty::mk_nil(ccx.tcx()));
+                let f = declare::declare_cfn(ccx, $name, Type::func(&[$($arg),*], &$ret),
+                                             ty::mk_nil(ccx.tcx()));
                 ccx.intrinsics().borrow_mut().insert($name, f.clone());
                 return Some(f);
             }
@@ -901,9 +895,9 @@ fn declare_intrinsic(ccx: &CrateContext, key: & &'static str) -> Option<ValueRef
                 // The `if key == $name` is already in ifn!
                 ifn!($name, fn($($arg),*) -> $ret);
             } else if *key == $name {
-                let f = base::decl_cdecl_fn(ccx, stringify!($cname),
-                                      Type::func(&[$($arg),*], &$ret),
-                                      ty::mk_nil(ccx.tcx()));
+                let f = declare::declare_cfn(ccx, stringify!($cname),
+                                             Type::func(&[$($arg),*], &$ret),
+                                             ty::mk_nil(ccx.tcx()));
                 ccx.intrinsics().borrow_mut().insert($name, f.clone());
                 return Some(f);
             }

src/librustc_trans/trans/context.rs

@@ -378,8 +378,7 @@ pub fn trans_fail<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
     let filename = C_str_slice(ccx, filename);
     let line = C_u32(ccx, loc.line as u32);
     let expr_file_line_const = C_struct(ccx, &[v_str, filename, line], false);
-    let expr_file_line = consts::addr_of(ccx, expr_file_line_const,
-                                         "panic_loc", call_info.id);
+    let expr_file_line = consts::addr_of(ccx, expr_file_line_const, "panic_loc");
     let args = vec!(expr_file_line);
     let did = langcall(bcx, Some(call_info.span), "", PanicFnLangItem);
     let bcx = callee::trans_lang_call(bcx,
@@ -407,8 +406,7 @@ pub fn trans_fail_bounds_check<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
     let filename = C_str_slice(ccx,  filename);
     let line = C_u32(ccx, loc.line as u32);
     let file_line_const = C_struct(ccx, &[filename, line], false);
-    let file_line = consts::addr_of(ccx, file_line_const,
-                                    "panic_bounds_check_loc", call_info.id);
+    let file_line = consts::addr_of(ccx, file_line_const, "panic_bounds_check_loc");
     let args = vec!(file_line, index, len);
     let did = langcall(bcx, Some(call_info.span), "", PanicBoundsCheckFnLangItem);
     let bcx = callee::trans_lang_call(bcx,

src/librustc_trans/trans/controlflow.rs

@@ -196,8 +196,9 @@ use llvm::debuginfo::*;
 use metadata::csearch;
 use middle::subst::{self, Substs};
 use trans::{self, adt, machine, type_of};
-use trans::common::{self, NodeIdAndSpan, CrateContext, FunctionContext, Block,
-                    C_bytes, NormalizingClosureTyper};
+use trans::common::{self, NodeIdAndSpan, CrateContext, FunctionContext, Block, C_bytes,
+                    NormalizingClosureTyper};
+use trans::declare;
 use trans::_match::{BindingInfo, TrByCopy, TrByMove, TrByRef};
 use trans::monomorphize;
 use trans::type_::Type;
@@ -4071,7 +4072,7 @@ pub fn insert_reference_to_gdb_debug_scripts_section_global(ccx: &CrateContext)
 /// section.
 fn get_or_insert_gdb_debug_scripts_section_global(ccx: &CrateContext)
                                                   -> llvm::ValueRef {
-    let section_var_name = b"__rustc_debug_gdb_scripts_section__\0";
+    let section_var_name = "__rustc_debug_gdb_scripts_section__";
 
     let section_var = unsafe {
         llvm::LLVMGetNamedGlobal(ccx.llmod(),
@@ -4085,10 +4086,11 @@ fn get_or_insert_gdb_debug_scripts_section_global(ccx: &CrateContext)
         unsafe {
             let llvm_type = Type::array(&Type::i8(ccx),
                                         section_contents.len() as u64);
-            let section_var = llvm::LLVMAddGlobal(ccx.llmod(),
-                                                  llvm_type.to_ref(),
-                                                  section_var_name.as_ptr()
-                                                    as *const _);
+
+            let section_var = declare::define_global(ccx, section_var_name,
+                                                     llvm_type).unwrap_or_else(||{
+                ccx.sess().bug(&format!("symbol `{}` is already defined", section_var_name))
+            });
             llvm::LLVMSetSection(section_var, section_name.as_ptr() as *const _);
             llvm::LLVMSetInitializer(section_var, C_bytes(ccx, section_contents));
             llvm::LLVMSetGlobalConstant(section_var, llvm::True);