RFR: 8307160: Fix AWT/2D/A11Y to support the permissive- flag on the Microsoft Visual C compiler [v52]
Julian Waters
jwaters at openjdk.org
Tue Mar 26 00:17:56 UTC 2024
On Mon, 25 Mar 2024 09:02:22 GMT, Magnus Ihse Bursie <ihse at openjdk.org> wrote:
> > The only thing I'm uncertain about is the pData local, which I don't see much benefit in removing since the null check associated with it still has to remain for code semantics to be correct
>
> The point is that you can do the null check on the correct variable directly, instead of going a detour with pData. So instead of:
>
> ```
> RealType realVal;
> void* pData = getVal()
> if (pData == null) {
> // bail out
> }
> realVal = (RealType) pData;
> ```
>
> you can just do:
>
> ```
> RealType realVal = getVal();
> if (realVal == null) {
> // bail out
> }
> ```
>
> as the code normally would have been written, had there not been an old macro that used the "magic" temporary pData variable.
>
> This is a recurring pattern in this patch and needs to be fixed everywhere.
I have a concern since the null check bailout involves THROW_NULL_PDATA_IF_NOT_DESTROYED, which is no longer accurate if we remove the pData local. Maybe Phil can suggest an alternative to that, because I don't know what that might be. Although, if given the choice I'd use RAII through unique_ptr like Thomas suggested, it just seems so much cleaner to me. Speaking of, hopefully this causes enough noise to get Phil's attention again. In the meantime, I've reverted as much of the formatting changes I could possibly find
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15096#issuecomment-2019140763
More information about the build-dev
mailing list