Function abs() on Every malfunctioning?

Hi,
can someone tell me what's wrong with these few lines of code:

long stepValue = 1000000;
long absStepValue;

void setup() {
  Serial.begin( 115200 );
  absStepValue = abs(stepValue);
  Serial.println("test abs() :");
  Serial.println(stepValue);
  Serial.println(absStepValue);
}

void loop() {
}

On an UNO I get:

test abs() :
1000000
1000000

On Nano Every I get:

test abs() :
1000000
16960

Try this way:

long stepValue = 1000000L;

it should not change anything as there is no math, so it will be used as is (and an int is 32 bit on that platform if this is what you had in mind)

Once it's in the long variable, it's handled as a long.

abs() is defined as a macro in Arduino.h (which is a bad idea...)

#define abs(x) ((x)>0?(x):-(x))

That makes no difference. As you can see from the println(stepValue) result, stepValue has its correct value of 1000000.

actually on second thoughts, the Nano Every uses the ArduinoCore-megaavr

so they should use the stdlib abs() definition

the default definition of abs() is using int and not long. may be try with labs()

see https://cplusplus.com/reference/cstdlib/abs/

if I don't use abs() it works:

long stepValue = -1000000;
long absStepValue;
long absStepValue2;

void setup() {
  Serial.begin( 115200 );
  absStepValue = abs(stepValue);
  absStepValue2 = ((stepValue)>0?(stepValue):-(stepValue));
  Serial.println("test abs() :");
  Serial.println(stepValue);
  Serial.println(absStepValue);
  Serial.println(absStepValue2);
}

void loop() {
}

gives:

test abs() :
1000000
16960
1000000

This seems to be a nasty trap.

I added an answer while you were typing. Try this

long stepValue = -1000000;
long absStepValue;

void setup() {
 Serial.begin( 115200 );
 absStepValue = labs(stepValue);
 Serial.println("test abs() :");
 Serial.println(stepValue);
 Serial.println(absStepValue);
}

void loop() {}

(the macro works because the optimiser is likely getting rid of the macro)

Since I don't have Every I can't test it,

it does not matter if it's an Every or not. It's the C++ standard that applies. It states that

The type of the integer literal is the first type in which the value can fit

so when you write

long stepValue = -1000000;

the compiler sees 1000000 without any suffix and as it does not fit into an int, the compiler goes for the smallest container that works which is long int. Then it applies the unary minus operator to the value represented by the literal (remember there are no negative integer literals) and as the value is already seen as a long, it remains a long . This long is then assigned to stepValue

adding the l suffix just tells the compiler to use a long for that value. That matters if you have maths operation which could be conducted as int instead of long like
const long anHour = 1000 * 60 * 60; // challenge on 8 bit µC as 1000, 60 and 60 are seen as int and the result does not fit in a 16 bit int

Yes, that works, thanks. It was surprising because it worked on an UNO with abs() - which is also an 8-bit processor.

Yes - on a UNO it's the macro from Arduino.h which is invoked and so the type is inferred whereas on some platforms it's the stdlib abs() definition and then you get the default int parameter type and that's where you got bitten as it truncates to 16 Bits.

didn't you get a warning when compiling ? (if you activate all warnings ?)

No, I didn't get a warning. I always have all warnings active :wink:

OK - too bad... it's indeed quite a catch

1 Like

Well, because labs() is C++ standard it should work with all platforms? I was about to generally use the ternary operator to be safe.
Thanks, now I can go on updating my library :sunglasses:

yes and Arduino did not #define that one to my knowledge otherwise just #undef abs in your code like they do for SoftwareSerial

If you need something generic, you could consider the following:

template <class T> T myAbs(T const value) {
  return value >= 0 ? value : -value;
}

Usage:

signed char c {5};
signed char mc {-5};
int i {1000};
int mi {-1000};
long l {100000};
long ml {-100000};
float f {1.0};
float mf {-1.0};
double d {1.0};
double md {-1.0};

Serial.println(myAbs(c));
Serial.println(myAbs(mc));
Serial.println(myAbs(i));
Serial.println(myAbs(mi));
Serial.println(myAbs(l));
Serial.println(myAbs(ml));
Serial.println(myAbs(f));
Serial.println(myAbs(mf));
Serial.println(myAbs(d));
Serial.println(myAbs(md));

Output:

5
5
1000
1000
100000
100000
1.00
1.00
1.00
1.00
1 Like

Thanks for your suggestion, but I don't need it that generic. labs() works for me.

First the title on this thread is incorrect.
While I get the issue, the code is not using Abs() but abs()
Details like this matter.

In terms of issues, this is a great example of what can happen when using macros that silently override math functions.
It can silently create unexpected behaviors and confusion, particularly when there is potential silent casting going on.
For example, the macro is typeless so comparisons and results may work differently than when the variable is passed to a function that expects and uses a particular type that is different from the type of the variable involved.
This can often happen on processors like the AVR where ints are only 16 bits but the variables being used are larger than that.

In this case, I would expect differences and odd results between using abs() as a macro vs calling the abs() function) since abs() is for ints and the variable being used was not an int and was larger than a 16 bit int.

Even worse, if Arduino libraries are doing things like undefining the math macros in their library headers, it means that user sketch code can behave differently depending on which library headers have been included.

It is absolutely stupid to be defining macros for math functions, particularly these days.
Yes back in the 80's in the early days of gcc you could often get better & faster code by using a macro vs the function.
These days gcc the optimizer is so good that it will inline the functions so you get the same code as using a macro so there is no need to ever use math macros.

Back in the 80s I worked on projects where speed and size would be critical so we often used macros for math functions, but when we did, we used a different name than the real function, a name that started with a capital letter.
i.e. Abs() etc...
So you had to explicitly go out of your way to get the macro.
This was a very hard rule for all of the developers.

If Arduino wants to override functions like abs() so users don't have to deal with the type issue of having to use the proper typed function like abs() vs labs() vs llabs() vs fabs() vs fabsf() vs fabsl() etc...
(which I think might be a good thing as Arduino is all about trying to make things easier for less technical users)
then they should do it with templates vs using macros.
AND, IMO, they should create a new function name like Abs() so there is no collision or confusion with standard math functions.

--- bill

1 Like

You are right, but unfortunately the website capitalises the first character by itself. I changed the title in such a way, that abs() is not the first word anymore :wink:

1 Like

yeah, that seems to be one of the motivation when they wrote it... may be time for a change...