Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

global_allocator can not not be defined inside module #44113

Closed
rkapl opened this issue Aug 27, 2017 · 13 comments · Fixed by #62735
Closed

global_allocator can not not be defined inside module #44113

rkapl opened this issue Aug 27, 2017 · 13 comments · Fixed by #62735
Labels
A-allocators Area: Custom and system allocators C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@rkapl
Copy link

rkapl commented Aug 27, 2017

When trying define a global allocator inside a module, you get a weird error message: [E0432]: unresolved import super. The global_allocator is expanded to code containing use super::<varname> but I don't know why rustc fails to resolve that in this case. Happens with both inline and file modules.

Looks like the module that is created by expanding global_allocator thinks its parent is the root crate despite being in the breaks module.

Example:

#![feature(global_allocator, allocator_api)]                                                                                                                                                                       
                                                                                                                                                                                                                   
struct TAlloc;                                                                                                                                                                                                     
                                                                                                                                                                                                                   
unsafe impl<'a> std::heap::Alloc for &'a TAlloc{                                                                                                                                                                   
    unsafe fn alloc(&mut self, _layout: std::heap::Layout) -> std::result::Result<*mut u8, std::heap::AllocErr> {                                                                                                  
        return Err(std::heap::AllocErr::Unsupported{details: "Stub allocator"});                                                                                                                                   
    }                                                                                                                                                                                                              
    unsafe fn dealloc(&mut self, _ptr: *mut u8, _layout: std::heap::Layout) {                                                                                                                                      
    }                                                                                                                                                                                                              
}                                                                                                                                                                                                                  
                                                                                                                                                                                                                   
mod breaks{                                                                                                                                                                                                        
    #[global_allocator]                                                                                                                                                                                            
    static ALLOCATOR: ::TAlloc = ::TAlloc;                                                                                                                                                                         
}                                                                                                                                                                                                                  
                                                                                                                                                                                                                   
pub fn main() {                                                                                                                                                                                                    
}

causes:

error[E0432]: unresolved import `super`
  --> alloc.rs:15:5
   |
15 |     static ALLOCATOR: ::TAlloc = ::TAlloc;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `ALLOCATOR` in the root
@carols10cents carols10cents added A-allocators Area: Custom and system allocators C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Aug 28, 2017
@mattico
Copy link
Contributor

mattico commented Sep 27, 2017

I looked into this a bit, writing some notes here so I don't forget what I found.

The problem is here:

let super_path = f.cx.path(f.span, vec![
Ident::from_str("super"),
f.global,
]);

The Path for the allocator is hard-coded to be super::<allocator name>. We need to instead get a proper Path for f.global.

There may be a function hidden in the compiler somewhere that can help get a Path for an Item. If there isn't we could keep a Vec<Mod> as we traverse modules and convert that to a Path if we find an allocator definition.

@mattico
Copy link
Contributor

mattico commented Sep 27, 2017

Another option is to generate stub impls of the global alloc functions in the AST but then fill them out later when there's more information available.

@whitequark
Copy link
Member

whitequark commented Oct 19, 2017

To add to this, this is also broken:

pub fn main() {
    #[global_allocator]
    static ALLOCATOR: ::TAlloc = ::TAlloc;
}

@mark-i-m
Copy link
Member

@mark-i-m
Copy link
Member

Started working on this... It's very WIP at the moment, though. If anyone has feedback, let me know.

bors added a commit that referenced this issue Jun 25, 2018
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
fitzgen added a commit to rustwasm/wasm-pack-template that referenced this issue Oct 9, 2018
@steveklabnik
Copy link
Member

We now have a good error for this:

error: `global_allocator` cannot be used in submodules
 --> src/main.rs:7:5
  |
7 |     static MY_HEAP: Heap = Heap::default();
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

So the real question is, is this something we want to enable in the future? If so, then this shouldn't be closed, and the bug would be tracking that. If we do not, then this should be closed.

@mark-i-m
Copy link
Member

mark-i-m commented Apr 3, 2019

Personally, i would like for that to be possible. For example, when building an OS, it would be nice to be able to everything in a memory management module. Currently, you have to break that abstraction to put the allocator in thr crate root, which then requires exposing the implementation outside the memory management module.

@mark-i-m
Copy link
Member

mark-i-m commented Apr 3, 2019

So I realized that I never gave an update here. The current status is as follows:

About a year ago, I attempted to implement a fix, but we ran into a lot of weird hygiene issues (that even alexcrichton wasn't able to help with). Then, I changed course and implemented the error that Steve posted above. That's about all I know how to do, and that was the end of my endeavors in this space.

Somebody with expertise in macro/hygiene wizardry and compiler internals is needed to fix the problems we ran into in #49320.

@rkapl
Copy link
Author

rkapl commented Apr 3, 2019

For me, the error message is more important than solving the problem perfectly. In my use case, i have just moved the allocator to the root -- but the real problem was finding this solution.

@jq-rs
Copy link

jq-rs commented Apr 25, 2019

Adding submodule support may be useful when combining several crates with custom global allocator with c libraries. With several crates the ´´´__rg_alloc´´´ symbol is overlapping in the linking phase and linking is failing. Or is there a workaround already for this (besides allowing multidefs)?

@mark-i-m
Copy link
Member

I think it is intentionally disallowed to have multiple global allocators. After all, they would no longer be global...

@jq-rs
Copy link

jq-rs commented Apr 25, 2019

In this case they are the same allocator as they are using the same global alloc crate. Still, as they are from different internal crates, they are seen as different allocators during linking. Multidefs is the workaround, but it of course may cause other issues elsewhere. But yes, I agree that maybe submodules is not the way to properly solve this problem.

@petrochenkov
Copy link
Contributor

Fixed in #62735.

Centril added a commit to Centril/rust that referenced this issue Jul 24, 2019
Turn `#[global_allocator]` into a regular attribute macro

It was a 99% macro with exception of some diagnostic details.

As a result of the change, `#[global_allocator]` now works in nested modules and even in nameless blocks.

Fixes rust-lang#44113
Fixes rust-lang#58072
Centril added a commit to Centril/rust that referenced this issue Jul 24, 2019
Turn `#[global_allocator]` into a regular attribute macro

It was a 99% macro with exception of some diagnostic details.

As a result of the change, `#[global_allocator]` now works in nested modules and even in nameless blocks.

Fixes rust-lang#44113
Fixes rust-lang#58072
Centril added a commit to Centril/rust that referenced this issue Jul 25, 2019
Turn `#[global_allocator]` into a regular attribute macro

It was a 99% macro with exception of some diagnostic details.

As a result of the change, `#[global_allocator]` now works in nested modules and even in nameless blocks.

Fixes rust-lang#44113
Fixes rust-lang#58072
Centril added a commit to Centril/rust that referenced this issue Jul 25, 2019
Turn `#[global_allocator]` into a regular attribute macro

It was a 99% macro with exception of some diagnostic details.

As a result of the change, `#[global_allocator]` now works in nested modules and even in nameless blocks.

Fixes rust-lang#44113
Fixes rust-lang#58072
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-allocators Area: Custom and system allocators C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants