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

Reimplement status codes by hand #321

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

einarmo
Copy link
Contributor

@einarmo einarmo commented Apr 16, 2024

Instead of using bitflags, this reimplements the status code struct by hand. Using bitflags has some unfortunate side effects in that it makes the debug implementation completely wrong. Instead of logging the name of the status code, bitflags usually produces a weird mix of other status codes, probably because the different status codes overlap. This is very inconvenient when debugging, as you will often just unwrap errors.

Instead of using a JS script for codegen, this generates the status code implementation from the CSV inline. While putting the CSV in rust code is slightly inconvenient, it does mean that the code is more self-contained. Implementation wise it is a pair of not-actually-that-hairy macros (no recursion, which is what tends to make these macros hard to work with).

I tried to do this without too many breaking changes, and the effect on the rest of the codebase is minimal. The implementation is now all in a single file, which is manageable because the generated code isn't committed. There are some changes to note:

  • Setting flags with the | operator is no longer supported, since we're not using bitflags. Instead use the various set_ methods to set parts of the status code.
  • Same with getting parts of the status code, use getters to fetch the status code components.

Instead of using bitflags, this reimplements the status code struct by
hand. Using bitflags has some unfortunate side effects in that it makes
the debug implementation completely wrong. This way we get a much richer
implementation of the status code, which is also more correct.

This includes a validate method, which might be handy, but it is not enforced
anywhere.

The implementation uses some relatively simple declarative macros, so
the old JS script used for codegen is removed.
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.

1 participant