1507887 - Review js/src/irregexp formatting

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P1)

Description

[Ted Campbell [:tcampbell]]

With upcoming policy changes to reformat mozilla-central to a consistent and automated style, we need to make sure our irregexp import is on the exclusion list. I'm not sure what specific paths we should have, but they should be listed in these files.

This should happen before the end of the month before the style update happens.

> tools/rewriting/ThirdPartyPaths.txt
> .clang-format-ignore

Comment 1

[Ted Campbell [:tcampbell]]

Andre, do you know what files need to maintain existing style for the irregexp update you were looking into?

Comment 2

[(no longer active)]

Should we dupe this against bug 1510128?

Comment 3

[Ted Campbell [:tcampbell]]

I'd like JS to take a final look at what happens here, but Bug 1510128 is already in-flight. Assigning this to Jan instead of duping in case we decide on a different end result.

Comment 4

[Jan de Mooij [:jandem]]

I think for now excluding irregexp makes sense because it gives us some extra time. However I'm not convinced it's necessarily the right thing to do: it will effectively add a new style to the code base that's mostly-SM style, non-Gecko style, non-V8 style. It sounds like anba's patches could be easily rebased with clang-format so I don't really see why we *shouldn't* reformat irregexp.

Comment 5

[Jason Orendorff [:jorendorff]]

Right. What we want is for irregexp to look like upstream. Reformatting might be the best way to do that. I just don't have time to confirm that right now.

Comment 6

[(no longer active)]

We can always format it later when we come to a resolution here, I don't think there is any particular rush to come to an answer.

Comment 7

[André Bargull [:anba]]

(In reply to Ted Campbell [:tcampbell] (PTO Dec 24-Jan 1) from comment #1)
> Andre, do you know what files need to maintain existing style for the
> irregexp update you were looking into?

Basically all files under "js/src/irregexp" (*), so exactly the ones which were excluded in bug 1510128.

(*) Except js/src/irregexp/RegExpCharacters.{h,cpp} which aren't from upstream, but see <https://searchfox.org/mozilla-central/rev/3e7afaa5b57b3f8ed100cd1f13bb0a93b9b2cb99/.clang-format-ignore#12-18>.