On Tue, 17 Oct 2023 11:33:00 GMT, Matthew Donovan <mdono...@openjdk.org> wrote:
>> test/lib/jdk/test/lib/Asserts.java line 588: >> >>> 586: } >>> 587: // fail() throws a RuntimeException so this is unreachable. >>> 588: return null; >> >> Hm, this is unfortunate. Even though this method throws along all code >> paths, the compiler doesn't do this analysis. Even though this "never >> happens" there's still a question of "but what if?" (I guess it could happen >> if somebody in the future were to modify the code above.) I'd suggest >> unconditionally throwing an exception here (probably InternalError) to be >> more explicit. Returning null will result in an inexplicable NPE if somebody >> dereferences the return value. > > Actually, this is a mistake. If I call `fail()` immediately after the test > method executes (because an exception wasn't thrown), the RuntimeException > which will promptly be caught in my try/catch and that isn't what we want. > I'm moving that invocation to the end which looks and works a lot better. Ah, right. Throwing unconditionally at the end of the method is the right thing. Good catch. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14932#discussion_r1364340476
