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

Remove some unnecessary things #55

Closed
wants to merge 5 commits into from
Closed

Conversation

m-ou-se
Copy link
Member

@m-ou-se m-ou-se commented Apr 1, 2024

@RalfJung This simplifies a few things, which might help in your adventure of putting this all inside the rust-lang/rust tree. If this is not useful, feel free to just close.

  • Removes the need for the miri-test-libstd feature for core.
  • Removes the need for the miri-test-libstd feature for alloc.
  • Removes fake/core, which seems unnecessary.

This does not remove the need for miri-test-libstd from std, because if we move that lib.rs to a [[test]], we will need to find another way to enable #![feature(rustc_private)] in there. (Not sure why the #![feature(rustc_private)] in the fake deps aren't enough. "Fixing" that could also be a solution, I suppose.)

Copy link
Member

@RalfJung RalfJung Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this still run doctests in libcore? I can't quite see how that would work. If the crate doesn't have a lib.rs it won't find any doctests to run, will it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this still run doctests in libcore?

I was hoping cargo test would run doctests of [[test]]s, but looks like it does not. :(

Copy link
Member

@RalfJung RalfJung Apr 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think only the bin and lib crates get doctested, unfortunately.

[dev-dependencies]
rand = { version = "0.8.5", default-features = false, features = ["alloc"] }
rand_xorshift = "0.3.0"

[[test]]
name = "collectionstests"
name = "alloc"
path = "../library/alloc/src/lib.rs"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that is clever -- so you are basically turning the unit tests into integration tests, but since they are mostly importing things from alloc (which will then be the sysroot crate) that still works? Or why exactly does this work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't. ^^' I merged this with the fake alloc, and now it should work.

@m-ou-se
Copy link
Member Author

m-ou-se commented Apr 1, 2024

Looks like this trick won't work for the doctests. (In part due to rust-lang/cargo#5477) Closing.

@m-ou-se m-ou-se closed this Apr 1, 2024
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.

2 participants