softdevteam / yksom

Other
8 stars 6 forks source link

Move from rboehm to libgc. #199

Closed ltratt closed 3 years ago

ltratt commented 3 years ago

This PR tries to move yksom from rboehm to libgc. However it fails to compile with:

error[E0603]: trait `GlobalAlloc` is private
  --> src/lib.rs:23:25
   |
23 | pub use libgc_internal::GlobalAlloc;
   |                         ^^^^^^^^^^^ private trait
   |
note: the trait `GlobalAlloc` is defined here
  --> /home/ltratt/.cargo/git/checkouts/libgc_internal-b80cf56d30ac30df/7e79d84/src/lib.rs:14:36
   |
14 |     alloc::{AllocError, Allocator, GlobalAlloc, Layout},
   |                                    ^^^^^^^^^^^

error[E0603]: trait `GlobalAlloc` is private
  --> src/lib.rs:23:25
   |
23 | pub use libgc_internal::GlobalAlloc;
   |                         ^^^^^^^^^^^ private trait
   |
note: the trait `GlobalAlloc` is defined here
  --> /home/ltratt/.cargo/git/checkouts/libgc_internal-b80cf56d30ac30df/7e79d84/src/lib.rs:14:36
   |
14 |     alloc::{AllocError, Allocator, GlobalAlloc, Layout},
   |                                    ^^^^^^^^^^^

warning: trait objects without an explicit `dyn` are deprecated
   --> src/gc.rs:323:23
    |
323 |         let s1gcd: Gc<T> = s1gc;
    |                       ^ help: use `dyn`: `dyn T`
    |
    = note: `#[warn(bare_trait_objects)]` on by default

warning: trait objects without an explicit `dyn` are deprecated
   --> src/gc.rs:324:23
    |
324 |         let s2gcd: Gc<T> = s2gc;
    |                       ^ help: use `dyn`: `dyn T`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0603`.
error: aborting due to previous error; 2 warnings emitted

For more information about this error, try `rustc --explain E0603`.
error: could not compile `libgc`

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed

I wonder if this is because libgc should be using #[global_allocator] or similar?

jacob-hughes commented 3 years ago

This LGTM. Just a minor comment as the name of the allocator in libgc has changed to remove explicit references to Boehm from the API. Once https://github.com/softdevteam/libgc/pull/38 lands, this should compile.

ltratt commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build failed:

ltratt commented 3 years ago

@jacob-hughes I'm not sure if this still waiting on another PR, or whether I need to change something in here or not. Any pointers?

jacob-hughes commented 3 years ago

Can you force-push a change where this is rebased against the latest libgc master? I think it should work then.

ltratt commented 3 years ago

I'm not sure I know what you mean -- are you suggesting rebasing against another repo?

jacob-hughes commented 3 years ago

Argh, I'm an idiot. this is a yksom PR. Sorry I got confused. Hmm, ok, so libgc should be up to date now. Let me try building it locally and see what's going on.

ltratt commented 3 years ago

OK this now works locally for me. Let's check it works on bors/buildbot!

bors try

bors[bot] commented 3 years ago

try

Build failed:

jacob-hughes commented 3 years ago

I think that just needs rustfmt'ing :crossed_fingers:

ltratt commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build failed:

ltratt commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build failed:

ltratt commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build failed:

jacob-hughes commented 3 years ago

There's a typo in the buildbot script, it should be rustgc instead of rustcgc.

ltratt commented 3 years ago

bors try

bors[bot] commented 3 years ago

try

Build succeeded:

jacob-hughes commented 3 years ago

:tada: Please squash

ltratt commented 3 years ago

Squashed.

jacob-hughes commented 3 years ago

bors r+

bors[bot] commented 3 years ago

Build succeeded:

smarr commented 3 years ago

Hi, does this currently work with externally available versions?

I get the following issue:

error: failed to select a version for `libgc`.
    ... required by package `yksom v0.1.0
versions that meet the requirements `*` are: 0.1.0
the package `yksom` depends on `libgc`, with features: `rustgc` but `libgc` does not have these features.

The relevant Rust versions seem to be:

$ rustc --version
rustc 1.53.0-nightly (361bfce30 2021-04-07)
$ rustup --version
rustup 1.23.1 (3df2264a9 2020-11-30)
$ cargo --version
cargo 1.53.0-nightly (65d57e6f3 2021-04-04)

For more details see:

https://travis-ci.com/github/SOM-st/SOM/jobs/496945019#L254-L258

ltratt commented 3 years ago

yksom is currently broken because of the GC: @jacob-hughes is hopefully unbreaking it soon!

smarr commented 3 years ago

Ok, thanks.