On Mon, 5 Jun 2023 14:38:47 GMT, Adam Sotona <asot...@openjdk.org> wrote:

>> Class-File API actually throws wide variety of exceptions based on the 
>> actual situation. Complete error handling code must cover 
>> `IndexOutOfBoundsException`, `IllegalStateException` and 
>> `IllegalArgumentException`. 
>> 
>> Based on previous discussions we decided to consolidate on 
>> `IllegalArgumentException` with possible sub-classes. 
>> 
>> It allows easy common error handling in majority of use case, however it 
>> allows also to distinguish source of the error when needed (for example 
>> `javap` needs to know if the exception comes from constant poll or not). 
>> 
>> Newly introduced `ConstantPoolException` extends `IllegalArgumentException` 
>> to indicate the source of the problem is in constant pool. 
>> 
>> Please review.
>> 
>> Thanks,
>> Adam
>
> Adam Sotona has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8308842-exceptions
>  - fixed javadoc
>  - 8308842: Consolidate exceptions thrown from Class-File API

Reviewed, only minor comment is the one about constructors in the new exception 
class.

src/java.base/share/classes/jdk/internal/classfile/constantpool/ConstantPoolException.java
 line 42:

> 40:         super();
> 41:     }
> 42: 

Exception classes in the JDK generally have four constructors: (), (Throwable), 
(String), and (String, Throwable).

-------------

Marked as reviewed by briangoetz (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14143#pullrequestreview-1462765241
PR Review Comment: https://git.openjdk.org/jdk/pull/14143#discussion_r1218206593

Reply via email to