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

Codebase-wide boolean audit #189

Merged
merged 1 commit into from
Dec 16, 2015
Merged

Codebase-wide boolean audit #189

merged 1 commit into from
Dec 16, 2015

Conversation

hostilefork
Copy link
Member

This commit represents the mostly mechanical remainder of a
compiler-assisted check over the codebase for usages of REBOOL
that were misinterpreted as integers. Bugs or solutions to confusing
usages have been committed separately.

To directly read and comment on the new definitions, see the new
"Boolean" section of %reb-c.h:

https://github.com/metaeducation/ren-c/pull/189/files#diff-6ef5cfafbcfde94617f41a68573fbe68R246

The previous separation between the 8-bit "REBOOL" for efficient
structure packing and the platform-optimal "REBFLG" for CPU
convenience is eliminated, favoring REBOOL for the common type
due to its similarity to bool as the C99 and C++ standard term for
a boolean value. In practice there actually were no REBOOL cases
used for efficient structure packing anyway, but this defines the type
REBOOL8 just in case.

REBFLG is renamed to REBFLGS and reclaimed as a platform
efficient carrier for several bitflags--not to be confused with the singular
REBOOL. The rename was prior to having a mechanism for being
able to differentiate REBFLG and REBOOL in a conventional build,
but with that ability added via an enum trick (on non-windows compiles)
then this may be reconsidered if REBFLG's "6-character-ethos" is
deemed more important than being plural.

Though testing booleans using any ordinary integer-bearing operators
one likes is still legal, the changes make it so that it is not legal to
directly assign integers to REBOOL. This means for instance that you
cannot write REBOOL b = 2;, and confines them to being the values
1 and 0 only. The inconvenience of this is that given the way C works,
this also rules out REBOOL b = (1 < 2); as that also is an integer
assignment. The macro LOGICAL is introduced to do the conversion,
as is the macro NOT for the inverse.

(Note: This is a complete replacement for casting instances such as
(REBOOL)some_expression...which did not do what whoever wrote that
thought it did (e.g. it would not coerce arbitrary bitmasks into 0 or 1.
in C++ (bool)some_expression would do that, but in C with REBOOL
was an 8-bit integer in C, so you'd just clip it to 8 bits.)

Two different tests were used to vet the codebase for bugs. One is kept
under the conditional STRICT_BOOL_COMPILER_TEST, which will make
REBOOL a pointer type to a dummy struct and then use bogus pointer
values to catch mixtures with integers. This does not catch the case of
literal assignment of "0" as a subtitute for FALSE, so that is addressed by
a different check which makes REBOOL an enumerated type. As this
is not compatible with the Windows definitions, the approach is not used
on Windows.

By its nature, the STRICT_BOOL_COMPILER_TEST does not use a 0
value to represent false...hence the produced executable is garbage...and
the only use is to locate potential bugs in offending usages.

This commit represents the mostly mechanical remainder of a
compiler-assisted check over the codebase for usages of REBOOL
that were misinterpreted as integers.  Bugs or solutions to confusing
usages have been committed separately.

The previous separation between the 8-bit "REBOOL" for efficient
structure packing and the platform-optimal "REBFLG" for CPU
convenience is eliminated, favoring REBOOL for the common type
due to its similarity to `bool` as the C99 and C++ standard term for
a boolean value.  In practice there actually were no REBOOL cases
used for efficient structure packing anyway, but this defines the type
REBOOL8 just in case.

REBFLG is renamed to REBFLGS and reclaimed as a platform
efficient carrier for several bitflags--not to be confused with the singular
REBOOL.

Though testing booleans using any ordinary integer-bearing operators
one likes is still legal, the changes make it so that it is not legal to
directly assign integers to REBOOL.  This means for instance that you
cannot write `REBOOL b = 2;`, and confines them to being the values
1 and 0 only.  The inconvenience of this is that given the way C works,
this also rules out `REBOOL b = (1 < 2);` as that also is an integer
assignment.  The macro LOGICAL is introduced to do the conversion,
as is the macro NOT for the inverse.

Two different tests were used to vet the codebase for bugs.  One is kept
under the conditional STRICT_BOOL_COMPILER_TEST, which will make
REBOOL a pointer type to a dummy struct and then use bogus pointer
values to catch mixtures with integers.  This does not catch the case of
literal assignment of "0" as a subtitute for FALSE, so that is addressed by
a different check which makes REBOOL an enumerated type.  As this
is not compatible with the Windows definitions, the approach is not used
on Windows.

By its nature, the STRICT_BOOL_COMPILER_TEST does not use a 0
value to represent false...hence the produced executable is garbage...and
the only use is to locate potential bugs in offending usages.
@hostilefork hostilefork merged commit 53e7bbf into metaeducation:master Dec 16, 2015
@hostilefork hostilefork deleted the boolean-audit branch December 16, 2015 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant