Skip to content

Re-enable needs_finalize intrinsic #43

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

Merged
merged 3 commits into from
Jan 19, 2022

Conversation

jacob-hughes
Copy link
Collaborator

This fixes the broken implementation of std::mem::needs_finalize() which was disabled because of changes in upstream to the needs_drop implementation in rustc.

This introduces a new module where needs_finalize is tested in isolation instead of being bolted onto the ever-changing needs_drop internals.

@ltratt
Copy link
Member

ltratt commented Jan 14, 2022

Please squash.

@jacob-hughes
Copy link
Collaborator Author

Squashed

@ltratt
Copy link
Member

ltratt commented Jan 14, 2022

bors r+

@bors
Copy link

bors bot commented Jan 14, 2022

Build failed:

@jacob-hughes
Copy link
Collaborator Author

Pushed a rustfmt fix, LMK if you're happy for me to squash.

@ltratt
Copy link
Member

ltratt commented Jan 14, 2022

Please squash.

@jacob-hughes
Copy link
Collaborator Author

Squashed

@ltratt
Copy link
Member

ltratt commented Jan 14, 2022

bors r+

@bors
Copy link

bors bot commented Jan 14, 2022

Build failed:

@jacob-hughes
Copy link
Collaborator Author

Lets give this another kick now that we are dockerized.

bors try

@jacob-hughes
Copy link
Collaborator Author

Am I ok to rebase this with master?

@bors
Copy link

bors bot commented Jan 18, 2022

try

Build failed:

@vext01
Copy link
Member

vext01 commented Jan 18, 2022

Took me a while to spot:

image

@ltratt
Copy link
Member

ltratt commented Jan 18, 2022

@jacob-hughes We've still got a build failure (well, a test failure), so we need to fix that before considering squashing.

@jacob-hughes
Copy link
Collaborator Author

Ok, this test is very irritating because I get the correct output on my Arch machine. Let me see if I can wildcard the broken bit, it's not the relevant part of the test anyway.

@jacob-hughes
Copy link
Collaborator Author

bors try

@bors
Copy link

bors bot commented Jan 18, 2022

try

Build failed:

@jacob-hughes
Copy link
Collaborator Author

bors try

@bors
Copy link

bors bot commented Jan 18, 2022

try

Build failed:

@jacob-hughes
Copy link
Collaborator Author

bors try

@bors
Copy link

bors bot commented Jan 18, 2022

try

Build failed:

@jacob-hughes
Copy link
Collaborator Author

I had a look at using a make test after lunch but it looks like it's going require a bit more effort at the moment. In the meantime I've ignored this test and I'll open an issue so that we can move on. When I squash, I'll make note of this in the commit message.

@ltratt
Copy link
Member

ltratt commented Jan 19, 2022

Please squash.

@jacob-hughes
Copy link
Collaborator Author

Squashed

@ltratt
Copy link
Member

ltratt commented Jan 19, 2022

bors r+

@bors
Copy link

bors bot commented Jan 19, 2022

Build failed:

@jacob-hughes
Copy link
Collaborator Author

bors try

@ltratt
Copy link
Member

ltratt commented Jan 19, 2022

If bors succeeds, then please squash. If it doesn't... don't :)

@bors
Copy link

bors bot commented Jan 19, 2022

try

Build succeeded:

The GcSmartPointer trait was a hangover which let the compiler identify
`Gc<T>` structs when they were defined as part of a non-rustc library.
Now that the `Gc` implementation is part of the standard library, this
is unnecessary and adds complexity.

This requires the prevent_early_finalization mir-opt tests to be
temporarily ignored due to an issue with non-deterministic builds when
bringing in the alloc crate across different machines. [1]

[1]: softdevteam#45
This fixes the broken implementation of `std::mem::needs_finalize()`
which because of changes in upstream to the needs_drop implementation in
rustc.

This introduces a new module where needs_finalize is tested in isolation
instead of being bolted onto the ever-changing needs_drop internals.
@jacob-hughes
Copy link
Collaborator Author

Squashed

@ltratt
Copy link
Member

ltratt commented Jan 19, 2022

bors r+

@bors
Copy link

bors bot commented Jan 19, 2022

Build succeeded:

@bors bors bot merged commit 5b091ae into softdevteam:master Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants