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

Add a parser for flags formatted as bar-separated-values #297

Merged
merged 16 commits into from Feb 8, 2023

Conversation

KodrAus
Copy link
Member

@KodrAus KodrAus commented Feb 2, 2023

The last blocker to #262

This PR adds a parser for flags formatted using the default Debug impl, like:

A | B | C | 0x0e

It also updates the default serde implementations to use the text format and parser for human-readable formats.

I'll update this PR with more details once I've got the parser properly written and tested.

@KodrAus KodrAus mentioned this pull request Feb 2, 2023
@KodrAus KodrAus marked this pull request as ready for review February 2, 2023 22:15
src/error.rs Outdated Show resolved Hide resolved
src/internal.rs Show resolved Hide resolved
@KodrAus KodrAus mentioned this pull request Feb 2, 2023
examples/serde_compat.rs Outdated Show resolved Hide resolved
@KodrAus
Copy link
Member Author

KodrAus commented Feb 6, 2023

One reasonable enhancement we could consider in the future is introducing a trait for parsing:

pub trait Parser {
    type Bits;

    fn name(&mut self, flag: &str) -> Result<Self::Bits, ParseError>;
    fn hex(&mut self, flag: &str) -> Result<Self::Bits, ParseError>;

    fn whitespace(&mut self, ws: &str) -> Result<(), ParseError> {
        Ok(())
    }

    fn separator(&mut self, separator: &str) -> Result<(), ParseError> {
        Ok(())
    }
}

We could then add a method to our generated internal type (not the public one) like:

pub fn parse(parser: &mut impl Parser<$T>) -> Result<Self, ParseError> { .. }

Then, end-users could write their own implementation of a parser that could ignore invalid flags, or treat them as case-insensitive.

@KodrAus
Copy link
Member Author

KodrAus commented Feb 8, 2023

I've published a library that maintains the serde compatibility functions: https://github.com/KodrAus/bitflags-serde-legacy/

@KodrAus
Copy link
Member Author

KodrAus commented Feb 8, 2023

Last thing I think we should do before merging is run some benchmarks, just to make sure that the cost of parsing doesn't increase too badly with the number of flags.

@KodrAus
Copy link
Member Author

KodrAus commented Feb 8, 2023

Some very simple benchmarking suggests around 10ns parsing per flag on a flags type with 10 flags. There would be plenty of room to improve this, but it doesn't look like we've got a risk of exploding parsing time out.

@KodrAus KodrAus merged commit 36d3c01 into bitflags:main Feb 8, 2023
facebook-github-bot pushed a commit to facebook/ocamlrep that referenced this pull request Jan 16, 2024
Summary:
## Motivation

