about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEli Friedman <eli.friedman@gmail.com>2015-06-28 10:36:46 -0700
committerEli Friedman <eli.friedman@gmail.com>2015-07-24 19:17:45 -0700
commit8ebf95257bfc9093cb25dba209ded303ec167e5f (patch)
treeaaaf49abe71d1d2255b4a1c5ee23e0f85e64b0d3
parent82d40cb2ba44a27b7db8ec185b5c532237db4b3e (diff)
Allow recursive static variables.
There isn't any particularly good reason for this restriction, so just
get rid of it, and fix trans to handle this case.
-rw-r--r--src/librustc/middle/check_static_recursion.rs4
-rw-r--r--src/librustc_trans/trans/base.rs52
-rw-r--r--src/librustc_trans/trans/consts.rs56
-rw-r--r--src/librustc_trans/trans/context.rs19
-rw-r--r--src/test/compile-fail/const-recursive.rs5
-rw-r--r--src/test/compile-fail/issue-17252.rs4
-rw-r--r--src/test/run-pass/static-recursive.rs15
7 files changed, 96 insertions, 59 deletions
diff --git a/src/librustc/middle/check_static_recursion.rs b/src/librustc/middle/check_static_recursion.rs
index 312a4708e8d..822106c5269 100644
--- a/src/librustc/middle/check_static_recursion.rs
+++ b/src/librustc/middle/check_static_recursion.rs
@@ -13,7 +13,7 @@
 
 use ast_map;
 use session::Session;
-use middle::def::{DefStatic, DefConst, DefAssociatedConst, DefVariant, DefMap};
+use middle::def::{DefConst, DefAssociatedConst, DefVariant, DefMap};
 use util::nodemap::NodeMap;
 
 use syntax::{ast, ast_util};
