JDK 9 RFR of JDK-8035076: Pattern$BnMS never used due to bug in Pattern$BnM.optimize

[Xueming Shen]

On 2/19/2014 7:47 PM, Martin Buchholz wrote:
> I don't see an actual test added here:
>
> http://cr.openjdk.java.net/~sherman/8035067/webrev/test/java/util/regex/RegExTest.java.udiff.html <http://cr.openjdk.java.net/%7Esherman/8035067/webrev/test/java/util/regex/RegExTest.java.udiff.html>
>

No actually test added, just added the bugid. The existing tests should cover all functional testing for the
supplementary characters. The fix is to enable the optimization path, black box type test does not work.

As I mentioned in the email, I did verify the fix by using the provided test case which uses reflect/Field
to go through regex's node chain. I feel uncomfortable to add that as a unit/regression test, as it is
too implementation detail dependent. But if the consensus is that type of test is OK to serve as a
unit/regression, I can surely add it in.

-Sherman

>
> On Wed, Feb 19, 2014 at 2:31 PM, Xueming Shen <xueming.shen at oracle.com <mailto:xueming.shen at oracle.com>> wrote:
>
>     Hi,
>
>     Please help codereview the change for JDK-8035076.
>
>     Issue: https://bugs.openjdk.java.net/browse/JDK-8035076
>     Webrev: http://cr.openjdk.java.net/~sherman/8035067/webrev <http://cr.openjdk.java.net/%7Esherman/8035067/webrev>
>
>     This is regression caused by the change we made back to jdk7 to support case
>     insensitive match, in which a base class SliceNode for Slice family was added and
>     we mistakenly subclass the SliceS class to this newly added class, instead of
>     the original Slice class. The BnM optimization for supplementary support, which
>     is based on the "instanceof Slice", is therefor disabled. Below is the original
>     webrev for that changeset.
>
>     http://cr.openjdk.java.net/~sherman/6486934_6233084_6504326_6436458 <http://cr.openjdk.java.net/%7Esherman/6486934_6233084_6504326_6436458>
>
>     The proposed change is to subclass SliceS from Slice node. The change has been
>     verified via the test case attached in the issue report. However I decided not to
>     include this test as a regression test as it is too "implementation detail dependent".
>
>     Thanks!
>     -Sherman
>
>
>