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

0.15 Tracking issue #262

Open
11 of 13 tasks
josephlr opened this issue Jun 4, 2021 · 17 comments
Open
11 of 13 tasks

0.15 Tracking issue #262

josephlr opened this issue Jun 4, 2021 · 17 comments

Comments

@josephlr
Copy link
Contributor

josephlr commented Jun 4, 2021

This issue tracks any breaking changes we want to make for 0.15.

NOTE: All breaking changes should be made against the next branch, this branch should then be merged in right before releasing 0.15. Also, remember to update Changelog.md.

@josephlr
Copy link
Contributor Author

josephlr commented Mar 24, 2022

Note that const_fn_fn_ptr_basics and const_fn_trait_bound were stabilized in 1.61

If we support only 1.61 and later, we could then remove most of our const_fn! from this crate.

@phil-opp @Freax13 would we want increase our minimum rust version to be 1.61 for v0.15.0? Would we feel comfortable increasing the minimum rust version in a patch release? Do we have a policy on this?

josephlr added a commit that referenced this issue Mar 24, 2022
Both `const_fn_fn_ptr_basics` and `const_fn_trait_bound` were stabilized
in Rust relese 1.61. We can't immediately remove the `const_fn!` quite
yet. 1.61 is not yet on stable, and even it were, removing them would
increase our MSRV to 1.61 (which we may or may not want to do in a patch
release).

See also: #262 (comment)

Signed-off-by: Joe Richey <[email protected]>
@phil-opp
Copy link
Member

Ah, that's great! Increasing the minimum Rust version for v0.15 sounds fine to me in general, as it is a semver-incompatible release with breaking changes. However, it's still 8 (?) weeks until Rust 1.61 becomes stable, so requiring it would mean that we have to postpone this release until May if we want it to keep supporting stable Rust. So I think it would be better to do this as part of a future v0.16 release. Until then, we could use the rustversion macro to provide the new const functionality when a new enough compiler is used.

@josephlr
Copy link
Contributor Author

Oh that crate looks amazing, it would let us get rid of our const_fn! macro entirely!!

If we don't need Rust 1.61 for v0.15, then I think we should have the MSRV be 1.56 so that we can use the rust-version field in our crate.

@phil-opp
Copy link
Member

If we don't need Rust 1.61 for v0.15, then I think we should have the MSRV be 1.56 so that we can use the rust-version field in our crate.

Sure, sounds good to me!

@josephlr
Copy link
Contributor Author

josephlr commented Mar 24, 2022

Oh wait we use inline assembly, so the min stable version is already 1.59 if the "instructions" feature is used. We should probably just make that the unconditionally minimum supported version for 0.15

@josephlr
Copy link
Contributor Author

josephlr commented Mar 31, 2022

One thing I was considering for 0.15 were changes to VirtAddr's underlying representation on 64-bit platforms. If we changed VirtAddr to look like:

#[repr(transparent)]
pub struct VirtAddr(*const ());

on 64-bit platforms, we could then have the following const fn methods:

impl VirtAddr {
    // Safety: raw pointer has to be canonical
    #[cfg(target_pointer_width = "64")]
    pub const unsafe fn from_ptr_unsafe<T>(ptr: *const T) -> Self {
        Self(ptr as *const ())
    }

    #[cfg(target_arch = "x86_64")]
    pub const fn from_ref<T>(r: &T) -> Self {
        // Safety: references point to valid data, so are canonical.
        unsafe { Self::from_ptr_unsafe(r) }
    }
}

This would make it possible to construct something like a TaskStateSegment, DescriptorTablePointer, or some other structure containing a VirtAddr at compile time.

The downside of this (other than implementation complexity) is that the following methods would have to be made non-const:

  • VirtAddr::as_u64()
  • VirtAddr::page_offset()
  • VirtAddr::p{1,2,3,4}_index() / VirtAddr::page_table_index()
  • The various alignment methods can't yet const, but making them const might be harder if the underlying datatype is pointer

I'm not sure how bad this downside would be.

@phil-opp
Copy link
Member

We probably need to think about pointer provenance at some point. There is currently an open proposal to make the usize to pointer conversion more strict. See the Rust's Unsafe Pointer Types Need An Overhaul post for some background information.

I'm not sure what the correct approach for our VirtAddr type would be. On one hand, we treat it like a pointer, e.g. in the VirtAddr::as_ptr method. On the other hand, the type should be allowed to cross address space bounds (e.g. kernels that keep track of the stack pointers of preempted threads), which might (?) be invalid under strict provenance rules. So maybe we need to split this type up at some point, e.g. by removing all pointer-related methods form VirtAddr and using pointer types in more places.

Given this issue, I don't think that it's a good idea to switch VirtAddr between pointer-based and u64-based depending on the platform. It would lead to more confusion and less clear semantics.

@Freax13
Copy link
Member

Freax13 commented Mar 31, 2022

Given this issue, I don't think that it's a good idea to switch VirtAddr between pointer-based and u64-based depending on the platform. It would lead to more confusion and less clear semantics.

I agree with that, having some methods only be const on 64-bit targets seems unintuitive. That being said I'd still be interested in potential use cases.

@josephlr
Copy link
Contributor Author

josephlr commented Apr 2, 2022

Those articles are very interesting, and I agree that changing things in this space is really complicated. It definitely seems like keeping VirtAddr as a u64 is the best bet for now.

So maybe we need to split this type up at some point, e.g. by removing all pointer-related methods form VirtAddr and using pointer types in more places.

This is a good idea, I'll see if I can put some examples together in a separate tracking issue.

@josephlr
Copy link
Contributor Author

@phil-opp @Freax13 is there anything else we want to get in before 0.15? All the currently pending PRs are additive, so could be done after we release 0.15

@Freax13
Copy link
Member

Freax13 commented Apr 17, 2022

I can't think of anything.

@phil-opp
Copy link
Member

Me neither!

@phil-opp
Copy link
Member

phil-opp commented Mar 8, 2023

What's the state of this? Should we finally prepare the 0.15 release?

@Freax13
Copy link
Member

Freax13 commented Mar 8, 2023

We could wait for #404, but this also wouldn't be a deal breaker for me, especially because we don't seem to have come to a final design on the changes just yet.

Other than that, I think we're ready.

@Freax13
Copy link
Member

Freax13 commented Mar 8, 2023

I'll get started on rebasing next onto master.

@Freax13
Copy link
Member

Freax13 commented Mar 8, 2023

I'll get started on rebasing next onto master.

Hm, never mind, I forgot we don't do rebases. I'll get started on merging next into master instead.

@Freax13
Copy link
Member

Freax13 commented Mar 8, 2023

I'll get started on rebasing next onto master.

Hm, never mind, I forgot we don't do rebases. I'll get started on merging next into master instead.

I'm done with that. Depending on whether on not we want to wait for some other breaking prs to also be included, I can create a pr.

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 a pull request may close this issue.

3 participants