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

refactor(parser): remove extraneous code from regex parsing #2008

Merged
merged 1 commit into from
Jan 13, 2024

Conversation

overlookmotel
Copy link
Contributor

@overlookmotel overlookmotel commented Jan 12, 2024

This PR removes some code in parsing regexp flags which is extraneous:

if !ch.is_ascii_lowercase() {
  self.error(diagnostics::RegExpFlag(ch, self.current_offset()));
  continue;
}

Which is followed by:

let flag = if let Ok(flag) = RegExpFlags::try_from(ch) {
  flag
} else {
  self.error(diagnostics::RegExpFlag(ch, self.current_offset()));
  continue;
};

!ch.is_ascii_lowercase() is equivalent to ch < 'a' || ch > 'z'. The compiler implements RegExpFlags::try_from(ch) as ch < 'd' || ch > 'y' and then a jump table. So ch.is_ascii_lowercase() does nothing that RegExpFlags::try_from(ch) doesn't do already.

https://godbolt.org/z/51GPPY9nx

(this PR built on top of #2007 for ease)

@github-actions github-actions bot added the A-parser Area - Parser label Jan 12, 2024
Copy link

codspeed-hq bot commented Jan 12, 2024

CodSpeed Performance Report

Merging #2008 will not alter performance

Comparing overlookmotel:parser-regex3 (1d6ac30) with main (712e99c)

Summary

✅ 14 untouched benchmarks

@Boshen
Copy link
Member

Boshen commented Jan 13, 2024

I gave you write access so you can use graphite 😄

@Boshen Boshen enabled auto-merge (squash) January 13, 2024 02:31
@Boshen Boshen merged commit 6996948 into oxc-project:main Jan 13, 2024
16 checks passed
@overlookmotel overlookmotel deleted the parser-regex3 branch January 13, 2024 02:40
IWANABETHATGUY pushed a commit to IWANABETHATGUY/oxc that referenced this pull request May 29, 2024
…ect#2008)

This PR removes some code in parsing regexp flags which is extraneous:

```rs
if !ch.is_ascii_lowercase() {
  self.error(diagnostics::RegExpFlag(ch, self.current_offset()));
  continue;
}
```

Which is followed by:

```rs
let flag = if let Ok(flag) = RegExpFlags::try_from(ch) {
  flag
} else {
  self.error(diagnostics::RegExpFlag(ch, self.current_offset()));
  continue;
};
```

`!ch.is_ascii_lowercase()` is equivalent to `ch < 'a' || ch > 'z'`. The
compiler implements `RegExpFlags::try_from(ch)` as `ch < 'd' || ch >
'y'` and then a jump table. So `ch.is_ascii_lowercase()` does nothing
that `RegExpFlags::try_from(ch)` doesn't do already.

https://godbolt.org/z/51GPPY9nx

(this PR built on top of oxc-project#2007 for ease)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parser Area - Parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants