Compute (instead of define) method descriptor in .optimizer.Signatures

[ZZZank]

In some applications, Rhino are shaded directly into the output jar, and are usually relocated to another package in order to prevent clashing.

Example Gradle script (over-simplified):

dependencies {
    shadow("org.mozilla:rhino:${rhino_version}")
}

shadowJar {
    relocate("org.mozilla.classfile", "some.application.shaded.rhino.classfile")
    relocate("org.mozilla.javascript", "some.application.shaded.rhino")
}

assemble.dependsOn(shadowJar)

But doing so will break Signatures because all method descriptors defined here are string constant, thus always referencing the original class even after classes are relocated

[aardvark179]

This feels like part of a larger change as this isn’t tackling the other literals in bytecode generation.

[ZZZank]

This feels like part of a larger change

It is, but manually replacing every used class name and method descriptors is tedious and error prone. So I'm cherrypicking the easy part as this PR

[gbrail]

I don't have a problem doing this -- if nothing else it makes the code easier to read and write -- but I want to understand where we're going.

Is the problem that you are generating bytecode what doesn't work because the Rhino package name has been shaded? Because if so then yes indeed, this is only part of what you need to fix that.

One possible fix would be to replace ALL calls to internal methods with INDY instructions but I know that would be a problem for you all unless they were all "constant call sites."

[aardvark179]

I assume the goal here is to make vendoring easier by either reducing or entirely eliminating at the changes that would need to be made in the class file writing. It’s not a bad goal, but without a test case and getting all the way there it’s the sort of thing that can easily regress.

[ZZZank]

Is the problem that you are generating bytecode what doesn't work because the Rhino package name has been shaded?

If the "shaded" is misspelled "relocated", yes. Relocating rhino classes like this:

relocate("org.mozilla.javascript", "some.application.shaded.rhino")

will change the (full) class name of Rhino classes. For example, org.mozilla.javascript.Context will be relocated to some.application.shaded.rhino.Context. This will make generated bytecode unable to find Rhino classes.

without a test case and getting all the way there it’s the sort of thing that can easily regress

A test case for it might be a bit complicated, as it will likely require a new subproject for simulating usage described here

And what's the regress referring to? Performance regression, or breaking features?