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

Martin Buchholz martinrb at google.com
Thu Feb 20 05:29:56 UTC 2014


OK, understood - looks good.

I might be tempted to add some reflective debugging capabilities and use
those in a test, so we can ensure that the "compiled form" of a regex
conforms to our expectations.


On Wed, Feb 19, 2014 at 8:57 PM, Xueming Shen <xueming.shen at oracle.com>wrote:

>
> 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
>
>
> 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>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
>>
>> 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
>>
>> 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
>>
>>
>>
>
>



More information about the core-libs-dev mailing list