This is technically a C++ bool and not a BOOL. We should be consistent here with types.

There is implicit conversion from bool to BOOL. We depend on this implicit conversion in thousands of places. Is depending on this implicit conversion bad for some reasons?

(I am fine with adding the explicit conversion in the two places you have commented on, but I do not understand why it helps.)

Is depending on this implicit conversion bad for some reasons?

No. It has no functional impact and as stated it is correct. My reasoning for it is the historical issue at the interop boundary where bool and BOOL are used interchangably and assumptions creep in. When I see P/Invoked methods I tend to make it explicit so it is clear in code what people are using. Most of the newer interop code uses bool throughout, but at the boundaries occasionally shifts to BOOL in an explicit manner as a way to acknowledge the type being used.

In this case, I think the caller could change the marshal to UnmanagedType.U1 and completely avoid the normalization that typically occurs. The DisableRuntimeMarshalling does impact that but as a demonstration of "best practice" we've been explicit about types.

I'm not pressing on it, but it is inconsistent with the other boolean returns in this PR. In fact, it is the only one that uses the implicit conversion.

Thank you for the discussion. I think I am going to leave it as is. I do not see the explicit ? TRUE : FALSE as adding clarity. I think it is fine to depend on implicit conversions.

I think the caller could change the marshal to UnmanagedType.U1 and completely avoid the normalization

I see this just as a style question. There is no material perf difference between BOOL and bool for custom P/Invokes. C/C++ compilers are very good at optimizing the bool normalizations. Depending on the compiler and the architecture, the difference can be one extra instruction in the worst case. And the second normalization happens as part of LibraryImport marshalling for both UnmanagedType.U1 and UnmanagedType.Bool.

Outside tests, I only see 6 UnmanagedType.U1 for custom P/Invokes in System.Security.Cryptography.Native.Android. All other custom P/Invokes in this repo use UnmanagedType.Bool.