Skip to content

perf(parser): faster checking for invalid modifiers #15717

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.

Sign up for GitHub

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

Merged
graphite-app merged 1 commit into from
Nov 16, 2025
Merged

perf(parser): faster checking for invalid modifiers #15717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions crates/oxc_parser/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,8 +1079,11 @@ pub fn invalid_rest_assignment_target(span: Span) -> OxcDiagnostic {
}

#[cold]
pub fn modifiers_cannot_appear_here(span: Span) -> OxcDiagnostic {
ts_error("1184", "Modifiers cannot appear here.").with_label(span)
pub fn modifiers_cannot_appear_here(
modifier: &Modifier,
_: Option<ModifierFlags>,
) -> OxcDiagnostic {
ts_error("1184", "Modifiers cannot appear here.").with_label(modifier.span)
}

#[cold]
Expand Down
128 changes: 71 additions & 57 deletions crates/oxc_parser/src/js/class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,24 +224,28 @@ impl<'a> ParserImpl<'a> {
/* permit_const_as_modifier */ true,
/* stop_on_start_of_class_static_block */ true,
);
self.verify_modifiers(
&modifiers,
ModifierFlags::all() - ModifierFlags::EXPORT,
false,
diagnostics::cannot_appear_on_class_elements,
);

// static { block }
if self.at(Kind::Static) && self.lexer.peek_token().kind() == Kind::LCurly {
for decorator in decorators {
self.error(diagnostics::decorators_are_not_valid_here(decorator.span));
}
for modifier in modifiers.iter() {
self.error(diagnostics::modifiers_cannot_appear_here(modifier.span));
}
self.verify_modifiers(
&modifiers,
ModifierFlags::empty(),
false,
diagnostics::modifiers_cannot_appear_here,
);
return self.parse_class_static_block(span);
}

self.verify_modifiers(
&modifiers,
!ModifierFlags::EXPORT,
false,
diagnostics::cannot_appear_on_class_elements,
);

let r#abstract = modifiers.contains(ModifierKind::Abstract);

let r#type = if r#abstract {
Expand Down Expand Up @@ -282,22 +286,29 @@ impl<'a> ParserImpl<'a> {
self.error(diagnostics::decorators_are_not_valid_here(decorator.span));
}

let mut has_seen_readonly_modifier = false;
for modifier in modifiers.iter() {
match modifier.kind {
ModifierKind::Readonly => has_seen_readonly_modifier = true,
ModifierKind::Static => {
if has_seen_readonly_modifier {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Readonly,
));
// No modifiers except `static` and `readonly` are valid here, and they must appear in that order
self.verify_modifiers(
&modifiers,
ModifierFlags::READONLY | ModifierFlags::STATIC,
true,
diagnostics::cannot_appear_on_an_index_signature,
);
if modifiers.contains_all_flags(ModifierFlags::READONLY | ModifierFlags::STATIC) {
// Has both `readonly` and `static` modifiers. Make sure `static` comes before `readonly`.
let mut has_seen_readonly_modifier = false;
for modifier in modifiers.iter() {
match modifier.kind {
ModifierKind::Readonly => has_seen_readonly_modifier = true,
ModifierKind::Static => {
if has_seen_readonly_modifier {
self.error(diagnostics::modifier_must_precede_other_modifier(
modifier,
ModifierKind::Readonly,
));
}
}
_ => {}
}
_ => self.error(diagnostics::cannot_appear_on_an_index_signature(
modifier,
Some(ModifierFlags::READONLY | ModifierFlags::STATIC),
)),
}
}

Expand All @@ -322,25 +333,29 @@ impl<'a> ParserImpl<'a> {
}

fn parse_class_element_name(&mut self, modifiers: &Modifiers<'a>) -> (PropertyKey<'a>, bool) {
for modifier in modifiers.iter() {
match modifier.kind {
ModifierKind::Const => {
self.error(diagnostics::const_class_member(modifier.span));
}
ModifierKind::In | ModifierKind::Out => {
self.error(diagnostics::can_only_appear_on_a_type_parameter_of_a_class_interface_or_type_alias(modifier.kind, modifier.span));
self.verify_modifiers(
modifiers,
!(ModifierFlags::CONST | ModifierFlags::IN | ModifierFlags::OUT),
false,
|modifier, _| {
match modifier.kind {
ModifierKind::Const => diagnostics::const_class_member(modifier.span),
ModifierKind::In | ModifierKind::Out => {
diagnostics::can_only_appear_on_a_type_parameter_of_a_class_interface_or_type_alias(modifier.kind, modifier.span)
}
_ => unreachable!(),
}
_ => {}
}
}
},
);

match self.cur_kind() {
Kind::PrivateIdentifier => {
let private_ident = self.parse_private_identifier();
// `private #foo`, etc. is illegal
if self.is_ts {
self.verify_modifiers(
modifiers,
ModifierFlags::all() - ModifierFlags::ACCESSIBILITY,
!ModifierFlags::ACCESSIBILITY,
false,
diagnostics::accessibility_modifier_on_private_property,
);
Expand Down Expand Up @@ -436,7 +451,7 @@ impl<'a> ParserImpl<'a> {
self.check_method_definition_accessor(&method_definition);
self.verify_modifiers(
modifiers,
ModifierFlags::all() - ModifierFlags::ASYNC - ModifierFlags::DECLARE,
!(ModifierFlags::ASYNC | ModifierFlags::DECLARE),
false,
diagnostics::modifier_cannot_be_used_here,
);
Expand Down Expand Up @@ -510,10 +525,23 @@ impl<'a> ParserImpl<'a> {
let optional = optional_span.is_some();

if generator || matches!(self.cur_kind(), Kind::LParen | Kind::LAngle) {
for modifier in modifiers.iter() {
match modifier.kind {
ModifierKind::Declare => {
self.error(diagnostics::cannot_appear_on_class_elements(
self.verify_modifiers(
modifiers,
!(ModifierFlags::DECLARE | ModifierFlags::READONLY),
false,
|modifier, _| match modifier.kind {
ModifierKind::Declare => diagnostics::cannot_appear_on_class_elements(
modifier,
Some(
ModifierFlags::ACCESSIBILITY
| ModifierFlags::STATIC
| ModifierFlags::ABSTRACT
| ModifierFlags::OVERRIDE
| ModifierFlags::ASYNC,
),
),
ModifierKind::Readonly => {
diagnostics::modifier_only_on_property_declaration_or_index_signature(
modifier,
Some(
ModifierFlags::ACCESSIBILITY
Expand All @@ -522,25 +550,11 @@ impl<'a> ParserImpl<'a> {
| ModifierFlags::OVERRIDE
| ModifierFlags::ASYNC,
),
));
)
}
ModifierKind::Readonly => {
self.error(
diagnostics::modifier_only_on_property_declaration_or_index_signature(
modifier,
Some(
ModifierFlags::ACCESSIBILITY
| ModifierFlags::STATIC
| ModifierFlags::ABSTRACT
| ModifierFlags::OVERRIDE
| ModifierFlags::ASYNC,
),
),
);
}
_ => {}
}
}
_ => unreachable!(),
},
);
return self.parse_method_declaration(
span, r#type, generator, name, computed, optional, modifiers, decorators,
);
Expand Down
37 changes: 31 additions & 6 deletions crates/oxc_parser/src/modifiers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,10 @@ impl<'a> Modifiers<'a> {
self.flags.contains(target.into())
}

pub fn contains_all_flags(&self, flags: ModifierFlags) -> bool {
self.flags.contains(flags)
}

pub fn iter(&self) -> impl Iterator<Item = &Modifier> + '_ {
self.modifiers.as_ref().into_iter().flat_map(|modifiers| modifiers.iter())
}
Expand Down Expand Up @@ -495,21 +499,42 @@ impl<'a> ParserImpl<'a> {
}
}

#[inline]
pub(crate) fn verify_modifiers<F>(
&mut self,
modifiers: &Modifiers<'a>,
allowed: ModifierFlags,
// if true, allowed is exact match; if false, allowed is a superset
// used whether to use for the diagnostic message
// If `true`, `allowed` is exact match; if `false`, `allowed` is a superset.
// Used for whether to pass `allowed` to `create_diagnostic` function.
strict: bool,
diagnose: F,
create_diagnostic: F,
) where
F: Fn(&Modifier, Option<ModifierFlags>) -> OxcDiagnostic,
{
for modifier in modifiers.iter() {
if !allowed.contains(modifier.kind.into()) {
self.error(diagnose(modifier, strict.then_some(allowed)));
if modifiers.flags.intersects(!allowed) {
// Invalid modifiers are rare, so handle this case in `#[cold]` function.
// Also `#[inline(never)]` to help `verify_modifiers` to get inlined.
#[cold]
#[inline(never)]
fn report<'a, F>(
parser: &mut ParserImpl<'a>,
modifiers: &Modifiers<'a>,
allowed: ModifierFlags,
strict: bool,
create_diagnostic: F,
) where
F: Fn(&Modifier, Option<ModifierFlags>) -> OxcDiagnostic,
{
let mut found_invalid_modifier = false;
for modifier in modifiers.iter() {
if !allowed.contains(ModifierFlags::from(modifier.kind)) {
parser.error(create_diagnostic(modifier, strict.then_some(allowed)));
found_invalid_modifier = true;
}
}
debug_assert!(found_invalid_modifier);
}
report(self, modifiers, allowed, strict, create_diagnostic);
}
}
}
16 changes: 7 additions & 9 deletions crates/oxc_parser/src/ts/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1195,15 +1195,13 @@ impl<'a> ParserImpl<'a> {

let kind = self.cur_kind();
if kind == Kind::LParen || kind == Kind::LAngle {
for modifier in modifiers.iter() {
if modifier.kind == ModifierKind::Readonly {
self.error(
diagnostics::modifier_only_on_property_declaration_or_index_signature(
modifier, None,
),
);
}
}
self.verify_modifiers(
modifiers,
!ModifierFlags::READONLY,
false,
diagnostics::modifier_only_on_property_declaration_or_index_signature,
);

let type_parameters = self.parse_ts_type_parameters();
let (this_param, params) = self
.parse_formal_parameters(FunctionKind::Declaration, FormalParameterKind::Signature);
Expand Down
Loading