Skip to content

Commit e333e6a

Browse files
borsbors
committed
Auto merge of #26630 - eefriedman:recursive-static, r=pnkfelix
***Edit: Fixed now.*** I'm pretty sure the way I'm using LLVMReplaceAllUsesWith here is unsafe... but before I figure out how to fix that, I'd like a reality-check: is this actually useful?
2 parents 04badd6 + 0eea0f6 commit e333e6a

15 files changed

+214
-105
lines changed

‎src/librustc/middle/check_static_recursion.rs

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

11-
// This compiler pass detects static items that refer to themselves
11+
// This compiler pass detects constants that refer to themselves
1212
// recursively.
1313

1414
use ast_map;
@@ -18,6 +18,7 @@ use util::nodemap::NodeMap;
1818

1919
use syntax::{ast, ast_util};
2020
use syntax::codemap::Span;
21+
use syntax::feature_gate::emit_feature_err;
2122
use syntax::visit::Visitor;
2223
use syntax::visit;
2324

@@ -125,8 +126,27 @@ impl<'a, 'ast: 'a> CheckItemRecursionVisitor<'a, 'ast> {
125126
}
126127
fn with_item_id_pushed<F>(&mut self, id: ast::NodeId, f: F)
127128
where F: Fn(&mut Self) {
128-
if self.idstack.iter().any(|x| *x == id) {
129-
span_err!(self.sess, *self.root_span, E0265, "recursive constant");
129+
if self.idstack.iter().any(|&x| x == id) {
130+
let any_static = self.idstack.iter().any(|&x| {
131+
if let ast_map::NodeItem(item) = self.ast_map.get(x) {
132+
if let ast::ItemStatic(..) = item.node {
133+
true
134+
} else {
135+
false
136+
}
137+
} else {
138+
false
139+
}
140+
});
141+
if any_static {
142+
if !self.sess.features.borrow().static_recursion {
143+
emit_feature_err(&self.sess.parse_sess.span_diagnostic,
144+
"static_recursion",
145+
*self.root_span, "recursive static");
146+
}
147+
} else {
148+
span_err!(self.sess, *self.root_span, E0265, "recursive constant");
149+
}
130150
return;
131151
}
132152
self.idstack.push(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/librustc_typeck/check/mod.rs

Lines changed: 12 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ use syntax::attr::AttrMetaMethods;
115115
use syntax::ast::{self, DefId, Visibility};
116116
use syntax::ast_util::{self, local_def};
117117
use syntax::codemap::{self, Span};
118+
use syntax::feature_gate::emit_feature_err;
118119
use syntax::owned_slice::OwnedSlice;
119120
use syntax::parse::token;
120121
use syntax::print::pprust;
@@ -4009,9 +4010,7 @@ fn check_const_with_ty<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
40094010

40104011
/// Checks whether a type can be represented in memory. In particular, it
40114012
/// identifies types that contain themselves without indirection through a
4012-
/// pointer, which would mean their size is unbounded. This is different from
4013-
/// the question of whether a type can be instantiated. See the definition of
4014-
/// `check_instantiable`.
4013+
/// pointer, which would mean their size is unbounded.
40154014
pub fn check_representable(tcx: &ty::ctxt,
40164015
sp: Span,
40174016
item_id: ast::NodeId,
@@ -4036,31 +4035,19 @@ pub fn check_representable(tcx: &ty::ctxt,
40364035
return true
40374036
}
40384037

4039-
/// Checks whether a type can be created without an instance of itself.
4040-
/// This is similar but different from the question of whether a type
4041-
/// can be represented. For example, the following type:
4042-
///
4043-
/// enum foo { None, Some(foo) }
4044-
///
4045-
/// is instantiable but is not representable. Similarly, the type
4046-
///
4047-
/// enum foo { Some(@foo) }
4048-
///
4049-
/// is representable, but not instantiable.
4038+
/// Checks whether a type can be constructed at runtime without
4039+
/// an existing instance of that type.
40504040
pub fn check_instantiable(tcx: &ty::ctxt,
40514041
sp: Span,
4052-
item_id: ast::NodeId)
4053-
-> bool {
4042+
item_id: ast::NodeId) {
40544043
let item_ty = tcx.node_id_to_type(item_id);
4055-
if !item_ty.is_instantiable(tcx) {
4056-
span_err!(tcx.sess, sp, E0073,
4057-
"this type cannot be instantiated without an \
4058-
instance of itself");
4059-
fileline_help!(tcx.sess, sp, "consider using `Option<{:?}>`",
4060-
item_ty);
4061-
false
4062-
} else {
4063-
true
4044+
if !item_ty.is_instantiable(tcx) &&
4045+
!tcx.sess.features.borrow().static_recursion {
4046+
emit_feature_err(&tcx.sess.parse_sess.span_diagnostic,
4047+
"static_recursion",
4048+
sp,
4049+
"this type cannot be instantiated at runtime \
4050+
without an instance of itself");
40644051
}
40654052
}
40664053

@@ -4199,11 +4186,6 @@ pub fn check_enum_variants<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>,
41994186
do_check(ccx, vs, id, hint);
42004187

42014188
check_representable(ccx.tcx, sp, id, "enum");
4202-
4203-
// Check that it is possible to instantiate this enum:
4204-
//
4205-
// This *sounds* like the same that as representable, but it's
4206-
// not. See def'n of `check_instantiable()` for details.
42074189
check_instantiable(ccx.tcx, sp, id);
42084190
}
42094191

‎src/libsyntax/feature_gate.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Status)] = &[
160160

161161
// Allows using #[prelude_import] on glob `use` items.
162162
("prelude_import", "1.2.0", Active),
163+
164+
// Allows the definition recursive static items.
165+
("static_recursion", "1.3.0", Active),
163166
];
164167
// (changing above list without updating src/doc/reference.md makes @cmr sad)
165168

@@ -338,6 +341,7 @@ pub struct Features {
338341
/// #![feature] attrs for non-language (library) features
339342
pub declared_lib_features: Vec<(InternedString, Span)>,
340343
pub const_fn: bool,
344+
pub static_recursion: bool
341345
}
342346

343347
impl Features {
@@ -362,6 +366,7 @@ impl Features {
362366
declared_stable_lang_features: Vec::new(),
363367
declared_lib_features: Vec::new(),
364368
const_fn: false,
369+
static_recursion: false
365370
}
366371
}
367372
}
@@ -859,6 +864,7 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler,
859864
declared_stable_lang_features: accepted_features,
860865
declared_lib_features: unknown_features,
861866
const_fn: cx.has_feature("const_fn"),
867+
static_recursion: cx.has_feature("static_recursion")
862868
}
863869
}
864870

‎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
}

0 commit comments

Comments
 (0)