Closed Bug 1311633 Opened 3 years ago Closed 3 years ago

Add a testing function that dumps RegExp bytecode.


(Core :: JavaScript Engine, enhancement)

Not set



Tracking Status
firefox52 --- fixed


(Reporter: arai, Assigned: arai)




(1 file, 1 obsolete file)

It would be nice to have a shell-only testing function that dumps RegExp bytecode for given pattern, flags, mode, and input.
Assignee: nobody → arai.unmht
Severity: normal → enhancement
dis_regexp dumps bytecode for given RegExp object, with match_only boolean and sample input (bytecode differs for input, especially latin1 or not).

unfortunately this cannot be done outside of RegExpObject or RegExpShared.
so I added RegExpShared::dumpBytecode.

it dumps bytecode, in the same format as dis() function.

the tricky part is bytecode length.
the bytecode has no information about its length, so we need to calculate the maximum PC from jumps and branches.
Attachment #8802872 - Flags: review?(till)
Moved to TestingFunctions.cpp and renamed to disRegExp.
no other changes.
Attachment #8802872 - Attachment is obsolete: true
Attachment #8802872 - Flags: review?(till)
Attachment #8803848 - Flags: review?(till)
Summary: Add a shell-only testing function that dumps RegExp bytecode. → Add a testing function that dumps RegExp bytecode.
Comment on attachment 8803848 [details] [diff] [review]
Add disRegExp testing function.

Review of attachment 8803848 [details] [diff] [review]:

So very nice. r=me with nits addressed.

::: js/src/builtin/TestingFunctions.cpp
@@ +3929,5 @@
> +    }
> +
> +    RootedObject obj(cx, &args[0].toObject());
> +    if (!obj)
> +        return false;

This seems like it shouldn't be possible, right?

In general, I'd combine checks here into `if (!args[0].isObject() || !args[0].toObject().is<RegExpObject>())`, and then only have one block in which the "First argument must be a RegExp" error is thrown.

::: js/src/vm/RegExpObject.cpp
@@ +884,5 @@
> +        return false;
> +
> +    return>dumpBytecode(cx, match_only, input);
> +}
> +#endif

Nit: add a "// DEBUG" comment to the endif.
Attachment #8803848 - Flags: review?(till) → review+
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.