refactor(parser): simplify ModifierFlags#20738
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Pull request overview
Refactors the parser’s modifier tracking to make ModifierKind the single source of truth by replacing the bitflags!-based ModifierFlags with a compact u16 bitfield wrapper and ModifierKind-centric constructors/query APIs, simplifying call sites and reducing the chance of mismatched flag/kind pairs.
Changes:
- Replaced
bitflags! ModifierFlagswith au16-backedModifierFlagswrapper plusnew/all_except/contains/with/withoutAPIs. - Updated parser call sites to construct/check allowed modifiers via
ModifierKindarrays instead of|,!, andfrom(...)conversions. - Adjusted diagnostics helper text generation and accessor-specific “allowed modifiers” computation to use the new APIs.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/oxc_parser/src/modifiers.rs | Introduces new ModifierFlags bitfield wrapper and updates modifier parsing/verification to use ModifierKind-based operations. |
| crates/oxc_parser/src/diagnostics.rs | Updates allowed-modifier help text logic and accessor modifier diagnostics to use count()/without(). |
| crates/oxc_parser/src/ts/types.rs | Updates verify_modifiers call sites to use ModifierFlags::new / all_except. |
| crates/oxc_parser/src/ts/statement.rs | Updates verify_modifiers call sites to use ModifierKind-based flag construction. |
| crates/oxc_parser/src/js/statement.rs | Updates TS enum const-modifier construction to use ModifierFlags::new([Const]). |
| crates/oxc_parser/src/js/object.rs | Imports ModifierKind and updates async modifier verification to ModifierFlags::new([Async]). |
| crates/oxc_parser/src/js/module.rs | Updates abstract modifier construction for export default class handling. |
| crates/oxc_parser/src/js/function.rs | Updates constructor/function allowed-modifier sets to ModifierFlags::new([...]). |
| crates/oxc_parser/src/js/class.rs | Updates multiple modifier allow/deny sets to new/all_except and refactors one diagnostic helper to a const ALLOWED. |
7f93983 to
e8a9a3c
Compare
09b7760 to
3eb377b
Compare
3eb377b to
a590ebe
Compare
e8a9a3c to
14868fb
Compare
a590ebe to
6587e2d
Compare
6587e2d to
33e028e
Compare
Merge activity
|
The types for modifiers were confusing. We had `ModifierFlags` type defined with `bitflags!` macro, and `ModifierKind` enum which represents the different possible flags. The two types weren't statically related, and consuming code needed to manually relate the two e.g. pairing usage of `ModifierFlags::DECLARE` with `ModifierKind::Declare`. This is unnecessarily complicated, and error-prone. This PR turns `ModifierFlags` into a wrapper around a `u16` and adds methods to construct, query, and set bits in the set by passing a `ModifierKind` or a set of `ModifierKind`s. Consuming code now only has one type to deal with which is the single source of truth about what modifiers are available - `ModifierKind`. Code is more verbose in some places, but personally I think this more explicit style is clearer. `ModifierFlags::new` is a const function, so even sizeable calls e.g. `ModifierFlags::new([ModifierKind::Public, ModifierKind::Private, ModifierKind::Protected])` is boiled down to a static `u16` at compile time. It compiles down to the same as the `bitflags!` version. This change also enables #20742, which improves the storage of `Modifiers`.
33e028e to
5aee74c
Compare
Pure refactor. Rename `ModifierFlags` to `ModifierKinds`. This type represents a collection of `ModifierKind`s, and after #20738 it's no longer implemented as `bitflags!`, so the name is no longer appropriate. This PR is pure renaming - rename the type, and variables that hold the type.

The types for modifiers were confusing. We had
ModifierFlagstype defined withbitflags!macro, andModifierKindenum which represents the different possible flags.The two types weren't statically related, and consuming code needed to manually relate the two e.g. pairing usage of
ModifierFlags::DECLAREwithModifierKind::Declare. This is unnecessarily complicated, and error-prone.This PR turns
ModifierFlagsinto a wrapper around au16and adds methods to construct, query, and set bits in the set by passing aModifierKindor a set ofModifierKinds. Consuming code now only has one type to deal with which is the single source of truth about what modifiers are available -ModifierKind.Code is more verbose in some places, but personally I think this more explicit style is clearer.
ModifierFlags::newis a const function, so even sizeable calls e.g.ModifierFlags::new([ModifierKind::Public, ModifierKind::Private, ModifierKind::Protected])is boiled down to a staticu16at compile time. It compiles down to the same as thebitflags!version.This change also enables #20742, which improves the storage of
Modifiers.