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.

Sign up for GitHub

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

Jump to bottom

Move to GitHub merge queues. #228

Merged

vext01 merged 5 commits into softdevteam:master from ltratt:move_to_ghmqs

Jan 17, 2024

Member

ltratt commented Nov 9, 2023

No description provided.


          Move to GitHub merge queues.

9499f0b

ltratt assigned vext01

vext01 added this pull request to the merge queue

github-merge-queue bot removed this pull request from the merge queue due to no response for status checks

vext01 added this pull request to the merge queue

vext01 removed this pull request from the merge queue due to a manual request

vext01 added this pull request to the merge queue

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

vext01 added this pull request to the merge queue


          Use the standard linker rather than forcing gold.

886af63

This setting causes problems and we don't gain sufficient benefit from
it.

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

Member

vext01 commented Nov 9, 2023

Any squashing to be done?

Member Author

ltratt commented Nov 9, 2023

I think the two commits should be separate.

vext01 added this pull request to the merge queue

jacob-hughes removed this pull request from the merge queue due to a manual request

jacob-hughes added this pull request to the merge queue

github-merge-queue bot removed this pull request from the merge queue due to failed status checks


          Update dependencies

8e39124

Contributor

jacob-hughes commented Nov 16, 2023

@vext01 This one should be ready to re-review and merge.

Member

vext01 commented Nov 16, 2023

I think @ltratt is better qualified for yksom changes.

Contributor

jacob-hughes commented Nov 16, 2023

Sure, but I think you'll need to do the final sign off because Laurie opened the PR.

jacob-hughes mentioned this pull request

Add yksom to CI softdevteam/alloy#100

Closed

Member Author

ltratt commented Nov 16, 2023

Yes, @vext01 you need to review this please.

vext01 reviewed

View reviewed changes

src/lib/vm/objects/block.rs

@@ @@ -79,6 +79,8 @@ pub struct Block { @@
               // this explicit impl is needed to tell the compiler we are not doing that.
               unsafe impl FinalizerSafe for Block {}
+              unsafe impl Sync for Block {}

Member

vext01 Nov 17, 2023

Do you need a comment saying why this is safe?

Bearing in mind I don't know much about the GC :)

Contributor

jacob-hughes Nov 17, 2023

Sure! I've documented it here: e43d305

Member

vext01 Nov 17, 2023

Sorry, I meant for the various unsafe impl Sync for Blah {}s.

Member Author

ltratt Nov 18, 2023

AFAICS all of them are now documented.

Member

vext01 commented Nov 18, 2023

Please squash.


          Implement Sync for yksom VM types

676db21

This is needed because their trait objects are passed to a GC so must be
FSA-safe.

jacob-hughes force-pushed the move_to_ghmqs branch from e43d305 to 676db21 Compare

November 20, 2023 09:32

Contributor

jacob-hughes commented Nov 20, 2023

Squashed

vext01 added this pull request to the merge queue

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

Contributor

jacob-hughes commented Nov 21, 2023

Ok so when softdevteam/alloy#101 lands, this should be merge-able :)

ltratt added this pull request to the merge queue

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

vext01 added this pull request to the merge queue

vext01 removed this pull request from the merge queue due to a manual request

Member

vext01 commented Nov 21, 2023

sorry, i didn't see it had failed.

Contributor

jacob-hughes commented Nov 21, 2023 •

edited

Loading

It's a rustfmt problem. I've fixed it here: 9aa838, let me know if I can squash.

Member

vext01 commented Nov 21, 2023

Go ahead.

jacob-hughes force-pushed the move_to_ghmqs branch from 9aa838a to 4b540c3 Compare

November 21, 2023 15:54

Contributor

jacob-hughes commented Nov 21, 2023

Squashed

vext01 added this pull request to the merge queue

Member

vext01 commented Nov 21, 2023

Did you forget to sign the commit?

vext01 removed this pull request from the merge queue due to a manual request

Contributor

jacob-hughes commented Nov 21, 2023

Weird! My gpg-agent has failed. Can I force-push an update?

Member

vext01 commented Nov 21, 2023

Please push a signed commit.


          Remove use of SmartString

0e4445f

The SmartString library uses pointer obfuscation tricks internally which
hide GC pointers from alloy. We must therefore remove it as it is
incompatible with conservative pointer scanning.

jacob-hughes force-pushed the move_to_ghmqs branch from 4b540c3 to 0e4445f Compare

November 21, 2023 16:14

Contributor

jacob-hughes commented Nov 21, 2023

Force-pushed a signed commit.

vext01 added this pull request to the merge queue

github-merge-queue bot removed this pull request from the merge queue due to failed status checks

Contributor

jacob-hughes commented Nov 21, 2023

Ugh I’ve not seen that one before.

Member

vext01 commented Nov 22, 2023

16:41:47 thread 'main' panicked at src/lib/vm/core.rs:404:13:
16:41:47 Not enough stack space to execute block.

ouch.

Member

vext01 commented Dec 19, 2023

Any news on this one?

Member Author

ltratt commented Dec 19, 2023

That's one for @jacob-hughes.

Contributor

jacob-hughes commented Jan 17, 2024

@vext01 You should be able to merge this one now :)

vext01 added this pull request to the merge queue

Merged via the queue into softdevteam:master with commit 5dc8566

2 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet