Skip to content

Commit 8ebf952

Browse files
committed
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.
1 parent 82d40cb commit 8ebf952

File tree

7 files changed

+96
-59
lines changed

7 files changed

+96
-59
lines changed

src/librustc/middle/check_static_recursion.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
use ast_map;
1515
use session::Session;
16-
use middle::def::{DefStatic, DefConst, DefAssociatedConst, DefVariant, DefMap};
16+
use middle::def::{DefConst, DefAssociatedConst, DefVariant, DefMap};
1717
use util::nodemap::NodeMap;
1818

1919
use syntax::{ast, ast_util};
@@ -37,7 +37,6 @@ struct CheckCrateVisitor<'a, 'ast: 'a> {
3737
impl<'a, 'ast: 'a> Visitor<'ast> for CheckCrateVisitor<'a, 'ast> {
3838
fn visit_item(&mut self, it: &'ast ast::Item) {
3939
match it.node {
40-
ast::ItemStatic(..) |
4140
ast::ItemConst(..) => {
4241
let mut recursion_visitor =
4342
CheckItemRecursionVisitor::new(self, &it.span);
@@ -217,7 +216,6 @@ impl<'a, 'ast: 'a> Visitor<'ast> for CheckItemRecursionVisitor<'a, 'ast> {
217216
match e.node {
218217
ast::ExprPath(..) => {
219218
match self.def_map.borrow().get(&e.id).map(|d| d.base_def) {
220-
Some(DefStatic(def_id, _)) |
221219
Some(DefAssociatedConst(def_id, _)) |
222220
Some(DefConst(def_id))
223221
if ast_util::is_local(def_id) => {

src/librustc_trans/trans/base.rs

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2090,7 +2090,7 @@ pub fn trans_item(ccx: &CrateContext, item: &ast::Item) {
20902090
let mut v = TransItemVisitor{ ccx: ccx };
20912091
v.visit_expr(&**expr);
20922092

2093-
let g = consts::trans_static(ccx, m, item.id);
2093+
let g = consts::trans_static(ccx, m, expr, item.id, &item.attrs);
20942094
update_linkage(ccx, g, Some(item.id), OriginalTranslation);
20952095
},
20962096
ast::ItemForeignMod(ref foreign_mod) => {
@@ -2334,44 +2334,25 @@ pub fn get_item_val(ccx: &CrateContext, id: ast::NodeId) -> ValueRef {
23342334
let sym = || exported_name(ccx, id, ty, &i.attrs);
23352335

23362336
let v = match i.node {
2337-
ast::ItemStatic(_, _, ref expr) => {
2337+
ast::ItemStatic(..) => {
23382338
// If this static came from an external crate, then
23392339
// we need to get the symbol from csearch instead of
23402340
// using the current crate's name/version
23412341
// information in the hash of the symbol
23422342
let sym = sym();
23432343
debug!("making {}", sym);
23442344

2345-
// We need the translated value here, because for enums the
2346-
// LLVM type is not fully determined by the Rust type.
2347-
let empty_substs = ccx.tcx().mk_substs(Substs::trans_empty());
2348-
let (v, ty) = consts::const_expr(ccx, &**expr, empty_substs, None);
2349-
ccx.static_values().borrow_mut().insert(id, v);
2350-
unsafe {
2351-
// boolean SSA values are i1, but they have to be stored in i8 slots,
2352-
// otherwise some LLVM optimization passes don't work as expected
2353-
let llty = if ty.is_bool() {
2354-
llvm::LLVMInt8TypeInContext(ccx.llcx())
2355-
} else {
2356-
llvm::LLVMTypeOf(v)
2357-
};
2358-
2359-
// FIXME(nagisa): probably should be declare_global, because no definition
2360-
// is happening here, but we depend on it being defined here from
2361-
// const::trans_static. This all logic should be replaced.
2362-
let g = declare::define_global(ccx, &sym[..],
2363-
Type::from_ref(llty)).unwrap_or_else(||{
2364-
ccx.sess().span_fatal(i.span, &format!("symbol `{}` is already defined",
2365-
sym))
2366-
});
2367-
2368-
if attr::contains_name(&i.attrs,
2369-
"thread_local") {
2370-
llvm::set_thread_local(g, true);
2371-
}
2372-
ccx.item_symbols().borrow_mut().insert(i.id, sym);
2373-
g
2374-
}
2345+
// Create the global before evaluating the initializer;
2346+
// this is necessary to allow recursive statics.
2347+
let llty = type_of(ccx, ty);
2348+
let g = declare::define_global(ccx, &sym[..],
2349+
llty).unwrap_or_else(|| {
2350+
ccx.sess().span_fatal(i.span, &format!("symbol `{}` is already defined",
2351+
sym))
2352+
});
2353+
2354+
ccx.item_symbols().borrow_mut().insert(i.id, sym);
2355+
g
23752356
}
23762357

23772358
ast::ItemFn(_, _, _, abi, _, _) => {
@@ -2738,6 +2719,13 @@ pub fn trans_crate(tcx: &ty::ctxt, analysis: ty::CrateAnalysis) -> CrateTranslat
27382719
if ccx.sess().opts.debuginfo != NoDebugInfo {
27392720
debuginfo::finalize(&ccx);
27402721
}
2722+
for &(old_g, new_g) in ccx.statics_to_rauw().borrow().iter() {
2723+
unsafe {
2724+
let bitcast = llvm::LLVMConstPointerCast(new_g, llvm::LLVMTypeOf(old_g));
2725+
llvm::LLVMReplaceAllUsesWith(old_g, bitcast);
2726+
llvm::LLVMDeleteGlobal(old_g);
2727+
}
2728+
}
27412729
}
27422730

27432731
// Translate the metadata.

src/librustc_trans/trans/consts.rs

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,9 @@ use middle::subst::Substs;
3737
use middle::ty::{self, Ty};
3838
use util::nodemap::NodeMap;
3939

40+
use std::ffi::{CStr, CString};
4041
use libc::c_uint;
41-
use syntax::{ast, ast_util};
42+
use syntax::{ast, ast_util, attr};
4243
use syntax::parse::token;
4344
use syntax::ptr::P;
4445

@@ -898,37 +899,70 @@ fn const_expr_unadjusted<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
898899
"bad constant expression type in consts::const_expr"),
899900
}
900901
}
901-
902-
pub fn trans_static(ccx: &CrateContext, m: ast::Mutability, id: ast::NodeId) -> ValueRef {
902+
pub fn trans_static(ccx: &CrateContext,
903+
m: ast::Mutability,
904+
expr: &ast::Expr,
905+
id: ast::NodeId,
906+
attrs: &Vec<ast::Attribute>)
907+
-> ValueRef {
903908
unsafe {
904909
let _icx = push_ctxt("trans_static");
905910
let g = base::get_item_val(ccx, id);
906-
// At this point, get_item_val has already translated the
907-
// constant's initializer to determine its LLVM type.
908-
let v = ccx.static_values().borrow().get(&id).unwrap().clone();
911+
912+
let empty_substs = ccx.tcx().mk_substs(Substs::trans_empty());
913+
let (v, _) = const_expr(ccx, expr, empty_substs, None);
914+
909915
// boolean SSA values are i1, but they have to be stored in i8 slots,
910916
// otherwise some LLVM optimization passes don't work as expected
911-
let v = if llvm::LLVMTypeOf(v) == Type::i1(ccx).to_ref() {
912-
llvm::LLVMConstZExt(v, Type::i8(ccx).to_ref())
917+
let mut val_llty = llvm::LLVMTypeOf(v);
918+
let v = if val_llty == Type::i1(ccx).to_ref() {
919+
val_llty = Type::i8(ccx).to_ref();
920+
llvm::LLVMConstZExt(v, val_llty)
913921
} else {
914922
v
915923
};
924+
925+
let ty = ccx.tcx().node_id_to_type(id);
926+
let llty = type_of::type_of(ccx, ty);
927+
let g = if val_llty == llty.to_ref() {
928+
g
929+
} else {
930+
// If we created the global with the wrong type,
931+
// correct the type.
932+
let empty_string = CString::new("").unwrap();
933+
let name_str_ref = CStr::from_ptr(llvm::LLVMGetValueName(g));
934+
let name_string = CString::new(name_str_ref.to_bytes()).unwrap();
935+
llvm::LLVMSetValueName(g, empty_string.as_ptr());
936+
let new_g = llvm::LLVMGetOrInsertGlobal(
937+
ccx.llmod(), name_string.as_ptr(), val_llty);
938+
// To avoid breaking any invariants, we leave around the old
939+
// global for the moment; we'll replace all references to it
940+
// with the new global later. (See base::trans_crate.)
941+
ccx.statics_to_rauw().borrow_mut().push((g, new_g));
942+
new_g
943+
};
916944
llvm::LLVMSetInitializer(g, v);
917945

918946
// As an optimization, all shared statics which do not have interior
919947
// mutability are placed into read-only memory.
920948
if m != ast::MutMutable {
921-
let node_ty = ccx.tcx().node_id_to_type(id);
922-
let tcontents = node_ty.type_contents(ccx.tcx());
949+
let tcontents = ty.type_contents(ccx.tcx());
923950
if !tcontents.interior_unsafe() {
924-
llvm::LLVMSetGlobalConstant(g, True);
951+
llvm::LLVMSetGlobalConstant(g, llvm::True);
925952
}
926953
}
954+
927955
debuginfo::create_global_var_metadata(ccx, id, g);
956+
957+
if attr::contains_name(attrs,
958+
"thread_local") {
959+
llvm::set_thread_local(g, true);
960+
}
928961
g
929962
}
930963
}
931964

965+
932966
fn get_static_val<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, did: ast::DefId,
933967
ty: Ty<'tcx>) -> ValueRef {
934968
if ast_util::is_local(did) { return base::get_item_val(ccx, did.node) }

src/librustc_trans/trans/context.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,6 @@ pub struct LocalCrateContext<'tcx> {
118118
/// Cache of emitted const values
119119
const_values: RefCell<FnvHashMap<(ast::NodeId, &'tcx Substs<'tcx>), ValueRef>>,
120120

121-
/// Cache of emitted static values
122-
static_values: RefCell<NodeMap<ValueRef>>,
123-
124121
/// Cache of external const values
125122
extern_const_values: RefCell<DefIdMap<ValueRef>>,
126123

@@ -129,6 +126,12 @@ pub struct LocalCrateContext<'tcx> {
129126
/// Cache of closure wrappers for bare fn's.
130127
closure_bare_wrapper_cache: RefCell<FnvHashMap<ValueRef, ValueRef>>,
131128

129+
/// List of globals for static variables which need to be passed to the
130+
/// LLVM function ReplaceAllUsesWith (RAUW) when translation is complete.
131+
/// (We have to make sure we don't invalidate any ValueRefs referring
132+
/// to constants.)
133+
statics_to_rauw: RefCell<Vec<(ValueRef, ValueRef)>>,
134+
132135
lltypes: RefCell<FnvHashMap<Ty<'tcx>, Type>>,
133136
llsizingtypes: RefCell<FnvHashMap<Ty<'tcx>, Type>>,
134137
adt_reprs: RefCell<FnvHashMap<Ty<'tcx>, Rc<adt::Repr<'tcx>>>>,
@@ -449,10 +452,10 @@ impl<'tcx> LocalCrateContext<'tcx> {
449452
const_unsized: RefCell::new(FnvHashMap()),
450453
const_globals: RefCell::new(FnvHashMap()),
451454
const_values: RefCell::new(FnvHashMap()),
452-
static_values: RefCell::new(NodeMap()),
453455
extern_const_values: RefCell::new(DefIdMap()),
454456
impl_method_cache: RefCell::new(FnvHashMap()),
455457
closure_bare_wrapper_cache: RefCell::new(FnvHashMap()),
458+
statics_to_rauw: RefCell::new(Vec::new()),
456459
lltypes: RefCell::new(FnvHashMap()),
457460
llsizingtypes: RefCell::new(FnvHashMap()),
458461
adt_reprs: RefCell::new(FnvHashMap()),
@@ -660,10 +663,6 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
660663
&self.local.const_values
661664
}
662665

663-
pub fn static_values<'a>(&'a self) -> &'a RefCell<NodeMap<ValueRef>> {
664-
&self.local.static_values
665-
}
666-
667666
pub fn extern_const_values<'a>(&'a self) -> &'a RefCell<DefIdMap<ValueRef>> {
668667
&self.local.extern_const_values
669668
}
@@ -677,6 +676,10 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
677676
&self.local.closure_bare_wrapper_cache
678677
}
679678

679+
pub fn statics_to_rauw<'a>(&'a self) -> &'a RefCell<Vec<(ValueRef, ValueRef)>> {
680+
&self.local.statics_to_rauw
681+
}
682+
680683
pub fn lltypes<'a>(&'a self) -> &'a RefCell<FnvHashMap<Ty<'tcx>, Type>> {
681684
&self.local.lltypes
682685
}

src/test/compile-fail/const-recursive.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,8 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// error-pattern: recursive constant
12-
static a: isize = b;
13-
static b: isize = a;
11+
const a: isize = b; //~ ERROR recursive constant
12+
const b: isize = a; //~ ERROR recursive constant
1413

1514
fn main() {
1615
}

src/test/compile-fail/issue-17252.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
static FOO: usize = FOO; //~ ERROR recursive constant
11+
const FOO: usize = FOO; //~ ERROR recursive constant
1212

1313
fn main() {
1414
let _x: [u8; FOO]; // caused stack overflow prior to fix
1515
let _y: usize = 1 + {
16-
static BAR: usize = BAR; //~ ERROR recursive constant
16+
const BAR: usize = BAR; //~ ERROR recursive constant
1717
let _z: [u8; BAR]; // caused stack overflow prior to fix
1818
1
1919
};

src/test/run-pass/static-recursive.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
static mut S: *const u8 = unsafe { &S as *const *const u8 as *const u8 };
12+
13+
pub fn main() {
14+
unsafe { assert_eq!(S, *(S as *const *const u8)); }
15+
}

0 commit comments

Comments
 (0)