RFR: 8296470: Refactor VMError::report STEP macro to improve readability

Thomas Stuefe stuefe at openjdk.org
Mon Nov 7 15:35:55 UTC 2022


On Mon, 7 Nov 2022 13:25:53 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

> Refactor the STEP macro in VMError::report to improve readability.
> Right now the macro contains multiple statements on one line and the non-conventional control flow is even harder to understand.
> 
> This enhancement aims to do two things:
>  1. It splits the macro into multiple lines with indentations which makes the structure of the C++ code generated by the preprocessor clearer.
>   2. Separates the internal step logic from the decision logic which decides if a step should be taken with a STEP_IF(step_name_str, condition) macro
> 
> Testing: tier 1 + GHA

Hi Axel,

I'm fine with the line break, but unsure about the STEP_IF.

I don't think it adds much readability, but it is a wide-spread patch that will add unnecessary noise for us backporters. Just my opinion, lets see what others think.

Cheers, Thomas

src/hotspot/share/utilities/vmError.cpp line 593:

> 591:       controlled_crash(TestCrashInErrorHandler);
> 592:       return true;
> 593:     }())

I don't think this is necessary. The tests for secondary crash handling are exhaustive, there is no reason why it should differ from crash condition handling outside of a crash condition. If the signal handler is correctly set up, it should work. If it is not, the prior step is sufficient.

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

Changes requested by stuefe (Reviewer).

PR: https://git.openjdk.org/jdk/pull/11018


More information about the hotspot-dev mailing list