On Wed, 12 Jul 2023 15:20:35 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Daohan Qu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Use Assert instead of throwing exceptions
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/tools/jcore/ByteCodeRewriter.java
> line 106:
>
>> 104: cpCacheIndex = bytes.swapInt(cpCacheIndex);
>> 105: short cpIndex = (short)
>> cpCache.getIndyEntryAt(cpCacheIndex).getConstantPoolIndex();
>> 106: if (!cpool.getTagAt(cpIndex).isInvokeDynamic()) {
>
> Should this be an assertion instead?
In fact, I’m also wondering if this `IllegalArgumentException` is appropriate
here. Thanks for your suggestions, I think it would be better to use
`Assert.that()` provided by `sun.jvm.hotspot.utilities.Assert` (`assert`
provided by Java won’t take effect without `-ea`) which is also a common
practice under `sun.jvm.hotspot.*` package.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14852#discussion_r1261935339
