[WIP] Allow global_allocators in submodules by mark-i-m · Pull Request #49320 · rust-lang/rust

mark-i-m · 2018-03-24T01:20:44Z

First commit is rustfmt.

Is this even the right approach? Right now it's just breaking everything...

rust-highfive · 2018-03-24T01:20:56Z

(rust_highfive has picked a reviewer for you, use r? to override)

bors · 2018-03-27T09:39:37Z

☔ The latest upstream changes (presumably #49279) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2018-04-04T20:57:19Z

I could review this if forced, but I'm not super familiar with what's going on here. @alexcrichton -- are you the right person?

alexcrichton · 2018-04-05T00:03:27Z

Sure yeah I don't mind reviewing this. @mark-i-m mind explaning a bit what the change here is doing?

mark-i-m · 2018-04-05T01:42:21Z

@alexcrichton Thanks!

The problem: global_allocators currently can only be declared in the crate root, but it would be nice to be able to declare them in submodules to. The current implementation is a magic proc_macro that gets expanded into an inline submodule with an allocator implementation (see the location of the edits of this PR; that's code that actually does this magic expansion). The implementation forces the item decorated with global_allocator to be in the crate root by hard-coding super::NAME_OF_STATIC as the path of the allocator.

Proposed solution: Rather than hard-coding super::NAME_OF_STATIC as the path of the static, we use the actual path of the static. Unfortunately, it doesn't appear that there is a way to get this path at name resolution when macro expansion happens, so instead, I am trying to keep track of the path as I fold. The current implementation in this PR has bugs, but more importantly, I don't know if that is the right way to approach the problem in the first place. Any help would be appreciated.

alexcrichton · 2018-04-05T15:11:05Z

Ok thanks! I was actually the original author of all this code so I'm mostly curious in the delta from the previous instantiation :)

I think, though that super::NAME should work here, right? We should be unconditionally generating a submodule that references the parent static. I suspect that the underlying issue here is hygiene, so perhaps the hygiene of super or the identifier could be sorted out to get this working?

mark-i-m · 2018-04-05T15:56:02Z

I suspect that the underlying issue here is hygiene, so perhaps the hygiene of super or the identifier could be sorted out to get this working?

TBH, I don't really know that much about how hygiene works in macro expansion (the rustc-guide chapter has yet to be written). Do you have any hints of how to get started?

alexcrichton · 2018-04-05T17:44:11Z

I'm not so sure myself (hence why this has never been fixed), but AFAIK it's all tied to identifiers/spans. It may not be necessary at all to even use super::NAME and instead we could just use NAME. Syntactically the Rust code would look invalid but it may hygienically resolve

mark-i-m · 2018-04-07T19:58:54Z

I get the following error:

failures:
---- [ui] ui/allocator-submodule.rs stdout ----
normalized stderr:
error[E0432]: unresolved import `MY_HEAP`
--> $DIR/allocator-submodule.rs:22:5
|
LL | static MY_HEAP: Heap = Heap::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `MY_HEAP` in the root
error: aborting due to previous error

alexcrichton · 2018-04-09T13:25:50Z

Ah if that doesn't work then it may mean that a special span needs to be used for super that contextualizes it correctly. Unfortunately though I'm still not so sure myself, I'm also pretty unfamiliar with hygiene.

mark-i-m · 2018-04-11T02:26:38Z

Unfortunately, I don't know enough about name resolution or hygiene to know how to do that. Do you have any suggested resources for learning more? Unfortunately, the rustc-guide section is a bit incomplete.

alexcrichton · 2018-04-11T17:42:44Z

Hm ok, an alternative is to change how things are expanded. Currently a #[global_allocator] item expands to itself and a submodule, but the contents of the submodule could be inlined next to #[global_allocator] which may help the resolution issue

mark-i-m · 2018-04-13T20:55:03Z

Thanks @alexcrichton! I will give that a try.

Quick question: What does the following line (112) do exactly?

let module = f.cx.monotonic_expander().fold_item(module).pop().unwrap();

alexcrichton · 2018-04-13T21:09:52Z

A good question! I'm... not actually sure. I may have copy/pasted that from somewhere else originally, and it may be safe to remove.

mark-i-m · 2018-04-14T00:28:45Z

It seems that we have the same hygiene problems with core. I think we need to bite the bullet and figure out how hygiene works in name resolution 😛 ... What we really need is

A way to construct a hygienic non-conflicting new ident on the fly.
A way to construct absolute paths to existing idents such that things like "crate root", super, and self are handled properly.

I feel like (2) already exists but we are doing it wrong?

mark-i-m · 2018-04-14T02:46:39Z

Ok, I think I'm onto something: when we create the new Mark for the macro expansion, I think we need to set the call_site so that we get hygiene for the location of the global item: 60fbde7#diff-a65fc158001a4a43e2add393cbf864a9R89

I also set the crate_name correctly when creating the ExpansionConfig, but I'm not sure what this is used for.

I am now getting "no super::MY_HEAP in root" or "no MY_HEAP in root" errors. I'm hoping that this is coming from the call_allocator-generated call to the allocator, since it uses the static item by name. Does this sound reasonable?

alexcrichton · 2018-04-14T15:14:15Z

Sorry I'm getting pretty mixed up in the diff right now, call the rustfmt changes get backed out for a future run later?

alexcrichton · 2018-04-14T15:15:10Z

This in general sounds plausible though! Want to add a test to make sure if it passes CI it's good to go?

mark-i-m · 2018-04-14T18:26:26Z

Sorry, I will pull out rustfmt and comments to another PR... It doesn't work yet, but if I'm right, then it might make like easier. I have a UI test I've been using locally. I will add it too... I'm on mobile at the moment, but when I get to my computer, I will do this...

mark-i-m · 2018-04-14T18:27:28Z

Also, I believe that last commit is the only one with any actual changes (for the moment)... Will update a bit later...

mark-i-m · 2018-04-14T19:38:10Z

@alexcrichton I have factored out the formatting and commenting into PR #49969. There are now two commits in this PR. The first makes the change to use the call-site for hygiene. The second adds a test for allocators in submodules (it should fail).

rust-highfive · 2018-04-14T20:20:49Z

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:00:46] configure: rust.quiet-tests := True
---
[00:43:07] F...............................................................................i...................
[00:43:13] .......................i............................................................................
---
[00:43:56] .....................i............................................................................i.
[00:44:02] ....................................................................................................
[00:44:09] ...........ii.......................................................................................
---
[00:44:20] error[E0432]: unresolved import `MY_HEAP`
[00:44:20] --> $DIR/allocator-submodule.rs:22:5
[00:44:20] |
[00:44:20] LL | static MY_HEAP: Global = Global::default(); //~ ERROR
[00:44:20] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `MY_HEAP` in the root
---
[00:44:20] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/allocator-submodule.stderr
[00:44:20] To update references, run this command from build directory:
[00:44:20] /checkout/src/test/ui/update-references.sh '/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui' 'allocator-submodule.rs'
[00:44:20]
[00:44:20] error: 1 errors occurred comparing output.
[00:44:20] status: exit code: 101
[00:44:20] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/allocator-submodule.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/allocator-submodule.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/allocator-submodule.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
---
[00:44:20] {"message":"unresolved import `MY_HEAP`","code":{"code":"E0432","explanation":"\nAn import was unresolved.\n\nErroneous code example:\n\n```compile_fail,E0432\nuse something::Foo; // error: unresolved import `something::Foo`.\n```\n\nPaths in `use` statements are relative to the crate root. To import items\nrelative to the current and parent modules, use the `self::` and `super::`\nprefixes, respectively. Also verify that you didn't misspell the import\nname and that the import exists in the module from where you tried to\nimport it. Example:\n\n```\nuse self::something::Foo; // ok!\n\nmod --explain E0432`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0432`.\n"}
---
[00:44:20] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-
---
$ dmesg | grep -i kill
[ 10.715793] init: failsafe main process (1095) killed by TERM signal

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

rust-highfive · 2018-04-14T20:25:41Z

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:00:50] configure: rust.quiet-tests := True
---
[00:46:36] F...............................................................................i...................
[00:46:43] .......................i............................................................................
---
[00:47:26] .....................i...........................................................................i..
[00:47:32] ....................................................................................................
[00:47:38] ...........ii.......................................................................................
---
[00:47:50] error[E0432]: unresolved import `MY_HEAP`
[00:47:50] --> $DIR/allocator-submodule.rs:22:5
[00:47:50] |
[00:47:50] LL | static MY_HEAP: Global = Global::default(); //~ ERROR
[00:47:50] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `MY_HEAP` in the root
---
[00:47:50] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/allocator-submodule.stderr
[00:47:50] To update references, run this command from build directory:
[00:47:50] /checkout/src/test/ui/update-references.sh '/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui' 'allocator-submodule.rs'
[00:47:50]
[00:47:50] error: 1 errors occurred comparing output.
[00:47:50] status: exit code: 101
[00:47:50] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/allocator-submodule.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/allocator-submodule.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/allocator-submodule.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
---
[00:47:50] {"message":"unresolved import `MY_HEAP`","code":{"code":"E0432","explanation":"\nAn import was unresolved.\n\nErroneous code example:\n\n```compile_fail,E0432\nuse something::Foo; // error: unresolved import `something::Foo`.\n```\n\nPaths in `use` statements are relative to the crate root. To import items\nrelative to the current and parent modules, use the `self::` and `super::`\nprefixes, respectively. Also verify that you didn't misspell the import\nname and that the import exists in the module from where you tried to\nimport it. Example:\n\n```\nuse self::something::Foo; // ok!\n\nmod something {\n pub struct Foo;\n}\n# fn main() {}\n```\n\nOr, if you tried to use a module from an external crate, you may have missed\nthe `extern crate` declaration (which is usually placed in the crate root):\n\n```\nextern crate core; // Required to use the `core` crate\n\nuse core::any;\n# fn main() {}\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/allocator-submodule.rs","byte_start":720,"byte_end":763,"line_start":22,"line_end":22,"column_start":5,"column_end":48,"is_primary":true,"text":[{"text":" static MY_HEAP: Global = Global::default(); //~ ERROR","highlight_start":5,"highlight_end":48}],"label":"no `MY_HEAP` in the root","suggested_replacement":null,"expansion":{"span":{"file_name":"/checkout/src/test/ui/allocator-submodule.rs","byte_start":720,"byte_end":763,"line_start":22,"line_end":22,"column_start":5,"column_end":48,"is_primary":false,"text":[{"text":" static MY_HEAP: Global = Global::default(); //~ ERROR","highlight_start":5,"highlight_end":48}],"label":null,"suggested_replacement":null,"expansion":null},"macro_decl_name":"#[global_allocator]","def_site_span":null}}],"children":[],"rendered":"error[E0432]: unresolved import `MY_HEAP`\n --> /checkout/src/test/ui/allocator-submodule.rs:22:5\n |\nLL | static MY_HEAP: Global = Global::default(); //~ ERROR\n | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `MY_HEAP` in the root\n\n"}
[00:47:50] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due tpython" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:47:50] expected success, got: exit code: 101
[00:47:50]
[00:47:50]
[00:47:50] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:47:50] Build completed unsuccessfully in 0:02:30
[00:47:50] Makefile:58: recipe for target 'check' failed
[00:47:50] make: *** [check] Error 1

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

