From 5da4ff8180b4ba67c6e206e3c910f2a7848c2d07 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Fri, 13 Apr 2018 15:58:16 -0500 Subject: [PATCH 1/6] Attempt to fix hygiene for global_allocator --- src/librustc_allocator/expand.rs | 47 +++++++++++++++++------------- src/librustc_driver/driver.rs | 11 ++++++- src/test/ui/allocator-submodule.rs | 37 +++++++++++++++++++++++ 3 files changed, 74 insertions(+), 21 deletions(-) create mode 100644 src/test/ui/allocator-submodule.rs diff --git a/src/librustc_allocator/expand.rs b/src/librustc_allocator/expand.rs index a9530964bffa2..cea1807cfba63 100644 --- a/src/librustc_allocator/expand.rs +++ b/src/librustc_allocator/expand.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![allow(unused_imports, unused_variables, dead_code)] + use rustc::middle::allocator::AllocatorKind; use rustc_errors; use syntax::ast::{Attribute, Crate, LitKind, StrStyle}; @@ -34,6 +36,7 @@ pub fn modify( sess: &ParseSess, resolver: &mut Resolver, krate: Crate, + crate_name: String, handler: &rustc_errors::Handler, ) -> ast::Crate { ExpandAllocatorDirectives { @@ -41,6 +44,7 @@ pub fn modify( sess, resolver, found: false, + crate_name: Some(crate_name), }.fold_crate(krate) } @@ -49,6 +53,7 @@ struct ExpandAllocatorDirectives<'a> { handler: &'a rustc_errors::Handler, sess: &'a ParseSess, resolver: &'a mut Resolver, + crate_name: Option, } impl<'a> Folder for ExpandAllocatorDirectives<'a> { @@ -77,44 +82,44 @@ impl<'a> Folder for ExpandAllocatorDirectives<'a> { } self.found = true; + // Create a fresh Mark for the new macro expansion we are about to do let mark = Mark::fresh(Mark::root()); mark.set_expn_info(ExpnInfo { - call_site: DUMMY_SP, + call_site: item.span, def_site: None, format: MacroAttribute(Symbol::intern(name)), allow_internal_unstable: true, allow_internal_unsafe: false, edition: hygiene::default_edition(), }); + + // Tie the span to the macro expansion info we just created let span = item.span.with_ctxt(SyntaxContext::empty().apply_mark(mark)); - let ecfg = ExpansionConfig::default(name.to_string()); + + // Create an expansion config + let ecfg = ExpansionConfig::default(self.crate_name.take().unwrap()); + + // Generate a bunch of new items using the AllocFnFactory let mut f = AllocFnFactory { span, kind: AllocatorKind::Global, global: item.ident, - core: Ident::from_str("core"), + core: Ident::with_empty_ctxt(Symbol::gensym("core")), cx: ExtCtxt::new(self.sess, ecfg, self.resolver), }; - let super_path = f.cx.path(f.span, vec![Ident::from_str("super"), f.global]); - let mut items = vec![ - f.cx.item_extern_crate(f.span, f.core), - f.cx.item_use_simple( - f.span, - respan(f.span.shrink_to_lo(), VisibilityKind::Inherited), - super_path, - ), - ]; - for method in ALLOCATOR_METHODS { - items.push(f.allocator_fn(method)); - } - let name = f.kind.fn_name("allocator_abi"); - let allocator_abi = Ident::with_empty_ctxt(Symbol::gensym(&name)); - let module = f.cx.item_mod(span, span, allocator_abi, Vec::new(), items); - let module = f.cx.monotonic_expander().fold_item(module).pop().unwrap(); + + let extcore = { + let extcore = f.cx.item_extern_crate(item.span, f.core); + f.cx.monotonic_expander().fold_item(extcore).pop().unwrap() + }; let mut ret = SmallVector::new(); ret.push(item); - ret.push(module); + ret.push(extcore); + ret.extend(ALLOCATOR_METHODS.iter().map(|method| { + let method = f.allocator_fn(method); + f.cx.monotonic_expander().fold_item(method).pop().unwrap() + })); return ret; } @@ -168,6 +173,7 @@ impl<'a> AllocFnFactory<'a> { let method = self.cx.path( self.span, vec![ + Ident::from_str("self"), self.core, Ident::from_str("alloc"), Ident::from_str("GlobalAlloc"), @@ -218,6 +224,7 @@ impl<'a> AllocFnFactory<'a> { let layout_new = self.cx.path( self.span, vec![ + Ident::from_str("self"), self.core, Ident::from_str("alloc"), Ident::from_str("Layout"), diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index c18a089268659..feeac9d938b6a 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -1051,10 +1051,19 @@ where }); } + // Expand global allocators, which are treated as an in-tree proc macro krate = time(sess, "creating allocators", || { - allocator::expand::modify(&sess.parse_sess, &mut resolver, krate, sess.diagnostic()) + allocator::expand::modify( + &sess.parse_sess, + &mut resolver, + krate, + crate_name.to_string(), + sess.diagnostic(), + ) }); + // Done with macro expansion! + after_expand(&krate)?; if sess.opts.debugging_opts.input_stats { diff --git a/src/test/ui/allocator-submodule.rs b/src/test/ui/allocator-submodule.rs new file mode 100644 index 0000000000000..b73068244b10b --- /dev/null +++ b/src/test/ui/allocator-submodule.rs @@ -0,0 +1,37 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that it is possible to create a global allocator in a submodule, rather than in the crate +// root. + +#![feature(alloc, allocator_api, global_allocator)] + +extern crate alloc; + +use std::alloc::{GlobalAlloc, Layout, Opaque}; + +struct MyAlloc; + +unsafe impl GlobalAlloc for MyAlloc { + unsafe fn alloc(&self, layout: Layout) -> *mut Opaque { + 0 as usize as *mut Opaque + } + + unsafe fn dealloc(&self, ptr: *mut Opaque, layout: Layout) {} +} + +mod submod { + use super::MyAlloc; + + #[global_allocator] + static MY_HEAP: MyAlloc = MyAlloc; +} + +fn main() {} From 792772a93b89b09bdc37643635260668659dcc09 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Sun, 20 May 2018 17:04:00 -0500 Subject: [PATCH 2/6] Prohibit global_allocator in submodules for now - we need to figure out hygiene first - change the test to check that the prohibition works with a good error msg - leaves some comments and debugging code - leaves some of our supposed fixes --- src/Cargo.lock | 1 + src/librustc_allocator/Cargo.toml | 1 + src/librustc_allocator/expand.rs | 119 ++++++++++++++++--------- src/librustc_allocator/lib.rs | 1 + src/test/ui/allocator-submodule.rs | 2 +- src/test/ui/allocator-submodule.stderr | 8 ++ 6 files changed, 91 insertions(+), 41 deletions(-) create mode 100644 src/test/ui/allocator-submodule.stderr diff --git a/src/Cargo.lock b/src/Cargo.lock index 1a5df1b2acf03..64eea50e3e07e 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -2035,6 +2035,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" name = "rustc_allocator" version = "0.0.0" dependencies = [ + "log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "rustc 0.0.0", "rustc_errors 0.0.0", "rustc_target 0.0.0", diff --git a/src/librustc_allocator/Cargo.toml b/src/librustc_allocator/Cargo.toml index 765cb80f35710..1cbde181cafae 100644 --- a/src/librustc_allocator/Cargo.toml +++ b/src/librustc_allocator/Cargo.toml @@ -14,3 +14,4 @@ rustc_errors = { path = "../librustc_errors" } rustc_target = { path = "../librustc_target" } syntax = { path = "../libsyntax" } syntax_pos = { path = "../libsyntax_pos" } +log = "0.4" diff --git a/src/librustc_allocator/expand.rs b/src/librustc_allocator/expand.rs index cea1807cfba63..5c9bf99d15cff 100644 --- a/src/librustc_allocator/expand.rs +++ b/src/librustc_allocator/expand.rs @@ -8,27 +8,30 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![allow(unused_imports, unused_variables, dead_code)] - use rustc::middle::allocator::AllocatorKind; use rustc_errors; -use syntax::ast::{Attribute, Crate, LitKind, StrStyle}; -use syntax::ast::{Arg, FnHeader, Generics, Mac, Mutability, Ty, Unsafety}; -use syntax::ast::{self, Expr, Ident, Item, ItemKind, TyKind, VisibilityKind}; -use syntax::attr; -use syntax::codemap::respan; -use syntax::codemap::{ExpnInfo, MacroAttribute}; -use syntax::ext::base::ExtCtxt; -use syntax::ext::base::Resolver; -use syntax::ext::build::AstBuilder; -use syntax::ext::expand::ExpansionConfig; -use syntax::ext::hygiene::{self, Mark, SyntaxContext}; -use syntax::fold::{self, Folder}; -use syntax::parse::ParseSess; -use syntax::ptr::P; -use syntax::symbol::Symbol; -use syntax::util::small_vector::SmallVector; -use syntax_pos::{Span, DUMMY_SP}; +use syntax::{ + ast::{ + self, Arg, Attribute, Crate, Expr, FnHeader, Generics, Ident, Item, ItemKind, + LitKind, Mod, Mutability, StrStyle, Ty, TyKind, Unsafety, VisibilityKind, + }, + attr, + codemap::{ + respan, ExpnInfo, MacroAttribute, + }, + ext::{ + base::{ExtCtxt, Resolver}, + build::AstBuilder, + expand::ExpansionConfig, + hygiene::{self, Mark, SyntaxContext}, + }, + fold::{self, Folder}, + parse::ParseSess, + ptr::P, + symbol::Symbol, + util::small_vector::SmallVector, +}; +use syntax_pos::Span; use {AllocatorMethod, AllocatorTy, ALLOCATOR_METHODS}; @@ -45,6 +48,7 @@ pub fn modify( resolver, found: false, crate_name: Some(crate_name), + in_submod: -1, // -1 to account for the "root" module }.fold_crate(krate) } @@ -54,10 +58,16 @@ struct ExpandAllocatorDirectives<'a> { sess: &'a ParseSess, resolver: &'a mut Resolver, crate_name: Option, + + // For now, we disallow `global_allocator` in submodules because hygiene is hard. Keep track of + // whether we are in a submodule or not. If `in_submod > 0` we are in a submodule. + in_submod: isize, } impl<'a> Folder for ExpandAllocatorDirectives<'a> { fn fold_item(&mut self, item: P) -> SmallVector> { + info!("in submodule {}", self.in_submod); + let name = if attr::contains_name(&item.attrs, "global_allocator") { "global_allocator" } else { @@ -72,12 +82,15 @@ impl<'a> Folder for ExpandAllocatorDirectives<'a> { } } + if self.in_submod > 0 { + self.handler + .span_err(item.span, "`global_allocator` cannot be used in submodules"); + return SmallVector::one(item); + } + if self.found { - self.handler.span_err( - item.span, - "cannot define more than one \ - #[global_allocator]", - ); + self.handler + .span_err(item.span, "cannot define more than one #[global_allocator]"); return SmallVector::one(item); } self.found = true; @@ -85,7 +98,7 @@ impl<'a> Folder for ExpandAllocatorDirectives<'a> { // Create a fresh Mark for the new macro expansion we are about to do let mark = Mark::fresh(Mark::root()); mark.set_expn_info(ExpnInfo { - call_site: item.span, + call_site: item.span, // use the call site of the static def_site: None, format: MacroAttribute(Symbol::intern(name)), allow_internal_unstable: true, @@ -104,27 +117,55 @@ impl<'a> Folder for ExpandAllocatorDirectives<'a> { span, kind: AllocatorKind::Global, global: item.ident, - core: Ident::with_empty_ctxt(Symbol::gensym("core")), + core: Ident::from_str("core"), cx: ExtCtxt::new(self.sess, ecfg, self.resolver), }; - let extcore = { - let extcore = f.cx.item_extern_crate(item.span, f.core); - f.cx.monotonic_expander().fold_item(extcore).pop().unwrap() - }; + // We will generate a new submodule. To `use` the static from that module, we need to get + // the `super::...` path. + let super_path = f.cx.path(f.span, vec![Ident::from_str("super"), f.global]); + + // Generate the items in the submodule + let mut items = vec![ + // import `core` to use allocators + f.cx.item_extern_crate(f.span, f.core), + // `use` the `global_allocator` in `super` + f.cx.item_use_simple( + f.span, + respan(f.span.shrink_to_lo(), VisibilityKind::Inherited), + super_path, + ), + ]; + + // Add the allocator methods to the submodule + items.extend( + ALLOCATOR_METHODS + .iter() + .map(|method| f.allocator_fn(method)), + ); - let mut ret = SmallVector::new(); + // Generate the submodule itself + let name = f.kind.fn_name("allocator_abi"); + let allocator_abi = Ident::with_empty_ctxt(Symbol::gensym(&name)); + let module = f.cx.item_mod(span, span, allocator_abi, Vec::new(), items); + let module = f.cx.monotonic_expander().fold_item(module).pop().unwrap(); + + // Return the item and new submodule + let mut ret = SmallVector::with_capacity(2); ret.push(item); - ret.push(extcore); - ret.extend(ALLOCATOR_METHODS.iter().map(|method| { - let method = f.allocator_fn(method); - f.cx.monotonic_expander().fold_item(method).pop().unwrap() - })); + ret.push(module); + return ret; } - fn fold_mac(&mut self, mac: Mac) -> Mac { - fold::noop_fold_mac(mac, self) + // If we enter a submodule, take note. + fn fold_mod(&mut self, m: Mod) -> Mod { + info!("enter submodule"); + self.in_submod += 1; + let ret = fold::noop_fold_mod(m, self); + self.in_submod -= 1; + info!("exit submodule"); + ret } } @@ -173,7 +214,6 @@ impl<'a> AllocFnFactory<'a> { let method = self.cx.path( self.span, vec![ - Ident::from_str("self"), self.core, Ident::from_str("alloc"), Ident::from_str("GlobalAlloc"), @@ -224,7 +264,6 @@ impl<'a> AllocFnFactory<'a> { let layout_new = self.cx.path( self.span, vec![ - Ident::from_str("self"), self.core, Ident::from_str("alloc"), Ident::from_str("Layout"), diff --git a/src/librustc_allocator/lib.rs b/src/librustc_allocator/lib.rs index 6595564fb30b5..b217d3665a245 100644 --- a/src/librustc_allocator/lib.rs +++ b/src/librustc_allocator/lib.rs @@ -10,6 +10,7 @@ #![feature(rustc_private)] +#[macro_use] extern crate log; extern crate rustc; extern crate rustc_errors; extern crate rustc_target; diff --git a/src/test/ui/allocator-submodule.rs b/src/test/ui/allocator-submodule.rs index b73068244b10b..40e5940c558db 100644 --- a/src/test/ui/allocator-submodule.rs +++ b/src/test/ui/allocator-submodule.rs @@ -31,7 +31,7 @@ mod submod { use super::MyAlloc; #[global_allocator] - static MY_HEAP: MyAlloc = MyAlloc; + static MY_HEAP: MyAlloc = MyAlloc; //~ ERROR global_allocator } fn main() {} diff --git a/src/test/ui/allocator-submodule.stderr b/src/test/ui/allocator-submodule.stderr new file mode 100644 index 0000000000000..6e7727f8889d4 --- /dev/null +++ b/src/test/ui/allocator-submodule.stderr @@ -0,0 +1,8 @@ +error: `global_allocator` cannot be used in submodules + --> $DIR/allocator-submodule.rs:34:5 + | +LL | static MY_HEAP: MyAlloc = MyAlloc; //~ ERROR global_allocator + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + From 08479aabd03d576e4b2050cd4733e75a2b932f80 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Tue, 5 Jun 2018 19:24:10 -0500 Subject: [PATCH 3/6] enable fold_mac --- src/librustc_allocator/expand.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/librustc_allocator/expand.rs b/src/librustc_allocator/expand.rs index 5c9bf99d15cff..547d5f26cf84b 100644 --- a/src/librustc_allocator/expand.rs +++ b/src/librustc_allocator/expand.rs @@ -13,7 +13,7 @@ use rustc_errors; use syntax::{ ast::{ self, Arg, Attribute, Crate, Expr, FnHeader, Generics, Ident, Item, ItemKind, - LitKind, Mod, Mutability, StrStyle, Ty, TyKind, Unsafety, VisibilityKind, + LitKind, Mac, Mod, Mutability, StrStyle, Ty, TyKind, Unsafety, VisibilityKind, }, attr, codemap::{ @@ -167,6 +167,11 @@ impl<'a> Folder for ExpandAllocatorDirectives<'a> { info!("exit submodule"); ret } + + // `fold_mac` is disabled by default. Enable it here. + fn fold_mac(&mut self, mac: Mac) -> Mac { + fold::noop_fold_mac(mac, self) + } } struct AllocFnFactory<'a> { From e3534028ebcd34b131228a05c1787ad8895209ec Mon Sep 17 00:00:00 2001 From: mark Date: Mon, 18 Jun 2018 21:11:59 -0500 Subject: [PATCH 4/6] fix test --- src/test/ui/allocator-submodule.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/test/ui/allocator-submodule.rs b/src/test/ui/allocator-submodule.rs index 40e5940c558db..39b65766924f9 100644 --- a/src/test/ui/allocator-submodule.rs +++ b/src/test/ui/allocator-submodule.rs @@ -15,16 +15,19 @@ extern crate alloc; -use std::alloc::{GlobalAlloc, Layout, Opaque}; +use std::{ + alloc::{GlobalAlloc, Layout}, + ptr, +}; struct MyAlloc; unsafe impl GlobalAlloc for MyAlloc { - unsafe fn alloc(&self, layout: Layout) -> *mut Opaque { - 0 as usize as *mut Opaque + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + ptr::null_mut() } - unsafe fn dealloc(&self, ptr: *mut Opaque, layout: Layout) {} + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {} } mod submod { From 997655cf87bc981b58b303c842343c750d104efa Mon Sep 17 00:00:00 2001 From: mark Date: Mon, 18 Jun 2018 21:59:32 -0500 Subject: [PATCH 5/6] actually fix test --- src/test/ui/allocator-submodule.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/allocator-submodule.stderr b/src/test/ui/allocator-submodule.stderr index 6e7727f8889d4..06e0d36e8a2b1 100644 --- a/src/test/ui/allocator-submodule.stderr +++ b/src/test/ui/allocator-submodule.stderr @@ -1,5 +1,5 @@ error: `global_allocator` cannot be used in submodules - --> $DIR/allocator-submodule.rs:34:5 + --> $DIR/allocator-submodule.rs:37:5 | LL | static MY_HEAP: MyAlloc = MyAlloc; //~ ERROR global_allocator | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 16d7f87b6cd416175cf145a17230051f83b15bf8 Mon Sep 17 00:00:00 2001 From: mark Date: Fri, 22 Jun 2018 19:59:29 -0500 Subject: [PATCH 6/6] used debug, not info --- src/librustc_allocator/expand.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_allocator/expand.rs b/src/librustc_allocator/expand.rs index 547d5f26cf84b..60d28d8098b4c 100644 --- a/src/librustc_allocator/expand.rs +++ b/src/librustc_allocator/expand.rs @@ -66,7 +66,7 @@ struct ExpandAllocatorDirectives<'a> { impl<'a> Folder for ExpandAllocatorDirectives<'a> { fn fold_item(&mut self, item: P) -> SmallVector> { - info!("in submodule {}", self.in_submod); + debug!("in submodule {}", self.in_submod); let name = if attr::contains_name(&item.attrs, "global_allocator") { "global_allocator" @@ -160,11 +160,11 @@ impl<'a> Folder for ExpandAllocatorDirectives<'a> { // If we enter a submodule, take note. fn fold_mod(&mut self, m: Mod) -> Mod { - info!("enter submodule"); + debug!("enter submodule"); self.in_submod += 1; let ret = fold::noop_fold_mod(m, self); self.in_submod -= 1; - info!("exit submodule"); + debug!("exit submodule"); ret }