On Wed, 22 Jul 2026 21:06:53 GMT, Coleen Phillimore <[email protected]> wrote:

> Please review this change that moves the Throwable and StackTrace classes 
> into a new file.  I didn't make any modifications to the code, just moved it, 
> added include directives to javaStackTraceClasses.hpp, minimized include 
> directives in the new files, and removed includes in javaClasses.cpp that 
> were no longer needed.
> 
> Tested with tier1 on Oracle supported platforms (product & fastdebug).
> 
> This change to be merged post valhalla integration.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

Looks good. I've posted a couple of nits.

src/hotspot/share/classfile/javaClasses.hpp line 699:

> 697:   static void java_printStackTrace(Handle throwable, TRAPS);
> 698:   // Debugging
> 699:   friend class JavaClasses;

Q: The lines 698-699 were not copied to new header. Is it intentional?

src/hotspot/share/classfile/javaStackTraceClasses.hpp line 230:

> 228: #undef CHECK_INIT
> 229: 
> 230: #endif // SHARE_CLASSFILE_JAVASTACKTRACECLASSES_HPP

Nit: This header does not have forward declarations for:
 - `Method`, `InstanceKlass`, `JavaThread`, and `outputStream`.
This can be a pre-existed issue in `javaClasses.hpp`.

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

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/32016#pullrequestreview-4760927224
PR Review Comment: https://git.openjdk.org/jdk/pull/32016#discussion_r3635670527
PR Review Comment: https://git.openjdk.org/jdk/pull/32016#discussion_r3635699914

Reply via email to