Go Down

### Topic: min, max, abs.. are macroses? (Read 25386 times)previous topic - next topic

#### kibergus

##### Dec 23, 2011, 07:06 pmLast Edit: Dec 26, 2011, 09:20 pm by Coding Badly Reason: 1
Why on the earth these functions are defined as macros:

Code: [Select]
`#define min(a,b) ((a)<(b)?(a):(b))#define max(a,b) ((a)>(b)?(a):(b))#define abs(x) ((x)>0?(x):-(x))#define constrain(amt,low,high) ((amt)<(low)?(low):((amt)>(high)?(high):(amt)))//#define round(x)     ((x)>=0?(long)((x)+0.5):(long)((x)-0.5))#define radians(deg) ((deg)*DEG_TO_RAD)#define degrees(rad) ((rad)*RAD_TO_DEG)#define sq(x) ((x)*(x))`

This is dangerous and leads to strange behavior when you supply calculated values to these functions because values are calculated twice. They definitely must be rewritten as inline functions. This would make them much more robust and much faster.

I can write appropriate patch if you want.

Moderator edit: [code] [/code] tags added.

#### kibergus

#1
##### Dec 23, 2011, 08:29 pmLast Edit: Dec 26, 2011, 11:49 am by AWOL Reason: 1
I understand, that it is not possible to write in ANSI C, but current implementation do not conform to C99 or C89 because abs must be a function. And these macro prevent form defining good overloaded functions from C++ std:: namespace.
And I'm sure that these macroses do much more harm than good: they behave differently than functions with the same names on other platforms. They look like functions but the do not work like functions. It means that someone not familiar to arduino is unable to read/write programs with these macroses.

#### Jantje

#2
##### Dec 23, 2011, 09:18 pm
kirbergus
I don't mind a good discussion but please watch your language. Definitely if you make statements which are not solid.
It is not macroses but macros
You do not use WTF. Definitely not in a title.
2) the "must be a function"
The C89 standard you refer to is more commonly known as ANSI C. ANSI C was officially released in 1989. I have been programming in C from 1991 till 2000 for my living. In all those years I used a define for max. It is new to me that this must be a function. If you are sure that ANSI C  states that max must be a function please tell me where this is stated.
The C99 is the newest version of C. The compiler used by the Arduino team is (like most other C compilers) not 100% compliant with C99. As far as I know there is no statement that max must be a function. Please state where it states min max and abs  must be functions. Also refer to where it states all C compilers must comply to C99.
3) No macros but Inline
You state that these macros must be rewritten as inline. Inline is new in C99. It comes from C++. Inline is class related and on a AVR you do not want to have classes for something as basic as integers.
4) why a macro?
A macro has a great advantage. That is, it works with any type that implements the methods you use. So you do not need a maxint maxlong maxstring.....
5) You are right there is a risk with macros
On the content you are right that
Code: [Select]
` MyNumber=5; max(++MyNumber);` will result in MyNumber being 7 and not 6;  This is however a design decision that has been taken a long time ago on very good ground by people that didn't know about Arduino because it simply didn't exist yet. So blaming Arduino is not the way to go. It has been like this for decades now. I feel no sense of urgency.
6) overloaded functions from C++ std
I see no reason why you could not overload. You can use the #undef instructions if you want.

If you do not comply to these rules I will no longer respond on your posts.
Best regards
Jantje
Do not PM me a question unless you are prepared to pay for consultancy.
Nederlandse sectie - http://arduino.cc/forum/index.php/board,77.0.html -

#### kibergus

#3
##### Dec 23, 2011, 11:06 pm

kirbergus
I don't mind a good discussion but please watch your language. Definitely if you make statements which are not solid.
It is not macroses but macros
You do not use WTF. Definitely not in a title.

Thanks. I was a little bit upset that I have to reinvent standard library. I've corrected errors.

2) the "must be a function"
The C89 standard you refer to is more commonly known as ANSI C. ANSI C was officially released in 1989. I have been programming in C from 1991 till 2000 for my living. In all those years I used a define for max. It is new to me that this must be a function. If you are sure that ANSI C  states that max must be a function please tell me where this is stated.
The C99 is the newest version of C. The compiler used by the Arduino team is (like most other C compilers) not 100% compliant with C99. As far as I know there is no statement that max must be a function. Please state where it states min max and abs  must be functions. Also refer to where it states all C compilers must comply to C99.

I didn't want to say that max must be a function accordingly to standard. Only abs must, in ANSI C too. I wanted to say that today in 2011, nearly in 2012 there is no need to put user right at the edge of a pit, even if you warn him. Look at this macro:

define sq(x) ((x)*(x))

It squares a variable. Why you would ever like to use it? If you have a single variable x it is easier and shorter to write x*x. But if you have a big statement which you would like to square it is very handy just to put it inside sq(). If sq was a function it would be computationally efficient too.
So if you see that using sq()would ease your life you nearly definitely going to make a mistake. This macro pushes user to a wrong way.

3) No macros but Inline
You state that these macros must be rewritten as inline. Inline is new in C99. It comes from C++. Inline is class related and on a AVR you do not want to have classes for something as basic as integers.

inline has nothing with classes. inline is just a hint for compiler that you would like to inline the function, nothing more. Using inlined function for integer does not convert them to classes.

4) why a macro?
A macro has a great advantage. That is, it works with any type that implements the methods you use. So you do not need a maxint maxlong maxstring.....

Actually this macro works only with different types of integers, float, double, pointers which can be implicitly converted to void*. If you have operator< defined for other type you are already using C++ and you can use templates which are much better then macro in this situation.

5) You are right there is a risk with macros
On the content you are right that
Code: [Select]
` MyNumber=5; max(++MyNumber);` will result in MyNumber being 7 and not 6;  This is however a design decision that has been taken a long time ago on very good ground by people that didn't know about Arduino because it simply didn't exist yet. So blaming Arduino is not the way to go. It has been like this for decades now. I feel no sense of urgency.

If you look at main arduino.cc page you ca read "It's intended for artists, designers, hobbyists, and anyone interested in creating interactive objects or environments". Time has changed, programming languages has changed.This design decision was made "a long time ago on very good " old ground. Artists, designers, hobbyists do not need carefully documented well known rakes. 20 years ago it was the best solution but now we have more fool proof programming technologies. Which are still as efficient as old ones. They can be even more efficient in some cases.

6) overloaded functions from C++ std
I see no reason why you could not overload. You can use the #undef instructions if you want.

Thanks for the hint. I've did it for my building environment.

#### bperrybap

#4
##### Dec 26, 2011, 05:47 am

4) why a macro?
A macro has a great advantage. That is, it works with any type that implements the methods you use. So you do not need a maxint maxlong maxstring.....

But macros also can have issues and create some intended consequences that functions do not.

Consider this example:

Code: [Select]
`bar = sq(foo++);`

How much is foo incremented?
foo is incremented twice when sq() is a macro and if it is a function then it is incremented only once
as most people would probably expect.

Other unexpected things happen if the data is not a simple variable type and is a function.
Consider this example:

Code: [Select]
`bar = abs(foo());`

The function foo() will be called multiple times if abs() is a macro vs only once if it is a function.
If foo() has the potential to return different values on each call, then the results become unpredictable.
When abs() is a macro, consider the case when the first call to foo() returns a negative value and
the second time it returns  a positive value.
In this case the final result from abs() will be negative which nobody would expect from an abs() "function".

So there is a functional difference that can create non obvious issues when these "functions" are
implemented as macros vs real functions.

--- bill

#### maniacbug

#5
##### Dec 26, 2011, 02:12 pm
Yah, I agree with the OP.  Macros like these should have gone out with bell-bottoms. They conflict with the ::abs and friends definitions, and they lead to unexpected side-effects.  Yes we could work around it with #undefs.  You can work around many problems, that doesn't make it NOT a problem.

#6
##### Dec 27, 2011, 12:45 am

If they are not macros, how should they be implemented?  Templates?  Inline-functions?

#### maniacbug

#7
##### Dec 27, 2011, 01:13 am

If they are not macros, how should they be implemented?  Templates?  Inline-functions?

The idea is to get them out of the pre-processor and into the compiler.  So the basic answer to that question is "anything parsed by the compiler".

A classic implementation is this:
Code: [Select]
`template<typename T>  const T&  max(const T& a, const T& b)  { return (a > b) ? a : b; }`

This has the interesting side effect of requiring the types to be identical, which safeguards against accidentally comparing disparate types, which gives the compiler permission to get funky on your types (er, I mean, implicitly convert them on you), which can lead to unpredictable results.  If that's overly pedantic, max() can be declared with two types, in which case it will behave more like normal.

Leaving it in the pre-processor means you rob files from defining it another namespace, or even as a method in a class!

Try compiling this:

Code: [Select]
`class wtf{  int max(void)  {    return 1;  }};void setup(void){  wtf w;  Serial.print(w.max());}`

Code: [Select]
`sketch_dec26a.cpp:6:15: error: macro "max" requires 2 arguments, but only 1 givensketch_dec26a.cpp:15:22: error: macro "max" requires 2 arguments, but only 1 givensketch_dec26a:2: error: function definition does not declare parameters`

#8
##### Dec 27, 2011, 03:46 am

The risk is that existing C source files will no longer compile.  But, as far as I can tell, nothing in the core is effected and none of the libraries available from this tool are effected...

http://arduino.cc/forum/index.php/topic,63215.0.html

Any other C source files to worry about?

#### kibergus

#9
##### Dec 29, 2011, 07:05 pmLast Edit: Dec 29, 2011, 07:17 pm by kibergus Reason: 1
This is what I currently use in my project. Including this file from C++ source would result in undefining macros and defining appropriate templates. Including from C source should have no effect because of macro guards.
It uses some C++11 features so -std=gnu++0x flag should be passed to gcc. It also calls gcc specific abs realizations for floating point types so it is not possible to use this with compiler other than gcc.
I've also defined new and delete operators. As far as I understand they are already defined in latest arduino. So you may need to remove them to compile your project.

P.S. Using it shouldn't have any impact on program size or speed if you are using macros only with single variables.

#10
##### Dec 29, 2011, 08:26 pm

No round?  Did you exclude it on purpose?

You allow the parameters to be different types.  You're not concerned about the possibility of implicit type-conversions?

Quote
It uses some C++11 features so -std=gnu++0x flag should be passed to gcc.

I suspect that will not happen.

Quote
P.S. Using it shouldn't have any impact on program size or speed if you are using macros only with single variables.

In my testing it does.  In some cases this...

Code: [Select]
`template <class T> const T& minT ( const T& a, const T& b ){  return( (a<b) ? a : b );}`

Produces less code the the min macro.

#### kibergus

#11
##### Dec 29, 2011, 09:51 pm

No round?  Did you exclude it on purpose?

In my version of arduino library round is commented out:
Code: [Select]
`//#define round(x)     ((x)>=0?(long)((x)+0.5):(long)((x)-0.5))`
So I didn't include it.

You allow the parameters to be different types.  You're not concerned about the possibility of implicit type-conversions?

Right. That's how it is done in gcc's library:
Code: [Select]
`  template<typename _Tp>    inline const _Tp&    max(const _Tp& __a, const _Tp& __b)    {      // concept requirements      __glibcxx_function_requires(_LessThanComparableConcept<_Tp>)      //return  __a < __b ? __b : __a;      if (__a < __b) return __b;      return __a;    }`
So I think C++ standard allows to have different types here. At first version I didn't allow to have different types but I've got weird errors nearly identical types. Default C++ types have quite logical type conversion rules and class constructors in most cases should be defined as explicit so as to prohibit implicit type conversions. So if your classes are well written you should not be aware of implicit type conversions much.

Quote
It uses some C++11 features so -std=gnu++0x flag should be passed to gcc.

I suspect that will not happen.

C++11 is cool. Using it for my project allows me to spend less time and write more readable code. And I really need it for implementing good mixin design patterns. If you use makefiles you can add compiler flag easily. I do use makefiles because I'm used to vim.

I think, that you can find C++99 compliant solutions here:
https://github.com/maniacbug/StandardCplusplus
or here:
http://andybrown.me.uk/ws/2011/01/15/the-standard-template-library-stl-for-avr-with-c-streams/
But unfortunately I didn't succeeded in compiling these libraries today.

Quote
P.S. Using it shouldn't have any impact on program size or speed if you are using macros only with single variables.

In my testing it does.  In some cases this...
Code: [Select]
`template <class T> const T& minT ( const T& a, const T& b ){  return( (a<b) ? a : b );}`
Produces less code the the min macro.

Well, it shouldn't if you pass single variable to macro which is an advised way to use macro. And it easily can be faster and consume less memory if you pass expressions in-to macro.

#### PatrickPijnappel

#12
##### Jan 02, 2012, 12:56 pm
I personally just yesterday ran into the problems mentioned above.

Code: [Select]
`int value = constrain(Serial.readByte(), 0, 5);`

Anyway, I would just add that:
a) I am strongly in favor of replacing these macros by something better, if at all feasible.
b) If they end up not being replaced, I would at least suggest that they are properly documented and warned about in the reference. I had to look in the source files to find out.

#### bperrybap

#13
##### Jan 02, 2012, 07:53 pm
The real problem with the macros isn't that they are macros but that they are too simplistic.
gcc provides a way to declare and use locals as well as extract the data types of
the arguments to declare the locals inside the macros.

This only works for gcc as it is a gcc extension.
Does anybody use anything else...

But it provides a very simple way to create macros that work properly and
will work for both C and C++

So for example this is better definition of max() that is still a macro:
Code: [Select]
`#define max(a,b) \       ({ typeof (a) _a = (a); \           typeof (b) _b = (b); \         _a > _b ? _a : _b; })`

And a few others:
Code: [Select]
`#define min(a,b) \       ({ typeof (a) _a = (a); \           typeof (b) _b = (b); \         _a < _b ? _a : _b; })`
Code: [Select]
`#define abs(x) \       ({ typeof (x) _x = (x); \          _x > 0 ? _x : -_x; })`
Although I would probabaly want to take a closer look at abs() to see if it is better to use >= vs >
i.e. does -0 create some problems?

etc...
So this could also easy be used for constrain() and round()

For my C entrenched brain it seems a whole lot easier than all the
template and class foo.

--- bill

#### pYro_65

#14
##### Jan 03, 2012, 07:31 am
that looks like a good fix, but IMO it is more obfuscated than a direct template.

As an object orientated programmer, templates are a natural choice.

Leaving bugs in software and expecting novice's to know the in's and out's of #define's is quite silly. since this thread stated there have been a number of threads with confused posters as to why an operation is done multiple times and undocumented when using arduino core functionality.

People having to learn the basics of templates is far more beneficial than, rare case #define logic

Forum Mod anyone?
https://arduino.land/Moduino/

Go Up