@@ -37,7 +37,6 @@ struct CheckCrateVisitor<'a, 'ast: 'a> {
 impl<'a, 'ast: 'a> Visitor<'ast> for CheckCrateVisitor<'a, 'ast> {
     fn visit_item(&mut self, it: &'ast ast::Item) {
         match it.node {
-            ast::ItemStatic(..) |
             ast::ItemConst(..) => {
                 let mut recursion_visitor =
                     CheckItemRecursionVisitor::new(self, &it.span);
@@ -217,7 +216,6 @@ impl<'a, 'ast: 'a> Visitor<'ast> for CheckItemRecursionVisitor<'a, 'ast> {
         match e.node {
             ast::ExprPath(..) => {
                 match self.def_map.borrow().get(&e.id).map(|d| d.base_def) {
-                    Some(DefStatic(def_id, _)) |
                     Some(DefAssociatedConst(def_id, _)) |
                     Some(DefConst(def_id))
                            if ast_util::is_local(def_id) => {
diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs
index 207251496e4..c331bf8d461 100644
--- a/src/librustc_trans/trans/base.rs
+++ b/src/librustc_trans/trans/base.rs
@@ -2090,7 +2090,7 @@ pub fn trans_item(ccx: &CrateContext, item: &ast::Item) {
           let mut v = TransItemVisitor{ ccx: ccx };
           v.visit_expr(&**expr);
 
-          let g = consts::trans_static(ccx, m, item.id);
+          let g = consts::trans_static(ccx, m, expr, item.id, &item.attrs);
           update_linkage(ccx, g, Some(item.id), OriginalTranslation);
       },
       ast::ItemForeignMod(ref foreign_mod) => {
@@ -2334,7 +2334,7 @@ pub fn get_item_val(ccx: &CrateContext, id: ast::NodeId) -> ValueRef {
             let sym = || exported_name(ccx, id, ty, &i.attrs);
 
             let v = match i.node {
-                ast::ItemStatic(_, _, ref expr) => {
+                ast::ItemStatic(..) => {
                     // If this static came from an external crate, then
                     // we need to get the symbol from csearch instead of
                     // using the current crate's name/version
@@ -2342,36 +2342,17 @@ pub fn get_item_val(ccx: &CrateContext, id: ast::NodeId) -> ValueRef {
                     let sym = sym();
                     debug!("making {}", sym);
 
-                    // We need the translated value here, because for enums the
-                    // LLVM type is not fully determined by the Rust type.
-                    let empty_substs = ccx.tcx().mk_substs(Substs::trans_empty());
-                    let (v, ty) = consts::const_expr(ccx, &**expr, empty_substs, None);
-                    ccx.static_values().borrow_mut().insert(id, v);
-                    unsafe {
-                        // boolean SSA values are i1, but they have to be stored in i8 slots,
-                        // otherwise some LLVM optimization passes don't work as expected
-                        let llty = if ty.is_bool() {
-                            llvm::LLVMInt8TypeInContext(ccx.llcx())
-                        } else {
-                            llvm::LLVMTypeOf(v)
-                        };
-
-                        // FIXME(nagisa): probably should be declare_global, because no definition
-                        // is happening here, but we depend on it being defined here from
-                        // const::trans_static. This all logic should be replaced.
-                        let g = declare::define_global(ccx, &sym[..],
-                                                       Type::from_ref(llty)).unwrap_or_else(||{
-                            ccx.sess().span_fatal(i.span, &format!("symbol `{}` is already defined",
-                                                                   sym))
-                        });
-
-                        if attr::contains_name(&i.attrs,
-                                               "thread_local") {
-                            llvm::set_thread_local(g, true);
-                        }
-                        ccx.item_symbols().borrow_mut().insert(i.id, sym);
-                        g
-                    }
+                    // Create the global before evaluating the initializer;
+                    // this is necessary to allow recursive statics.
+                    let llty = type_of(ccx, ty);
+                    let g = declare::define_global(ccx, &sym[..],
+                                                   llty).unwrap_or_else(|| {
+                        ccx.sess().span_fatal(i.span, &format!("symbol `{}` is already defined",
+                                                                sym))
+                    });
+
+                    ccx.item_symbols().borrow_mut().insert(i.id, sym);
+                    g
                 }
 
                 ast::ItemFn(_, _, _, abi, _, _) => {
@@ -2738,6 +2719,13 @@ pub fn trans_crate(tcx: &ty::ctxt, analysis: ty::CrateAnalysis) -> CrateTranslat
         if ccx.sess().opts.debuginfo != NoDebugInfo {
             debuginfo::finalize(&ccx);
         }
+        for &(old_g, new_g) in ccx.statics_to_rauw().borrow().iter() {
+            unsafe {
+                let bitcast = llvm::LLVMConstPointerCast(new_g, llvm::LLVMTypeOf(old_g));
+                llvm::LLVMReplaceAllUsesWith(old_g, bitcast);
+                llvm::LLVMDeleteGlobal(old_g);
+            }
+        }
     }
 
     // Translate the metadata.
diff --git a/src/librustc_trans/trans/consts.rs b/src/librustc_trans/trans/consts.rs
index deab11332c8..bd9c4a17177 100644
--- a/src/librustc_trans/trans/consts.rs
+++ b/src/librustc_trans/trans/consts.rs
@@ -37,8 +37,9 @@ use middle::subst::Substs;
 use middle::ty::{self, Ty};
 use util::nodemap::NodeMap;
 
+use std::ffi::{CStr, CString};
 use libc::c_uint;
-use syntax::{ast, ast_util};
+use syntax::{ast, ast_util, attr};
 use syntax::parse::token;
 use syntax::ptr::P;
 
@@ -898,37 +899,70 @@ fn const_expr_unadjusted<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                                 "bad constant expression type in consts::const_expr"),
     }
 }
-
-pub fn trans_static(ccx: &CrateContext, m: ast::Mutability, id: ast::NodeId) -> ValueRef {
+pub fn trans_static(ccx: &CrateContext,
+                    m: ast::Mutability,
+                    expr: &ast::Expr,
+                    id: ast::NodeId,
+                    attrs: &Vec<ast::Attribute>)
+                    -> ValueRef {
     unsafe {
         let _icx = push_ctxt("trans_static");
         let g = base::get_item_val(ccx, id);
-        // At this point, get_item_val has already translated the
-        // constant's initializer to determine its LLVM type.
-        let v = ccx.static_values().borrow().get(&id).unwrap().clone();
+
+        let empty_substs = ccx.tcx().mk_substs(Substs::trans_empty());
+        let (v, _) = const_expr(ccx, expr, empty_substs, None);
+
         // boolean SSA values are i1, but they have to be stored in i8 slots,
         // otherwise some LLVM optimization passes don't work as expected
-        let v = if llvm::LLVMTypeOf(v) == Type::i1(ccx).to_ref() {
-            llvm::LLVMConstZExt(v, Type::i8(ccx).to_ref())
+        let mut val_llty = llvm::LLVMTypeOf(v);
+        let v = if val_llty == Type::i1(ccx).to_ref() {
+            val_llty = Type::i8(ccx).to_ref();
+            llvm::LLVMConstZExt(v, val_llty)
         } else {
             v
         };
+
+        let ty = ccx.tcx().node_id_to_type(id);
+        let llty = type_of::type_of(ccx, ty);
+        let g = if val_llty == llty.to_ref() {
+            g
+        } else {
+            // If we created the global with the wrong type,
+            // correct the type.
+            let empty_string = CString::new("").unwrap();
+            let name_str_ref = CStr::from_ptr(llvm::LLVMGetValueName(g));
+            let name_string = CString::new(name_str_ref.to_bytes()).unwrap();
+            llvm::LLVMSetValueName(g, empty_string.as_ptr());
+            let new_g = llvm::LLVMGetOrInsertGlobal(
+                ccx.llmod(), name_string.as_ptr(), val_llty);
+            // To avoid breaking any invariants, we leave around the old
+            // global for the moment; we'll replace all references to it
+            // with the new global later. (See base::trans_crate.)
+            ccx.statics_to_rauw().borrow_mut().push((g, new_g));
+            new_g
+        };
         llvm::LLVMSetInitializer(g, v);
 
         // As an optimization, all shared statics which do not have interior
         // mutability are placed into read-only memory.
         if m != ast::MutMutable {
-            let node_ty = ccx.tcx().node_id_to_type(id);
-            let tcontents = node_ty.type_contents(ccx.tcx());
+            let tcontents = ty.type_contents(ccx.tcx());
             if !tcontents.interior_unsafe() {
-                llvm::LLVMSetGlobalConstant(g, True);
+                llvm::LLVMSetGlobalConstant(g, llvm::True);
             }
         }
+
         debuginfo::create_global_var_metadata(ccx, id, g);
+
+        if attr::contains_name(attrs,
+                               "thread_local") {
+            llvm::set_thread_local(g, true);
+        }
         g
     }
 }
 
+
 fn get_static_val<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, did: ast::DefId,
                             ty: Ty<'tcx>) -> ValueRef {
     if ast_util::is_local(did) { return base::get_item_val(ccx, did.node) }
diff --git a/src/librustc_trans/trans/context.rs b/src/librustc_trans/trans/context.rs
index 699b25b9ed0..1b6307f28bf 100644
--- a/src/librustc_trans/trans/context.rs
+++ b/src/librustc_trans/trans/context.rs
@@ -118,9 +118,6 @@ pub struct LocalCrateContext<'tcx> {
     /// Cache of emitted const values
     const_values: RefCell<FnvHashMap<(ast::NodeId, &'tcx Substs<'tcx>), ValueRef>>,
 
-    /// Cache of emitted static values
-    static_values: RefCell<NodeMap<ValueRef>>,
-
     /// Cache of external const values
     extern_const_values: RefCell<DefIdMap<ValueRef>>,
 
@@ -129,6 +126,12 @@ pub struct LocalCrateContext<'tcx> {
     /// Cache of closure wrappers for bare fn's.
     closure_bare_wrapper_cache: RefCell<FnvHashMap<ValueRef, ValueRef>>,
 
+    /// List of globals for static variables which need to be passed to the
+    /// LLVM function ReplaceAllUsesWith (RAUW) when translation is complete.
+    /// (We have to make sure we don't invalidate any ValueRefs referring
+    /// to constants.)
+    statics_to_rauw: RefCell<Vec<(ValueRef, ValueRef)>>,
+
     lltypes: RefCell<FnvHashMap<Ty<'tcx>, Type>>,
     llsizingtypes: RefCell<FnvHashMap<Ty<'tcx>, Type>>,
     adt_reprs: RefCell<FnvHashMap<Ty<'tcx>, Rc<adt::Repr<'tcx>>>>,
@@ -449,10 +452,10 @@ impl<'tcx> LocalCrateContext<'tcx> {
                 const_unsized: RefCell::new(FnvHashMap()),
                 const_globals: RefCell::new(FnvHashMap()),
                 const_values: RefCell::new(FnvHashMap()),
-                static_values: RefCell::new(NodeMap()),
                 extern_const_values: RefCell::new(DefIdMap()),
                 impl_method_cache: RefCell::new(FnvHashMap()),
                 closure_bare_wrapper_cache: RefCell::new(FnvHashMap()),
+                statics_to_rauw: RefCell::new(Vec::new()),
                 lltypes: RefCell::new(FnvHashMap()),
                 llsizingtypes: RefCell::new(FnvHashMap()),
                 adt_reprs: RefCell::new(FnvHashMap()),
@@ -660,10 +663,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
         &self.local.const_values
     }
 
-    pub fn static_values<'a>(&'a self) -> &'a RefCell<NodeMap<ValueRef>> {
-        &self.local.static_values
-    }
-
     pub fn extern_const_values<'a>(&'a self) -> &'a RefCell<DefIdMap<ValueRef>> {
         &self.local.extern_const_values
     }
@@ -677,6 +676,10 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
         &self.local.closure_bare_wrapper_cache
     }
 
+    pub fn statics_to_rauw<'a>(&'a self) -> &'a RefCell<Vec<(ValueRef, ValueRef)>> {
+        &self.local.statics_to_rauw
+    }
+
     pub fn lltypes<'a>(&'a self) -> &'a RefCell<FnvHashMap<Ty<'tcx>, Type>> {
         &self.local.lltypes
     }
diff --git a/src/test/compile-fail/const-recursive.rs b/src/test/compile-fail/const-recursive.rs
index ad05c7c423f..7c05a817243 100644
--- a/src/test/compile-fail/const-recursive.rs
+++ b/src/test/compile-fail/const-recursive.rs
@@ -8,9 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// error-pattern: recursive constant
-static a: isize = b;
-static b: isize = a;
+const a: isize = b; //~ ERROR recursive constant
+const b: isize = a; //~ ERROR recursive constant
 
 fn main() {
 }
diff --git a/src/test/compile-fail/issue-17252.rs b/src/test/compile-fail/issue-17252.rs
index 2e9ef8d6077..0c04e295e14 100644
--- a/src/test/compile-fail/issue-17252.rs
+++ b/src/test/compile-fail/issue-17252.rs
@@ -8,12 +8,12 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-static FOO: usize = FOO; //~ ERROR recursive constant
+const FOO: usize = FOO; //~ ERROR recursive constant
 
 fn main() {
     let _x: [u8; FOO]; // caused stack overflow prior to fix
     let _y: usize = 1 + {
-        static BAR: usize = BAR; //~ ERROR recursive constant
+        const BAR: usize = BAR; //~ ERROR recursive constant
         let _z: [u8; BAR]; // caused stack overflow prior to fix
         1
     };
diff --git a/src/test/run-pass/static-recursive.rs b/src/test/run-pass/static-recursive.rs
new file mode 100644
index 00000000000..edb0678cd78
--- /dev/null
+++ b/src/test/run-pass/static-recursive.rs
@@ -0,0 +1,15 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+static mut S: *const u8 = unsafe { &S as *const *const u8 as *const u8 };
+
+pub fn main() {
+    unsafe { assert_eq!(S, *(S as *const *const u8)); }
+}