Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change _Bool to more readable bool #92

Closed
jnikula opened this issue Oct 27, 2022 · 6 comments
Closed

change _Bool to more readable bool #92

jnikula opened this issue Oct 27, 2022 · 6 comments

Comments

@jnikula
Copy link
Owner

jnikula commented Oct 27, 2022

Opening an issue for merge request #77. I want the feature, but I think the implementation is too trivial. I actually have some code parsing what's in the source to see if _Bool should be preserved, but hitting some issues.

Original comment:

Looks like the root cause is that <stdbool.h> defines bool instead of typedeffing it. Only the preprocessor sees bool, and from the compiler's perspective, it's _Bool. Contrast with <stdint.h> which typedefs e.g. uint32_t.

The trouble with your fix is that it's very specific to this particular problem. What if the source code had _Bool? Are there other cases where we should use whatever is in the source (and what the preprocessor sees) instead of the underlying type? I.e. should we look at the tokenized input instead?

Originally posted by @jnikula in #77 (comment)

@BrunoMSantos
Copy link
Collaborator

With no prejudice towards your other points, I have an issue with the potential lack of consistency in this. If a user used a macro to specify a type conditional on (e.g.) architecture, it wouldn't have the same treatment as this bool one. Typedefs could solve it at the expense of code changes, but defines for function names resolving to compiler intrinsics for instance wouldn't have that luxury.

So maybe we need a way of extending this mechanism with user specified macros that should not be expanded either. I would probably use it in some places. We'd end up with symmetrical _unexpand_(stdc|user)_macros functions. What do you think?

@jnikula
Copy link
Owner Author

jnikula commented Oct 28, 2022

Frankly, if the choice were between adding a generic user configurable mechanism or doing nothing, I think I'd choose to do nothing. I just consider bool vs. _Bool to be enough of a special case, and simple enough to implement.

Well, maybe. I got it almost working, but was hitting issues with struct members. :/

@BrunoMSantos
Copy link
Collaborator

Yeah, the counter argument of complexity is a mighty one. I'll concede until we have concrete use cases at least.

@jnikula
Copy link
Owner Author

jnikula commented Aug 10, 2023

So I have an updated version of #135 locally, with a new built-in extension to convert _Bool to bool, but I still think as a solution to this particular problem it's overkill. Maybe we want that anyway, as a generic thing, but perhaps changing _Bool to bool if bool was used in code is the sane default?

@BrunoMSantos
Copy link
Collaborator

I think we need something indeed, but more in line with passing a cursor object to the event handler such that the event knows what is being expanded to what, if anything, and contextually make the right choice instead of a blind search & replace.

We already have events that do not take a cursor though, so maybe we can still expose the event and let projects do it this way as a stop gap approach. I still see little value in illustrating that specific use case, but it would serve as a test case for the event I suppose.

@jnikula
Copy link
Owner Author

jnikula commented Aug 11, 2023

I need to come up with a minimal testcase, but I was seeing different behaviour depending on whether I had #include <stdbool.h> or #define bool _Bool in code. It seemed like the tokenization was not there if the macro was defined in an include, and then it was not possible to properly determine whether the code originally had bool or _Bool. Need to investigate further.

jnikula added a commit that referenced this issue Oct 5, 2023
We see _Bool in documentation, because that's the C keyword, and
stdbool.h defines bool to _Bool rather than typedefs it.

Stop overcomplicating the matter, and just unconditionally convert _Bool
to bool. 'bool' is what most people want to see in code and
documentation. It should really be that simple.

As far as special casing goes, bool is easily special enough. C++ and
the new C23 have bool keywords.

Previously I thought we should at least check whether the code
originally had '_Bool' and preserve that, but now that we have C++
support, GCC and Clang stdbool.h actually also define the reverse
(#define _Bool bool) for C++. It gets overly complicated.

With the current type fixups in place, the fix is just a few lines of
code, and I find it hard to justify adding any more code for this.

Fixes #92.
jnikula added a commit that referenced this issue Oct 6, 2023
We see _Bool in documentation, because that's the C keyword, and
stdbool.h defines bool to _Bool rather than typedefs it.

Stop overcomplicating the matter, and just unconditionally normalize
_Bool to bool. 'bool' is what most people want to see in code and
documentation. It should really be that simple.

As far as special casing goes, bool is easily special enough. C++ and
the new C23 have bool keywords.

Previously I thought we should at least check whether the code
originally had '_Bool' and preserve that, but now that we have C++
support, GCC and Clang stdbool.h actually also define the reverse
(#define _Bool bool) for C++. It gets overly complicated.

With the current type fixups in place, the fix is just a few lines of
code, and I find it hard to justify adding any more code for this.

Fixes #92.
jnikula added a commit that referenced this issue Oct 6, 2023
We see _Bool in documentation, because that's the C keyword, and
stdbool.h defines bool to _Bool rather than typedefs it.

Stop overcomplicating the matter, and just unconditionally normalize
_Bool to bool. 'bool' is what most people want to see in code and
documentation. It should really be that simple.

As far as special casing goes, bool is easily special enough. C++ and
the new C23 have bool keywords.

Previously I thought we should at least check whether the code
originally had '_Bool' and preserve that, but now that we have C++
support, GCC and Clang stdbool.h actually also define the reverse
(#define _Bool bool) for C++. It gets overly complicated.

With the current type fixups in place, the fix is just a few lines of
code, and I find it hard to justify adding any more code for this.

Fixes #92.
@jnikula jnikula closed this as completed in b26d150 Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants