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

Static checking via feature(const_generics) and feature(const_evaluatable_checked) #471

Open
Medowhill opened this issue Apr 1, 2021 · 5 comments
Assignees
Labels
cantfix This cannot be worked on because it can be resolved only after a proper fix in another project

Comments

@Medowhill
Copy link
Collaborator

타입 매개변수를 const_assert! 안에서 사용할 수 없습니다. 이로 인해 const_assert!(mem::size_of::<T>() <= PGSIZE)와 같은 코드를 작성할 수 없고, 어쩔 수 없이 assert!(mem::size_of::<T>() <= PGSIZE)를 사용합니다.

우회책으로는 #468 (comment) 에서 언급한 것처럼 feature(const_generics)feature(const_evaluatable_checked)를 사용하는 방법이 있습니다. 그러나 이 방법을 사용하면 지금은 rv6 컴파일 시에 컴파일러가 패닉합니다.

이후에 컴파일러가 업데이트되어 패닉하는 문제가 해결되면 assert!를 없애고 컴파일 시간에 검사가 이루어지도록 수정해야 합니다.

@travis1829
Copy link
Collaborator

travis1829 commented Apr 2, 2021

추가: 위와 같은 const assert를 이용하면 Lock order을 강제할 수 있을 것 같습니다.
ex)

pub struct Lock<const ORDER: usize, T> { /* Omitted */ }

@travis1829
Copy link
Collaborator

  • release mode로 compile하거나, debug mode에서 incremental = false로 해놓고 compile을 하면 ICE가 발생하지 않는다고 합니다.
  • 아마, 다음과 같이 구현하면 될 것 같습니다.
//! Types that let you compile-time assert in `where` clauses,
//! especially about the input generic parameters.
//!
//! # Example
//! ```rust,no_run
//! # use core::mem;
//! #![feature(const_generics)]
//! #![feature(const_evaluatable_checked)]
//!
//! unsafe fn transmute<T, U>(t: T) -> U
//! where
//!     Assert2<
//!         { mem::size_of::<T>() == mem::size_of::<U>() },
//!         { mem::align_of::<T>() == mem::align_of::<U>() },
//!     >: True
//! {
//!     /* Omitted */
//! }
//! ```

pub struct Assert<const EXPR: bool>;
pub struct Assert2<const EXPR: bool, const EXPR2: bool>;
pub struct Assert3<const EXPR: bool, const EXPR2: bool, const EXPR3: bool>;

pub trait True {}
impl True for Assert<true> {}
impl True for Assert2<true, true> {}
impl True for Assert3<true, true, true> {}

@travis1829
Copy link
Collaborator

travis1829 commented Apr 16, 2021

  • 추가적으로, 이 문제는 근본적으로 function body에 const_assert!을 추가해서는 풀 수 없는 문제인 것 같습니다.
    • function body내에서 const_assert!을 쓸 경우, 그 const_assert!언제나 옳은 경우에만 compile이 성공하게 됩니다.
    • 하지만, generic을 사용하게 되면, const_assert!의 결과가 input generic에 따라 달라지게 됩니다. 그러므로, compiler는 이 const_assert!이 항상 참일지를 function body만 보고서는 알 수 없으므로, 근본적으로 function body내에서는 이런 형태의 const_assert!를 사용할 수 없습니다.
      • 참고로, 이 문제는 const_genericsconst_evaluatable_check를 추가하더라도 해결할 수 없습니다.
  • 그러므로, 이 문제는 위와 같이 function의 signature에 generic T에 대한 const bound를 추가해서만 해결할 수 있는 문제로 보입니다.

@Medowhill Medowhill added the cantfix This cannot be worked on because it can be resolved only after a proper fix in another project label Apr 30, 2021
@jeehoonkang jeehoonkang added future-work Will not be resolved during refactoring; should be dealt with in future work and removed future-work Will not be resolved during refactoring; should be dealt with in future work labels May 21, 2021
@jeehoonkang
Copy link
Member

@travis1829 태우님 안녕하세요, 오랜만입니다 ㅎㅎ 혹시 여유시간이 잠깐 있으시다면, 이 이슈 한번 봐주실 수 있으실까요? 바쁘고 힘든 시기임을 잘 알고 있으니 어렵다면 편하게 알려주세요.

@travis1829
Copy link
Collaborator

@jeehoonkang 결론만 말씀드리자면, 기존에 PR#489를 close without merge했던 이유를 완전히 해결하기는 아직 힘들어보입니다.
다만, ICE가 없어졌으므로 훨씬 깔끔하게는 할 수 있을 것 같습니다.

ex) (where 부분 추가)

pub fn as_uninit_mut<T>(&mut self) -> &mut MaybeUninit<T>
where
	[u8; PGSIZE - mem::size_of::<T>()]:,                // We need mem::size_of::<T>() <= PGSIZE
        [u8; PGSIZE % mem::align_of::<T>() + usize::MAX]:   // We need PGSIZE % mem::align_of::<T> == 0
{
	//...
}

Rust blog 등을 확인해보면, 아직은 위 예처럼 array length부분 등을 이용해 검사를 해보기를 권고하는 것 같습니다.
(추후에 훨찐 직관적인 syntax를 제공하겠다고는 했으나, 아직 안 나온 것 같습니다.)

또는 다음과 같이 자동으로 impl되는 trait을 추가해서 해결할수도 있습니다.
ex)

pub fn as_uninit_mut<T: FromPage>(&mut self) -> &mut MaybeUninit<T> {
	//...
}

/// This trait is auto-implemented for types that suffice the following.
/// * mem::size_of::<T>() <= PGSIZE
/// * PGSIZE % mem::align_of::<T>() == 0
pub trait FromPage {}
impl<T> FromPage for T
where
	[u8; PGSIZE - mem::size_of::<T>()]:,                // We need mem::size_of::<T>() <= PGSIZE
        [u8; PGSIZE % mem::align_of::<T>() + usize::MAX]:   // We need PGSIZE % mem::align_of::<T> == 0
{}

이런식으로 하면 이전 PR때처럼 module을 새로 만들거나 Assert, Assert2 등으로 구분할 필요가 없어지지만, 그래도 여전히 에러 메세지가 less descriptive하다는 문제가 있습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cantfix This cannot be worked on because it can be resolved only after a proper fix in another project
Projects
None yet
3 participants