Since the latest compiler update, we are getting `clippy::bad_bit_mask` errors at the callsites of `bitflags!` macros where one of the variant is zero.
[Upstream won't address it in the `1.x` branch](bitflags/bitflags#373) and recommends upgrading to the `2.x` branch.

We are very close to reaching **zero clippy lints** in [Mononoke and other servers](https://fburl.com/code/pd76yn5e), which would feel nice.

## Specific categories of changes (in case it helps with the code review)

The change from `1.x` to `2.x` introduces a number  of backward compatibility breakages which I had to workaround in our codebase.
See [the release notes for 2.0](https://github.com/bitflags/bitflags/releases/tag/2.0.0) for the explanation for the manual fixes I had to perform at each call site.
 ---
**Adding traits to derive:**
```
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
```
> Generated flags types now derive fewer traits. If you need to maintain backwards compatibility, you can derive the following yourself:
 ---
**Replacing read uses of `.bits` with `.bits()`**
> You can now use the .bits() method instead of the old .bits.
> The representation of generated flags types has changed from a struct with the single field bits to a newtype.
 ---
**Replacing raw setting of `.bits` with `.from_bits_retain()`**
Due to the point above, the representation of the type is not exposed anymore. From [the documentation](https://docs.rs/bitflags/latest/bitflags/example_generated/struct.Flags.html#method.from_bits_retain), `from_bits_retain` "convert from a bits value exactly", which matches the old behaviour
 ---
**Replacing the unsafe `from_bits_unchecked` method with `from_bits_retain`**
> The unsafe from_bits_unchecked method is now a safe from_bits_retain method.
 ---
**Extracting some structs outside of the `bitflags!` macro**
Apart from the derives that `bitflags` knows about, such as `serde`, `bitflags` now can't deal with custom derives in macros with the previous syntax. I followed the recommendation from [their documentation](https://docs.rs/bitflags/latest/bitflags/index.html#custom-derives) to declare the `struct` ahead of of the macro call and only declare the `impl` block inside the macro.
 ---
**Changes to test output**
This does not stand out in the release notes, but as of [this upstream PR](bitflags/bitflags#297), the `Debug` output of generated bitflags has changed. This means any tests that rely on this (and of course, there are a few) needed updating.
In particular, the `vespa` tests rely on that output in a non-obvious way. You might have to trust me (and CI) on these ones...

Reviewed By: dtolnay

Differential Revision: D49742979

fbshipit-source-id: c818c37af45f0964e8fdb7ec6173ad66bb982c00
facebook-github-bot pushed a commit to facebookexperimental/reverie that referenced this pull request Jan 16, 2024
Summary:
## Motivation

Since the latest compiler update, we are getting `clippy::bad_bit_mask` errors at the callsites of `bitflags!` macros where one of the variant is zero.
[Upstream won't address it in the `1.x` branch](bitflags/bitflags#373) and recommends upgrading to the `2.x` branch.

We are very close to reaching **zero clippy lints** in [Mononoke and other servers](https://fburl.com/code/pd76yn5e), which would feel nice.

## Specific categories of changes (in case it helps with the code review)

The change from `1.x` to `2.x` introduces a number  of backward compatibility breakages which I had to workaround in our codebase.
See [the release notes for 2.0](https://github.com/bitflags/bitflags/releases/tag/2.0.0) for the explanation for the manual fixes I had to perform at each call site.
 ---
**Adding traits to derive:**
```
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
```
> Generated flags types now derive fewer traits. If you need to maintain backwards compatibility, you can derive the following yourself:
 ---
**Replacing read uses of `.bits` with `.bits()`**
> You can now use the .bits() method instead of the old .bits.
> The representation of generated flags types has changed from a struct with the single field bits to a newtype.
 ---
**Replacing raw setting of `.bits` with `.from_bits_retain()`**
Due to the point above, the representation of the type is not exposed anymore. From [the documentation](https://docs.rs/bitflags/latest/bitflags/example_generated/struct.Flags.html#method.from_bits_retain), `from_bits_retain` "convert from a bits value exactly", which matches the old behaviour
 ---
**Replacing the unsafe `from_bits_unchecked` method with `from_bits_retain`**
> The unsafe from_bits_unchecked method is now a safe from_bits_retain method.
 ---
**Extracting some structs outside of the `bitflags!` macro**
Apart from the derives that `bitflags` knows about, such as `serde`, `bitflags` now can't deal with custom derives in macros with the previous syntax. I followed the recommendation from [their documentation](https://docs.rs/bitflags/latest/bitflags/index.html#custom-derives) to declare the `struct` ahead of of the macro call and only declare the `impl` block inside the macro.
 ---
**Changes to test output**
This does not stand out in the release notes, but as of [this upstream PR](bitflags/bitflags#297), the `Debug` output of generated bitflags has changed. This means any tests that rely on this (and of course, there are a few) needed updating.
In particular, the `vespa` tests rely on that output in a non-obvious way. You might have to trust me (and CI) on these ones...

Reviewed By: dtolnay

Differential Revision: D49742979

fbshipit-source-id: c818c37af45f0964e8fdb7ec6173ad66bb982c00
facebook-github-bot pushed a commit to facebook/errpy that referenced this pull request Jan 16, 2024
Summary:
## Motivation

Since the latest compiler update, we are getting `clippy::bad_bit_mask` errors at the callsites of `bitflags!` macros where one of the variant is zero.
[Upstream won't address it in the `1.x` branch](bitflags/bitflags#373) and recommends upgrading to the `2.x` branch.

We are very close to reaching **zero clippy lints** in [Mononoke and other servers](https://fburl.com/code/pd76yn5e), which would feel nice.

## Specific categories of changes (in case it helps with the code review)

The change from `1.x` to `2.x` introduces a number  of backward compatibility breakages which I had to workaround in our codebase.
See [the release notes for 2.0](https://github.com/bitflags/bitflags/releases/tag/2.0.0) for the explanation for the manual fixes I had to perform at each call site.
 ---
**Adding traits to derive:**
```
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
```
> Generated flags types now derive fewer traits. If you need to maintain backwards compatibility, you can derive the following yourself:
 ---
**Replacing read uses of `.bits` with `.bits()`**
> You can now use the .bits() method instead of the old .bits.
> The representation of generated flags types has changed from a struct with the single field bits to a newtype.
 ---
**Replacing raw setting of `.bits` with `.from_bits_retain()`**
Due to the point above, the representation of the type is not exposed anymore. From [the documentation](https://docs.rs/bitflags/latest/bitflags/example_generated/struct.Flags.html#method.from_bits_retain), `from_bits_retain` "convert from a bits value exactly", which matches the old behaviour
 ---
**Replacing the unsafe `from_bits_unchecked` method with `from_bits_retain`**
> The unsafe from_bits_unchecked method is now a safe from_bits_retain method.
 ---
**Extracting some structs outside of the `bitflags!` macro**
Apart from the derives that `bitflags` knows about, such as `serde`, `bitflags` now can't deal with custom derives in macros with the previous syntax. I followed the recommendation from [their documentation](https://docs.rs/bitflags/latest/bitflags/index.html#custom-derives) to declare the `struct` ahead of of the macro call and only declare the `impl` block inside the macro.
 ---
**Changes to test output**
This does not stand out in the release notes, but as of [this upstream PR](bitflags/bitflags#297), the `Debug` output of generated bitflags has changed. This means any tests that rely on this (and of course, there are a few) needed updating.
In particular, the `vespa` tests rely on that output in a non-obvious way. You might have to trust me (and CI) on these ones...

Reviewed By: dtolnay

Differential Revision: D49742979

fbshipit-source-id: c818c37af45f0964e8fdb7ec6173ad66bb982c00
facebook-github-bot pushed a commit to facebookincubator/antlir that referenced this pull request Jan 16, 2024
Summary:
## Motivation

Since the latest compiler update, we are getting `clippy::bad_bit_mask` errors at the callsites of `bitflags!` macros where one of the variant is zero.
[Upstream won't address it in the `1.x` branch](bitflags/bitflags#373) and recommends upgrading to the `2.x` branch.

We are very close to reaching **zero clippy lints** in [Mononoke and other servers](https://fburl.com/code/pd76yn5e), which would feel nice.

## Specific categories of changes (in case it helps with the code review)

The change from `1.x` to `2.x` introduces a number  of backward compatibility breakages which I had to workaround in our codebase.
See [the release notes for 2.0](https://github.com/bitflags/bitflags/releases/tag/2.0.0) for the explanation for the manual fixes I had to perform at each call site.
 ---
**Adding traits to derive:**
```
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
```
> Generated flags types now derive fewer traits. If you need to maintain backwards compatibility, you can derive the following yourself:
 ---
**Replacing read uses of `.bits` with `.bits()`**
> You can now use the .bits() method instead of the old .bits.
> The representation of generated flags types has changed from a struct with the single field bits to a newtype.
 ---
**Replacing raw setting of `.bits` with `.from_bits_retain()`**
Due to the point above, the representation of the type is not exposed anymore. From [the documentation](https://docs.rs/bitflags/latest/bitflags/example_generated/struct.Flags.html#method.from_bits_retain), `from_bits_retain` "convert from a bits value exactly", which matches the old behaviour
 ---
**Replacing the unsafe `from_bits_unchecked` method with `from_bits_retain`**
> The unsafe from_bits_unchecked method is now a safe from_bits_retain method.
 ---
**Extracting some structs outside of the `bitflags!` macro**
Apart from the derives that `bitflags` knows about, such as `serde`, `bitflags` now can't deal with custom derives in macros with the previous syntax. I followed the recommendation from [their documentation](https://docs.rs/bitflags/latest/bitflags/index.html#custom-derives) to declare the `struct` ahead of of the macro call and only declare the `impl` block inside the macro.
 ---
**Changes to test output**
This does not stand out in the release notes, but as of [this upstream PR](bitflags/bitflags#297), the `Debug` output of generated bitflags has changed. This means any tests that rely on this (and of course, there are a few) needed updating.
In particular, the `vespa` tests rely on that output in a non-obvious way. You might have to trust me (and CI) on these ones...

Test Plan: dall_e_sandcastle_mondrian_style

Reviewed By: dtolnay

Differential Revision: D49742979

fbshipit-source-id: c818c37af45f0964e8fdb7ec6173ad66bb982c00
facebook-github-bot pushed a commit to facebookexperimental/hermit that referenced this pull request Jan 16, 2024
Summary:
## Motivation

Since the latest compiler update, we are getting `clippy::bad_bit_mask` errors at the callsites of `bitflags!` macros where one of the variant is zero.
[Upstream won't address it in the `1.x` branch](bitflags/bitflags#373) and recommends upgrading to the `2.x` branch.

We are very close to reaching **zero clippy lints** in [Mononoke and other servers](https://fburl.com/code/pd76yn5e), which would feel nice.

## Specific categories of changes (in case it helps with the code review)

The change from `1.x` to `2.x` introduces a number  of backward compatibility breakages which I had to workaround in our codebase.
See [the release notes for 2.0](https://github.com/bitflags/bitflags/releases/tag/2.0.0) for the explanation for the manual fixes I had to perform at each call site.
 ---
**Adding traits to derive:**
```
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
```
> Generated flags types now derive fewer traits. If you need to maintain backwards compatibility, you can derive the following yourself:
 ---
**Replacing read uses of `.bits` with `.bits()`**
> You can now use the .bits() method instead of the old .bits.
> The representation of generated flags types has changed from a struct with the single field bits to a newtype.
 ---
**Replacing raw setting of `.bits` with `.from_bits_retain()`**
Due to the point above, the representation of the type is not exposed anymore. From [the documentation](https://docs.rs/bitflags/latest/bitflags/example_generated/struct.Flags.html#method.from_bits_retain), `from_bits_retain` "convert from a bits value exactly", which matches the old behaviour
 ---
**Replacing the unsafe `from_bits_unchecked` method with `from_bits_retain`**
> The unsafe from_bits_unchecked method is now a safe from_bits_retain method.
 ---
**Extracting some structs outside of the `bitflags!` macro**
Apart from the derives that `bitflags` knows about, such as `serde`, `bitflags` now can't deal with custom derives in macros with the previous syntax. I followed the recommendation from [their documentation](https://docs.rs/bitflags/latest/bitflags/index.html#custom-derives) to declare the `struct` ahead of of the macro call and only declare the `impl` block inside the macro.
 ---
**Changes to test output**
This does not stand out in the release notes, but as of [this upstream PR](bitflags/bitflags#297), the `Debug` output of generated bitflags has changed. This means any tests that rely on this (and of course, there are a few) needed updating.
In particular, the `vespa` tests rely on that output in a non-obvious way. You might have to trust me (and CI) on these ones...

Reviewed By: dtolnay

Differential Revision: D49742979

fbshipit-source-id: c818c37af45f0964e8fdb7ec6173ad66bb982c00
facebook-github-bot pushed a commit to facebook/hhvm that referenced this pull request Jan 16, 2024
Summary:
## Motivation

Since the latest compiler update, we are getting `clippy::bad_bit_mask` errors at the callsites of `bitflags!` macros where one of the variant is zero.
[Upstream won't address it in the `1.x` branch](bitflags/bitflags#373) and recommends upgrading to the `2.x` branch.

We are very close to reaching **zero clippy lints** in [Mononoke and other servers](https://fburl.com/code/pd76yn5e), which would feel nice.

## Specific categories of changes (in case it helps with the code review)

The change from `1.x` to `2.x` introduces a number  of backward compatibility breakages which I had to workaround in our codebase.
See [the release notes for 2.0](https://github.com/bitflags/bitflags/releases/tag/2.0.0) for the explanation for the manual fixes I had to perform at each call site.
 ---
**Adding traits to derive:**
```
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
```
> Generated flags types now derive fewer traits. If you need to maintain backwards compatibility, you can derive the following yourself:
 ---
**Replacing read uses of `.bits` with `.bits()`**
> You can now use the .bits() method instead of the old .bits.
> The representation of generated flags types has changed from a struct with the single field bits to a newtype.
 ---
**Replacing raw setting of `.bits` with `.from_bits_retain()`**
Due to the point above, the representation of the type is not exposed anymore. From [the documentation](https://docs.rs/bitflags/latest/bitflags/example_generated/struct.Flags.html#method.from_bits_retain), `from_bits_retain` "convert from a bits value exactly", which matches the old behaviour
 ---
**Replacing the unsafe `from_bits_unchecked` method with `from_bits_retain`**
> The unsafe from_bits_unchecked method is now a safe from_bits_retain method.
 ---
**Extracting some structs outside of the `bitflags!` macro**
Apart from the derives that `bitflags` knows about, such as `serde`, `bitflags` now can't deal with custom derives in macros with the previous syntax. I followed the recommendation from [their documentation](https://docs.rs/bitflags/latest/bitflags/index.html#custom-derives) to declare the `struct` ahead of of the macro call and only declare the `impl` block inside the macro.
 ---
**Changes to test output**
This does not stand out in the release notes, but as of [this upstream PR](bitflags/bitflags#297), the `Debug` output of generated bitflags has changed. This means any tests that rely on this (and of course, there are a few) needed updating.
In particular, the `vespa` tests rely on that output in a non-obvious way. You might have to trust me (and CI) on these ones...

Reviewed By: dtolnay

Differential Revision: D49742979

fbshipit-source-id: c818c37af45f0964e8fdb7ec6173ad66bb982c00
facebook-github-bot pushed a commit to facebookincubator/below that referenced this pull request Jan 16, 2024
Summary:
## Motivation

Since the latest compiler update, we are getting `clippy::bad_bit_mask` errors at the callsites of `bitflags!` macros where one of the variant is zero.
[Upstream won't address it in the `1.x` branch](bitflags/bitflags#373) and recommends upgrading to the `2.x` branch.

We are very close to reaching **zero clippy lints** in [Mononoke and other servers](https://fburl.com/code/pd76yn5e), which would feel nice.

## Specific categories of changes (in case it helps with the code review)

The change from `1.x` to `2.x` introduces a number  of backward compatibility breakages which I had to workaround in our codebase.
See [the release notes for 2.0](https://github.com/bitflags/bitflags/releases/tag/2.0.0) for the explanation for the manual fixes I had to perform at each call site.
 ---
**Adding traits to derive:**
```
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
```
> Generated flags types now derive fewer traits. If you need to maintain backwards compatibility, you can derive the following yourself:
 ---
**Replacing read uses of `.bits` with `.bits()`**
> You can now use the .bits() method instead of the old .bits.
> The representation of generated flags types has changed from a struct with the single field bits to a newtype.
 ---
**Replacing raw setting of `.bits` with `.from_bits_retain()`**
Due to the point above, the representation of the type is not exposed anymore. From [the documentation](https://docs.rs/bitflags/latest/bitflags/example_generated/struct.Flags.html#method.from_bits_retain), `from_bits_retain` "convert from a bits value exactly", which matches the old behaviour
 ---
**Replacing the unsafe `from_bits_unchecked` method with `from_bits_retain`**
> The unsafe from_bits_unchecked method is now a safe from_bits_retain method.
 ---
**Extracting some structs outside of the `bitflags!` macro**
Apart from the derives that `bitflags` knows about, such as `serde`, `bitflags` now can't deal with custom derives in macros with the previous syntax. I followed the recommendation from [their documentation](https://docs.rs/bitflags/latest/bitflags/index.html#custom-derives) to declare the `struct` ahead of of the macro call and only declare the `impl` block inside the macro.
 ---
**Changes to test output**
This does not stand out in the release notes, but as of [this upstream PR](bitflags/bitflags#297), the `Debug` output of generated bitflags has changed. This means any tests that rely on this (and of course, there are a few) needed updating.
In particular, the `vespa` tests rely on that output in a non-obvious way. You might have to trust me (and CI) on these ones...

Reviewed By: dtolnay

Differential Revision: D49742979

fbshipit-source-id: c818c37af45f0964e8fdb7ec6173ad66bb982c00
facebook-github-bot pushed a commit to facebook/sapling that referenced this pull request Jan 16, 2024
Summary:
## Motivation

Since the latest compiler update, we are getting `clippy::bad_bit_mask` errors at the callsites of `bitflags!` macros where one of the variant is zero.
[Upstream won't address it in the `1.x` branch](bitflags/bitflags#373) and recommends upgrading to the `2.x` branch.

We are very close to reaching **zero clippy lints** in [Mononoke and other servers](https://fburl.com/code/pd76yn5e), which would feel nice.

## Specific categories of changes (in case it helps with the code review)

The change from `1.x` to `2.x` introduces a number  of backward compatibility breakages which I had to workaround in our codebase.
See [the release notes for 2.0](https://github.com/bitflags/bitflags/releases/tag/2.0.0) for the explanation for the manual fixes I had to perform at each call site.
 ---
**Adding traits to derive:**
```
#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Clone, Copy)]
```
> Generated flags types now derive fewer traits. If you need to maintain backwards compatibility, you can derive the following yourself:
 ---
**Replacing read uses of `.bits` with `.bits()`**
> You can now use the .bits() method instead of the old .bits.
> The representation of generated flags types has changed from a struct with the single field bits to a newtype.
 ---
**Replacing raw setting of `.bits` with `.from_bits_retain()`**
Due to the point above, the representation of the type is not exposed anymore. From [the documentation](https://docs.rs/bitflags/latest/bitflags/example_generated/struct.Flags.html#method.from_bits_retain), `from_bits_retain` "convert from a bits value exactly", which matches the old behaviour
 ---
**Replacing the unsafe `from_bits_unchecked` method with `from_bits_retain`**
> The unsafe from_bits_unchecked method is now a safe from_bits_retain method.
 ---
**Extracting some structs outside of the `bitflags!` macro**
Apart from the derives that `bitflags` knows about, such as `serde`, `bitflags` now can't deal with custom derives in macros with the previous syntax. I followed the recommendation from [their documentation](https://docs.rs/bitflags/latest/bitflags/index.html#custom-derives) to declare the `struct` ahead of of the macro call and only declare the `impl` block inside the macro.
 ---
**Changes to test output**
This does not stand out in the release notes, but as of [this upstream PR](bitflags/bitflags#297), the `Debug` output of generated bitflags has changed. This means any tests that rely on this (and of course, there are a few) needed updating.
In particular, the `vespa` tests rely on that output in a non-obvious way. You might have to trust me (and CI) on these ones...

Reviewed By: dtolnay

Differential Revision: D49742979

fbshipit-source-id: c818c37af45f0964e8fdb7ec6173ad66bb982c00
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.

None yet

1 participant