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

[Thomas Stuefe]

On Tue, 8 Nov 2022 07:23:20 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> 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.
>> 
>> I was worried about that too. This patch initially came about when I was writing #11017 which required rewriting the macro logic and add extra nested scope. The fact that the common case for a step was checking the verbose flag made me refactor out just the verbose case. And then I just refactored out the rest the few extra conditions as well. 
>> 
>> But I am pretty ambivalent about this change. #11017 contains the newline changes as well. So if there are more opinions that this makes back porting work harder it can be dropped.
>>  
>> 
>>> 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.
>> 
>> I added it because in my initial change (9449501f063e7662a530adf921ae53dadd0312a0), a theoretical crash in a condition would recover but not proceed to the next step. The test change is to avoid such a regression if someone thinks of doing the same simplification of the macro condition logic. (It would crash before `_current_step` was updated.)
>
> Ah, I understand. But you changed this to two nested ifs. So this is no problem anymore. 
> 
> I still don't think it is needed with your new version. IIUC it prevents us from the problem you describe if someone changes the STEP macros in the future *and* has a very complex if statement that would crash.

About the STEP_IF, the more I look at it, the more I like it. So I am ambivalent. Let's hear what others say (@coleenp?).

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

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