conpty: remove accidental bool; use BOOL instead#20035
Conversation
DHowett
changed the title
| if (_initialVisibility) | ||
| { | ||
| THROW_IF_FAILED(ConptyShowHidePseudoConsole(_hPC.get(), _initialVisibility)); | ||
| THROW_IF_FAILED(ConptyShowHidePseudoConsole(_hPC.get(), _initialVisibility ? TRUE : FALSE)); |
There was a problem hiding this comment.
@lhecker years of doing win32 and i am still unclear whether stdbool/bool can implicitly convert to BOOL ;P
There was a problem hiding this comment.
Yeah, it's well defined. BOOL is an alias for int32, so bool <> BOOL works implicitly. The compiler inserts != 0 checks as needed. But the ABI is different! AArch64 does not require callers to zero upper bits in register arguments and bool is just 1 byte.
There was a problem hiding this comment.
If you care about the ARM64 ABI break. C has _Bool, which should match C++'s bool. As well, you could include <stdbool.h>, it defines a macro bool, so that bool is _Bool when using C. _Bool is part of the C standard, so it is safe to use.
There should also be an ABI break on x86, because of the use of WINAPI and the change from 1 byte bool to the 4 bytes BOOL. I did some testing and it looks like the exported function name should stays the same on x86, so there is no ABI break there.
DHowett
force-pushed
the
dev/duhowett/conpty-bOoL
branch
from
46f5ce4 to
b715412
Compare
|
I think you want to close #20030. |
|
huh, i'm not sure where I got the ID I did. thanks. I'm not particularly concerned about the ABI; since it's deployed side by side (or in some cases, statically linked) per application, authors are not updating the source version without also updating the binary version. :) |
3 participants
Closes #20030