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

boolean data type causes logic errors. #2147

Closed
Chris--A opened this issue Jun 28, 2014 · 6 comments
Closed

boolean data type causes logic errors. #2147

Chris--A opened this issue Jun 28, 2014 · 6 comments

Comments

@Chris--A
Copy link
Contributor

It has come to my attention that the boolean type alias produces results differing to the C++ standard boolean type bool.

Consider this sketch:

void setup() {
  Serial.begin(9600);
  Serial.println( true == (bool) 57 ? "true" : "false" );
  Serial.println( true == (boolean)57 ? "true" : "false" );
  Serial.println( true == bool( true ) ? "true" : "false" );
  Serial.println( bool( 1 | 2 | 4 | 8 ) == boolean( 1 | 2 | 4 | 8 ) ? "true" : "false" );
  Serial.println( true == false + 1 ? "true" : "false" );
  Serial.println( true + true == true ? "true" : "false" );
  Serial.println( bool( true + true ) == true ? "true" : "false" );
  Serial.println( boolean( true + true ) == true ? "true" : "false" );
}

void loop(){}

Using this sketch with boolean declared as a typedef to uint8_t ( which is what the current core does ) the output to the serial monitor is:

true
false
true
false
true
false
true
false

Whereas, after changing the core to allow a boolean to be declared as bool. The sketch produces this output:

true
true
true
true
true
false
true
true

It appears boolean has been implemented to provide boolean functionality with some C code in the Arduino core. I recommend changing the core so by the time boolean is seen by a sketch it is either a typedef or #define to the real bool type.

These are the modifications I propose be implemented in Arduino.h

#ifdef __cplusplus
    typedef bool boolean;
#else
    typedef uint8_t boolean;
    #define false 0
    #define true !false
#endif

On a side note, the defines for true and false should be removed. Boolean's ( the real one ) are subject to integral promotion and convert to either 0 or 1. Also the standard forbids macros named the same as keywords. If the C code needs defines, use TRUE & FALSE instead.

I have created a pull request here:
#2151

I also have a proof I'm in the middle of writing as to why the defines true and false need to be removed from the C++ side. I will add it soon.

@Chris--A
Copy link
Contributor Author

I will post this as a pull request soon, with the additional info for the defines true and false.

@matthijskooijman
Copy link
Collaborator

In C, the bool type is also available through including <stdbool.h>, which leaves it to the compiler to define the proper types and macros to make things work.

I would welcome a change to more standard typings, but some thought has to be given to compatility: are there real cases where a sketch would break with this change?

@Chris--A
Copy link
Contributor Author

I have just made a pull request. #2151

However with true and false defined as 0 & 1 this sketch fails. Stdbool fixes that, but not the fact it actually defines a keyword, so its not needed.

My fix is safe as it is using the real deal in C++, and C sees it how it was once defined.

This fails for instance in 1.5.6

void setup() {
  Serial.begin( 9600 );

  DoSpecific( true );
  DoSpecific( false );
  DoSpecific( 1 );
  DoSpecific( 0 );
}

void loop(){}

void DoSpecific( bool value ){
  Serial.print( "The boolean value is: " );
  Serial.println( value ? "TRUE" : "FALSE" );
}

void DoSpecific( int value ){
  Serial.print( "The integer value is: " );
  Serial.println( value, HEX );
}

@matthijskooijman
Copy link
Collaborator

Why is it a problem that stdbool "defines a keyword" exactly? stdbool.h is shipped by gcc, so I'd say they'll know what is and is not allowed? Note that the #define true true, so they don't actually change the meaning of the keywords AFAICS?

@Chris--A
Copy link
Contributor Author

Its not a problem, its just not allowed

A translation unit shall not #define or #undef names lexically identical to keywords".

Also it is not needed, stdbool is only an additional header to process, its features are contained within my patch, but mine also fixes the Arduino specific boolean type for C++.

And yes the stdbool version of true & false is safe, after all the define is a copy paste job.

cmaglie added a commit that referenced this issue Jan 7, 2015
@cmaglie
Copy link
Member

cmaglie commented Jan 7, 2015

Fixed with 20ac20f

@cmaglie cmaglie closed this as completed Jan 7, 2015
earlephilhower pushed a commit to earlephilhower/Arduino that referenced this issue Jan 29, 2019
Match current Arduino definition to avoid issues with comparison
operations.

arduino/Arduino#2147
arduino/Arduino@20ac20f

Fixes esp8266#5440
devyte pushed a commit to esp8266/Arduino that referenced this issue Feb 7, 2019
Match current Arduino definition to avoid issues with comparison
operations.

arduino/Arduino#2147
arduino/Arduino@20ac20f

Fixes #5440
ollie1400 pushed a commit to ollie1400/Arduino that referenced this issue May 2, 2022
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

No branches or pull requests

3 participants