search

Robbepop / modular-bitfield

Macro to generate bitfields for structs that allow for modular use of enums.
Apache License 2.0
155 stars 40 forks source link

Implements visibility for struct and fields, closes #21 #22

Closed WorldSEnder closed 3 years ago

WorldSEnder commented 3 years ago

This first and foremost adds visibility attributes to the declared getters and setters, and the struct itself. It also adds a small piece of documentation to new. Note that new and to_bytes are never exposed to the other modules, that is up to debate, but I wouldn't want to expose private invariants like that. PS: I think one ought to name the getter simply after the field name without the get_ prefix, might as well, since this is a breaking change anyway?

Also updated the expected error messages from the testcases to the newest compiler version and formatted touched files.

Robbepop commented 3 years ago

I just added CI to this project for improved confidence in incoming PRs: https://github.com/Robbepop/modular-bitfield/pull/23 Would be nice if you could rebase on top after it has been merged into master.

WorldSEnder commented 3 years ago

The miri tests are failing, I'm not sure if

https://github.com/Robbepop/modular-bitfield/blob/2266f431e860975787ab04752f5c529b0444aa0e/.github/workflows/rust.yml#L83-L84

should be

          args: test -- -Zmiri-disable-isolation
Robbepop commented 3 years ago

The miri tests are failing, I'm not sure if

https://github.com/Robbepop/modular-bitfield/blob/2266f431e860975787ab04752f5c529b0444aa0e/.github/workflows/rust.yml#L83-L84

should be

          args: test -- -Zmiri-disable-isolation

The modern way of setting these argument is by populating MIRIFLAGS: set -xg MIRIFLAGS "-Zmiri-disable-isolation"; cargo miri test However, this yields yet another error for our UI tests:

test tests ... error: unsupported operation: can't call foreign function: pipe2
  --> /home/robrob/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/unix/pipe.rs:27:26
   |
27 |             cvt(unsafe { libc::pipe2(fds.as_mut_ptr(), libc::O_CLOEXEC) })?;
   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't call foreign function: pipe2
   |
   = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support

So I just guess that our UI tests based on trybuild are beyond unusable with miri.

We might either disable miri again but for now keep in mind that the miri pass is not required for passing our CI.

Robbepop commented 3 years ago

LGTM. Also thanks a lot for the new tests! 👍