Closed Bug 1642493 Opened 8 months ago Closed 8 months ago

Irregexp: Tidy up loose ends


(Core :: JavaScript Engine, task, P1)




Tracking Status
firefox79 --- fixed


(Reporter: iain, Assigned: iain)




(9 files)

Now that the new engine has landed and made it into 78, we don't need the old engine any more, and we can clean up a variety of small things.

I wrote this testcase while fixing an issue with upstream V8's handling of non-unicode ignoreCase RegExps. (See here:

I'm finally getting around to landing it.

(There will be another patch in the future to fix an equivalent issue with back-references, but it's waiting on upstream review.)

I wrote this testcase while trying to figure out the spec for named captures, and discovered that V8 was not handling it properly. Our implementation does the right thing here, so I'm landing this testcase to make sure it stays that way.

Depends on D77725

This code no longer sparks joy.

Depends on D77726

I kept a running list of code that would be dead once we removed the old engine. This is what was on that list.

Note: The regexp_parse tests used a testing function that parsed a regexp and returned an AST to compare against the expected AST. They are being deleted because the tests are fragile in the face of upstream changes, not particularly useful, and a lot of work to convert.

Depends on D77727

It's not new- any more.

This patch moves the files, replaces new-regexp with irregexp throughout the code, re-alphabetizes #includes, and adds a few missing #includes to RegExpTypes.h and util/vector.h that were exposed by the reordering.

Depends on D77728

This is the only place where any code outside the irregexp directory directly includes a V8 header. Tidying it up makes the next patch easier.

Depends on D77729

I've been thinking about doing this for a while, and finally got around to doing it.

This patch moves the files that are directly imported from V8 into their own subdirectory of irregexp, to make it more obvious which code is part of V8 and which code is part of the shim. I also took the opportunity to rename the shim files to follow our naming conventions (FileName.cpp) instead of V8's (

Most of this code is mechanical. The mildly interesting parts are:

  1. In, when figuring out where the regexp shim include should go, we no longer have to work quite as hard to alphabetize it, because irregexp/RegExpShim.h will always come after irregexp/imported/*. Instead, we just keep scanning until we find a newline.

  2. Some of the V8 code #includes "regexp-macro-assembler-arch.h". Up until now, that's the name we used for the header file for our RegExpMacroAssembler implementation. Now that the implementation lives in a separate directory from the V8 code, I've renamed the header to RegExpNativeMacroAssembler.h (to match the .cpp file), and added a stub regexp-macro-assembler-arch.h header in imported that #includes the real thing.

In the next patch, we take advantage of the new directory structure to get more precise with clang-format-ignore.

Depends on D77730

Now this code is less of a weird outlier.

Depends on D77731

The code that was previously in new-regexp has been moved into irregexp, and the code in irregexp has been deleted. Update the license file accordingly.

Depends on D77732

Severity: -- → N/A
Pushed by
Add case-folding testcase r=mgaudet
Add named capture proxy testcase r=mgaudet
Remove old copy of irregexp and ENABLE_NEW_REGEXP ifdefs r=mgaudet
Clean up remaining references to ENABLE_NEW_REGEXP and old irregexp r=mgaudet
Rename new-regexp to irregexp r=mgaudet
Move RegExpStackScope inside irregexp r=mgaudet
Separate imported files from shim r=mgaudet
Apply clang-format to shim files r=mgaudet
Pushed by
Remove obsolete directory from license.html r=mhoye
You need to log in before you can comment on or make changes to this bug.