Skip to content

Commit

Permalink
Auto merge of #51335 - mark-i-m:allocator, r=oli-obk
Browse files Browse the repository at this point in the history
Prohibit `global_allocator` in submodules

Background: #44113 is caused by weird interactions with hygiene. Hygiene is hard. After a lot of playing around, we decided that the best path forward would be to prohibit `global_allocator`s from being in submodules for now. When somebody gets it working, we can re-enable it.

This PR contains the following
- Some hygiene "fixes" -- things I suspect are the correct thing to do that will make life easier in the future. This includes using call_site hygiene for the generated module and passing the correct crate name to the expansion config.
- Comments and minor formatting fixes
- Some debugging code
- Code to prohibit `global_allocator` in submodules
- Test checking that the proper error occurs.

cc #44113 #49320 #51241

r? @alexcrichton
  • Loading branch information
bors committed Jun 25, 2018
2 parents 5f9c7f9 + 16d7f87 commit 446aef6
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 29 deletions.
1 change: 1 addition & 0 deletions src/Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2035,6 +2035,7 @@ source = "registry+https:/rust-lang/crates.io-index"
name = "rustc_allocator"
version = "0.0.0"
dependencies = [
"log 0.4.1 (registry+https:/rust-lang/crates.io-index)",
"rustc 0.0.0",
"rustc_errors 0.0.0",
"rustc_target 0.0.0",
Expand Down
1 change: 1 addition & 0 deletions src/librustc_allocator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
107 changes: 79 additions & 28 deletions src/librustc_allocator/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,37 +10,45 @@

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, Mac, 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};

pub fn modify(
sess: &ParseSess,
resolver: &mut Resolver,
krate: Crate,
crate_name: String,
handler: &rustc_errors::Handler,
) -> ast::Crate {
ExpandAllocatorDirectives {
handler,
sess,
resolver,
found: false,
crate_name: Some(crate_name),
in_submod: -1, // -1 to account for the "root" module
}.fold_crate(krate)
}

Expand All @@ -49,10 +57,17 @@ struct ExpandAllocatorDirectives<'a> {
handler: &'a rustc_errors::Handler,
sess: &'a ParseSess,
resolver: &'a mut Resolver,
crate_name: Option<String>,

// 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<Item>) -> SmallVector<P<Item>> {
debug!("in submodule {}", self.in_submod);

let name = if attr::contains_name(&item.attrs, "global_allocator") {
"global_allocator"
} else {
Expand All @@ -67,57 +82,93 @@ 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;

// 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, // use the call site of the static
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"),
cx: ExtCtxt::new(self.sess, ecfg, self.resolver),
};

// 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,
),
];
for method in ALLOCATOR_METHODS {
items.push(f.allocator_fn(method));
}

// Add the allocator methods to the submodule
items.extend(
ALLOCATOR_METHODS
.iter()
.map(|method| f.allocator_fn(method)),
);

// 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();

let mut ret = SmallVector::new();
// Return the item and new submodule
let mut ret = SmallVector::with_capacity(2);
ret.push(item);
ret.push(module);

return ret;
}

// If we enter a submodule, take note.
fn fold_mod(&mut self, m: Mod) -> Mod {
debug!("enter submodule");
self.in_submod += 1;
let ret = fold::noop_fold_mod(m, self);
self.in_submod -= 1;
debug!("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)
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_allocator/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#![feature(rustc_private)]

#[macro_use] extern crate log;
extern crate rustc;
extern crate rustc_errors;
extern crate rustc_target;
Expand Down
11 changes: 10 additions & 1 deletion src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
40 changes: 40 additions & 0 deletions src/test/ui/allocator-submodule.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// 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.

// 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},
ptr,
};

struct MyAlloc;

unsafe impl GlobalAlloc for MyAlloc {
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
ptr::null_mut()
}

unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {}
}

mod submod {
use super::MyAlloc;

#[global_allocator]
static MY_HEAP: MyAlloc = MyAlloc; //~ ERROR global_allocator
}

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/allocator-submodule.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: `global_allocator` cannot be used in submodules
--> $DIR/allocator-submodule.rs:37:5
|
LL | static MY_HEAP: MyAlloc = MyAlloc; //~ ERROR global_allocator
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

0 comments on commit 446aef6

Please sign in to comment.