mark-i-m · 2018-05-10T17:55:18Z

@alexcrichton Sorry for the delay, I was in a bit of a time crunch. Let me check.

(also, I rebased so the branch doesn't fall too far behind, but there are no changes, and it still doesn't work)

rust-highfive · 2018-05-10T18:50:51Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:46:40] [00:46:40] ---- [ui] ui/feature-gate-global_allocator.rs stdout ----
[00:46:40] diff of stderr:
[00:46:40] [00:46:40] - error[E0658]: the `#[global_allocator]` attribute is an experimental feature
[00:46:40] - --> $DIR/feature-gate-global_allocator.rs:11:1
[00:46:40] + error[E0463]: can't find crate for `core`
[00:46:40] + --> $DIR/feature-gate-global_allocator.rs:12:1
[00:46:40] 3 |
[00:46:40] - LL | #[global_allocator] //~ ERROR: attribute is an experimental feature
[00:46:40] - |
[00:46:40] - |
[00:46:40] - = help: add #![feature(global_allocator)] to the crate attributes to enable
[00:46:40] + LL | static A: usize = 0;
[00:46:40] 8 [00:46:40] 9 error: aborting due to previous error
[00:46:40] 10 [00:46:40] ---
[00:46:40] [00:46:40] The actual stderr differed from the expected stderr.
[00:46:40] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/feature-gate-global_allocator.stderr
[00:46:40] To update references, run this command from build directory:
[00:46:40] /checkout/src/test/ui/update-references.sh '/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui' 'feature-gate-global_allocator.rs'
[00:46:40] error: 1 errors occurred comparing output.
[00:46:40] status: exit code: 101
[00:46:40] status: exit code: 101
[00:46:40] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/feature-gate-global_allocator.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/feature-gate-global_allocator.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/feature-gate-global_allocator.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
[00:46:40] ------------------------------------------
[00:46:40] [00:46:40] ------------------------------------------
[00:46:40] stderr:
[00:46:40] stderr:
[00:46:40] ------------------------------------------
[00:46:40] {"message":"can't find crate for `core`","code":{"code":"E0463","explanation":"\nA plugin/crate was declared but cannot be found. Erroneous code example:\n\n```compile_fail,E0463\n#![feature(plugin)]\n#![plugin(cookie_monster)] // error: can't find crate for `cookie_monster`\nextern crate cake_is_a_lie; // error: can't find crate for `cake_is_a_lie`\n```\n\nYou need to link your code to the relevant crate in order to be able to use it\n(through Cargo or the `-L` option of rustc example). Plugins are crates as\nwell, and you link to them the same way.\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/feature-gate-global_allocator.rs","byte_start":535,"byte_end":555,"line_start":12,"line_end":12,"column_start":1,"column_end":21,"is_primary":true,"text":[{"text":"static A: usize = 0;","highlight_start":1,"highlight_end":21}],"label":"can't find crate","suggested_replacement":null,"expansion":null}],"children":[],"rendered":"error[E0463]: can't find crate for `core`\n --> /checkout/src/test/ui/feature-gate-global_allocator.rs:12:1\n |\nLL | static A: usize = 0;\n | ^^^^^^^^^^^^^^^^^^^^ can't find crate\n\n"}
[00:46:40] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:46:40] {"message":"For more information about this error, try `rustc --explain E0463`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0463`.\n"}
[00:46:40] ------------------------------------------
[00:46:40] [00:46:40] thread '[ui] ui/feature-gate-global_allocator.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3033:9
[00:46:40] note: Run with `RUST_BACKTRACE=1` for a backtrace.
---
[00:46:40] [00:46:40] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:492:22
[00:46:40] [00:46:40] [00:46:40] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:46:40] [00:46:40] [00:46:40] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:46:40] Build completed unsuccessfully in 0:02:31
[00:46:40] Build completed unsuccessfully in 0:02:31
[00:46:40] Makefile:58: recipe for target 'check' failed
[00:46:40] make: *** [check] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:08b06da0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

mark-i-m · 2018-05-10T19:01:39Z

@alexcrichton Ok, so here's what I have.

If I don't generate an extern crate core but manually do that in the code to be compiled, it seems to compile and run as expected. Likewise, the expanded code also compiles when I manually add in the extern.

If I generate an extern crate core programmatically instead, I merely get an error:

$ cargo +stage1 expand Compiling my_test v0.1.0 (file:///tmp/my_test)warning: ignoring --out-dir flag due to -o flagwarning: ignoring -C extra-filename flag due to -o flagerror[E0463]: can't find crate for `core` --> src/main.rs:21:5 |21 | static MY_HEAP: MyAlloc = MyAlloc; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crateerror: aborting due to previous errorFor more information about this error, try `rustc --explain E0463`.error: Could not compile `my_test`.To learn more, run the command again with --verbose.cat: /tmp/cargo-expand.UUz20ChlwC2T/expanded: No such file or directory

TimNN · 2018-05-15T17:01:20Z

@alexcrichton: Do you have any ideas about the error above?

alexcrichton · 2018-05-16T14:47:44Z

Ah unfortunately I don't recognize the error nor know the best way to fix it :(

mark-i-m · 2018-05-17T17:32:04Z

Hmm... ok so what would be the best path to take? I'm not really sure that I have the knowledge to proceed productively on this issue, so it might be best to leave this for someone else?

IMHO, we should disallow allocators in submodules for now, making it a hard error. That would unblock stabilization, right? Then, we should leave notes in #44113 for the next intrepid explorer?

alexcrichton · 2018-05-18T20:44:23Z

Yes until this is fixed I think it's fine to just disallow this with a hard error

mark-i-m · 2018-05-19T16:39:53Z

Ok, I will try to implement that this week. Would that be done by just keep track in the Folder if we ever try to fold a submodule?

I will also leaves some comments on the issue about some of the things we tried.

alexcrichton · 2018-05-20T03:02:16Z

I believe so yeah that should work out well enough

shepmaster · 2018-06-02T14:17:55Z

Ping from triage, @mark-i-m ! Will you have time to revisit this soon?

mark-i-m · 2018-06-02T20:08:56Z

@shepmaster Sorry, I'm without a computer for the next few days. If you want to close this to keep the issue tracker clean, feel free. I will try to do it soon.

mark-i-m · 2018-06-02T23:25:21Z

Hmm... I actually did get a chance to revisit this (unexpectedly). I pushed what I have so far.

I'm getting the following errors:

Building stage1 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Compiling core v0.0.0 (file:///nobackup/rust/src/libcore)
Compiling unwind v0.0.0 (file:///nobackup/rust/src/libunwind)
Compiling compiler_builtins v0.0.0 (file:///nobackup/rust/src/rustc/compiler_builtins_shim)
Compiling alloc_jemalloc v0.0.0 (file:///nobackup/rust/src/liballoc_jemalloc)
Compiling rustc_asan v0.0.0 (file:///nobackup/rust/src/librustc_asan)
Compiling rustc_lsan v0.0.0 (file:///nobackup/rust/src/librustc_lsan)
Compiling rustc_msan v0.0.0 (file:///nobackup/rust/src/librustc_msan)
Compiling rustc_tsan v0.0.0 (file:///nobackup/rust/src/librustc_tsan)
Compiling std v0.0.0 (file:///nobackup/rust/src/libstd)
Compiling libc v0.0.0 (file:///nobackup/rust/src/rustc/libc_shim)
Compiling alloc v0.0.0 (file:///nobackup/rust/src/liballoc)
Compiling std_unicode v0.0.0 (file:///nobackup/rust/src/libstd_unicode)
Compiling alloc_system v0.0.0 (file:///nobackup/rust/src/liballoc_system)
Compiling panic_abort v0.0.0 (file:///nobackup/rust/src/libpanic_abort)
Compiling panic_unwind v0.0.0 (file:///nobackup/rust/src/libpanic_unwind)
error: `global_allocator` cannot be used in submodules
--> librustc_tsan/lib.rs:27:1
|
error: `global_allocator` cannot be used in submodules
--> librustc_asan/lib.rs:27:1
|
27 | static ALLOC: System = System;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
27 | static ALLOC: System = System;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to previous error
error: aborting due to previous error
error: Could not compile `rustc_asan`.

I haven't really gotten to investigate further but my guess is that imported crates are treated like submodules? @alexcrichton Any ideas?

emilyalbini · 2018-06-03T08:20:22Z

Ugh, you have to open a new PR since you force-pushed the branch...

mark-i-m · 2018-06-03T12:41:11Z

Really? Why? I do that every time I rebase

TimNN · 2018-06-03T12:44:44Z

Really? Why? I do that every time I rebase

GitHub oddities. Once a PR is closed (which happened above) it cannot be re-opened once there has been a force push to the underlying branch.

mark-i-m · 2018-06-03T12:48:43Z

Oh, good to know...

@alexcrichton

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

rust-highfive assigned nikomatsakis Mar 24, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 24, 2018

alexcrichton assigned alexcrichton and unassigned nikomatsakis Apr 5, 2018

mark-i-m force-pushed the allocator branch from 610d57f to 60fbde7Compare April 14, 2018 02:41

mark-i-m mentioned this pull request Apr 14, 2018
Various rustfmt and commenting changes #49969
Merged

mark-i-m force-pushed the allocator branch from 60fbde7 to 3949da8Compare April 14, 2018 19:34

mark-i-m added 2 commits May 10, 2018 12:52

Add failing allocator submodule test
650d5d9

Still trying to make core work
4d1ebe4

mark-i-m force-pushed the allocator branch from 7a799a4 to 4d1ebe4Compare May 10, 2018 17:53

TimNN added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 15, 2018

emilyalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 28, 2018

emilyalbini closed this Jun 2, 2018

emilyalbini added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 2, 2018

This was referenced Jun 3, 2018
Stabilize GlobalAlloc and #[global_allocator] #51241
Merged
Prohibit global_allocator in submodules #51335
Merged

mark-i-m mentioned this pull request Apr 3, 2019
global_allocator can not not be defined inside module #44113
Closed

Uh oh!

Uh oh!

Conversation

mark-i-m commented Mar 24, 2018

Uh oh!

rust-highfive commented Mar 24, 2018

Uh oh!

bors commented Mar 27, 2018

Uh oh!

nikomatsakis commented Apr 4, 2018

Uh oh!

alexcrichton commented Apr 5, 2018

Uh oh!

mark-i-m commented Apr 5, 2018

Uh oh!

alexcrichton commented Apr 5, 2018

Uh oh!

mark-i-m commented Apr 5, 2018

Uh oh!

alexcrichton commented Apr 5, 2018

Uh oh!

mark-i-m commented Apr 7, 2018

Uh oh!

alexcrichton commented Apr 9, 2018

Uh oh!

mark-i-m commented Apr 11, 2018

Uh oh!

alexcrichton commented Apr 11, 2018

Uh oh!

mark-i-m commented Apr 13, 2018

Uh oh!

alexcrichton commented Apr 13, 2018

Uh oh!

mark-i-m commented Apr 14, 2018

Uh oh!

mark-i-m commented Apr 14, 2018

Uh oh!

alexcrichton commented Apr 14, 2018

Uh oh!

alexcrichton commented Apr 14, 2018

Uh oh!

mark-i-m commented Apr 14, 2018

Uh oh!

mark-i-m commented Apr 14, 2018

Uh oh!

mark-i-m commented Apr 14, 2018

Uh oh!

rust-highfive commented Apr 14, 2018

Uh oh!

rust-highfive commented Apr 14, 2018

Uh oh!

mark-i-m commented May 10, 2018

Uh oh!

rust-highfive commented May 10, 2018

Uh oh!

mark-i-m commented May 10, 2018

Uh oh!

TimNN commented May 15, 2018

Uh oh!

alexcrichton commented May 16, 2018

Uh oh!

mark-i-m commented May 17, 2018

Uh oh!

alexcrichton commented May 18, 2018

Uh oh!

mark-i-m commented May 19, 2018

Uh oh!

alexcrichton commented May 20, 2018

Uh oh!

shepmaster commented Jun 2, 2018

Uh oh!

mark-i-m commented Jun 2, 2018

Uh oh!

mark-i-m commented Jun 2, 2018

Uh oh!

emilyalbini commented Jun 3, 2018

Uh oh!

mark-i-m commented Jun 3, 2018

Uh oh!

TimNN commented Jun 3, 2018