Repair problems ISO C signatures in <cmath>

[ckormanyos]

As discussed at length here, repair C signatures for functions like isnanf, etc. in <cmath>.

[ckormanyos]

See also #17

[ckormanyos]

Lines like this are actually incorrect since ISO C specifies return value of int that is different than the bool specified in ISO C++.

[ckormanyos]

Hi @mikrocoder in your original report, there are some errof messages, but they are incomplete.

At the moment, I can find three clear errors in lines 142, 143 and 144 here.

Could you please find out what happens in your test case if you replace in the three lines mentioned above the type bool with the type int in those cases locally?

In my local test cases, I can reproduce some reported errors. All of the errors that I can find locally on my clean machine are eliminated when these three changes are made. In your original report, however, some other errors were mentioned. So I'd like your feedback if these three lines are the only problematic ones...

[mikrocoder]

Hello,

unfortunately the 3 changes do not help. There are still declarations errors. By the way, the functions return an int only in C. In C++ a bool is returned. So bool would be correct in C++.

The outputs of Microchip Studio are:

Severity	Code	Description	Project	File	Line
Message		'float copysignf(float, float)' previously defined here	avrLibStdCpp	c:\avrtoolchain\avr-gcc-11.3.0_mingw32_binutils2.38\avr\include\math.h	389
Message		'float fabsf(float)' previously defined here	avrLibStdCpp	c:\avrtoolchain\avr-gcc-11.3.0_mingw32_binutils2.38\avr\include\math.h	163
Error		conflicting declaration of C function 'bool isfinitef(float)'	avrLibStdCpp	C:\Users\Worker\Documents\Atmel Studio\7.0\WorkSpace_AVR128DB48\avrLibStdCpp\avrLibStdCpp\math.cc	142
Error		conflicting declaration of C function 'bool isinff(float)'	avrLibStdCpp	C:\Users\Worker\Documents\Atmel Studio\7.0\WorkSpace_AVR128DB48\avrLibStdCpp\avrLibStdCpp\math.cc	137
Error		conflicting declaration of C function 'bool isnanf(float)'	avrLibStdCpp	C:\Users\Worker\Documents\Atmel Studio\7.0\WorkSpace_AVR128DB48\avrLibStdCpp\avrLibStdCpp\math.cc	132
Message		previous declaration 'int isfinitef(float)'	avrLibStdCpp	c:\avrtoolchain\avr-gcc-11.3.0_mingw32_binutils2.38\avr\include\math.h	365
Message		previous declaration 'int isinff(float)'	avrLibStdCpp	C:\avrToolchain\avrLibStdCpp\include\cmath	143
Message		previous declaration 'int isnanf(float)'	avrLibStdCpp	C:\avrToolchain\avrLibStdCpp\include\cmath	142
Error		recipe for target 'math.o' failed	avrLibStdCpp	C:\Users\Worker\Documents\Atmel Studio\7.0\WorkSpace_AVR128DB48\avrLibStdCpp\avrLibStdCpp\Release\Makefile	141
Error		redefinition of 'float copysignf(float, float)'	avrLibStdCpp	C:\Users\Worker\Documents\Atmel Studio\7.0\WorkSpace_AVR128DB48\avrLibStdCpp\avrLibStdCpp\math.cc	147
Error		redefinition of 'float fabsf(float)'	avrLibStdCpp	C:\Users\Worker\Documents\Atmel Studio\7.0\WorkSpace_AVR128DB48\avrLibStdCpp\avrLibStdCpp\math.cc	32

It is not output more precisely. What information is missing?

Then I made the changes from bool to int still in the definitions. In the math.cc. Line 132, 137 and 142.
With this it still does not work. Messages are

Severity	Code	Description	Project	File	Line
Message		'float copysignf(float, float)' previously defined here	avrLibStdCpp	c:\avrtoolchain\avr-gcc-11.3.0_mingw32_binutils2.38\avr\include\math.h	389
Message		'float fabsf(float)' previously defined here	avrLibStdCpp	c:\avrtoolchain\avr-gcc-11.3.0_mingw32_binutils2.38\avr\include\math.h	163
Message		'int isfinitef(float)' previously defined here	avrLibStdCpp	c:\avrtoolchain\avr-gcc-11.3.0_mingw32_binutils2.38\avr\include\math.h	365
Error		recipe for target 'math.o' failed	avrLibStdCpp	C:\Users\Worker\Documents\Atmel Studio\7.0\WorkSpace_AVR128DB48\avrLibStdCpp\avrLibStdCpp\Release\Makefile	141
Error		redefinition of 'float copysignf(float, float)'	avrLibStdCpp	C:\Users\Worker\Documents\Atmel Studio\7.0\WorkSpace_AVR128DB48\avrLibStdCpp\avrLibStdCpp\math.cc	147
Error		redefinition of 'float fabsf(float)'	avrLibStdCpp	C:\Users\Worker\Documents\Atmel Studio\7.0\WorkSpace_AVR128DB48\avrLibStdCpp\avrLibStdCpp\math.cc	32
Error		redefinition of 'int isfinitef(float)'	avrLibStdCpp	C:\Users\Worker\Documents\Atmel Studio\7.0\WorkSpace_AVR128DB48\avrLibStdCpp\avrLibStdCpp\math.cc	142

[ckormanyos]

Hi @mikrocoder I tried with the compiler in standalone mode.

I will try to "hook up" the 11.3 compiler and the STL to my running ATMEL Studio. I need to see if I can reproduce what you observe.

[ckormanyos]

Ahhh... OK. I see that we also need to patch math.cc.

The comprehensive fixes are running in my iso_c_cmath_corrections fork located here.

@mikrocoder it's a bit to ask, but would you be so kind as to be able to check the changes in my fork locally, before we do the next steps?

[ckormanyos]

Hi @mikrocoder the only relevant changes are located in the files src/math.cc and include/cmath

[ckormanyos]

Also, if you are following this thread, there have been numerous recent changes and improvements in handling 32/64-bit double and long double recently in AVR compiler.

So even when we get done with this particular patch, we might do some reviewing. And I would not rule out the possibility that some tiny inconsistencies creep up as we progress forward. but... One issue at a time...

[mikrocoder]

Hello,

I don't understand this as well as you do. Also, as I said, the return value in C++ is actually bool and not int. This should be clarified first before creating forks. What exactly should I check in your files?
64 bit double is still the future. Must be activated by switch. But for this more files are missing. Is not important at the moment.

[mikrocoder]

Hello,

I just remembered that combie had pointed me to a modified math.h back then. This is from Zak Kemple.
https://blog.zakkemble.net/avr-gcc-builds/
I don't know now if this is an old math.h version or an extra modified one.
If I replace the math.h in
C:\avrToolchain\avr-gcc-11.3.0_mingw32_binutils2.38\avr\include
then everything compiles without errors.
math.zip
It works, but it can't be a permanent state. Or? Or do we AVR programmers have to live with this?

Translated with www.DeepL.com/Translator (free version)

[ckormanyos]

Hi @mikrocoder I'm happy you asked.

Floating-point classification functions are confusing and there have been recent specification changes regarding them in C/C++.

I'm not sure I understand each and every point, but the behavior of functions such as isnan, isfinite, etc. differs between C99/C++11 (see here). My understanding is also that the return types differ, bool for C++ and int for C.

In addition to isnan, for instance, there are also macros bearing suffixes f and l for the float and long``double versions of these functions. So you will usually find macros or macro-like functions such as isnanf, isnanl, etc.

All of this leads to a somewhat confusing state of affairs for floating-point classification functionis.

[ckormanyos]

It works, but it can't be a permanent state.

That patch, if I'm not mistaken, deals with mostly 32/64-bit double and long double. It is not a permanent state and the <math.h in your build of GCC 11.3 looks better to me.

The only problem in this entire thread is that some of the C-language functions/macros for floating-point classification, also fabs and copysign had disagreements in <math.h> with the macros/functions present in the STL port here. In fact, the STL port incorrectly listed the return type of the C-language isnan as bool, whereas for the C-linkage version, it should be int, but remain bool when using C++ linkage.

The 11.3 compiler got stricter in these checks (evidently) and had small changes in <math.h>. What I've done on my fork/branch simply accounts for the changes in <math.h>.

I will probably be taking a much closer look into all this off-and-on. But the patch in my fork goes in the right direction. Ultimately, i do not agree with the presence of math.cc and I usually handle this as header-only, non-compiled code in other domains. So we might ultimately change some of this in the future.

• My belief is that your GCC 11.3 has some nice corrections.
• Some artifacts were incorrect in our STL port.
• These are now corrected in my fork and work for pre/post-11.3 (remain unchanged for previous versions, but patched for 11.3 and higher).
• Discussions 32/64-bit are unrelated to this topic and can be tested/handled (if issues arrise) at other times.

[mikrocoder]

Hello,

now that I remembered what I had to change in my avr-gcc 11.2.0 back then to be able to use your avrStdLibCpp, namely the replacement of the math.h, I can safely claim that the original math.h of avr-libc by Jörg Wunsch, has remained unchanged between 11.2.0 and 11.3.0. It has certainly been like this for much longer. So I have to correct my first statement in this regard. I do not know why Zak Kemple has changes in it.

But I ask myself why "you" did not have these error messages under Linux until now. The math.h is the same in all toolchains. No matter if Linux or Windows. I have compared. That would be another question besides that would interest me.

I find it very nice that you take the time for it. 👍 Make you please no stress. 😃

Translated with www.DeepL.com/Translator (free version)

[ckormanyos]

the replacement of the math.h

With time and some work, I'd prefer to avoid changing the C-library header. The idea is that the C++ standar library is built on top of the existing C-headers. We shouldn't need to change headers to account for the C++ library.

But if you're happy with it, OK.

If you do check out my branch on the fork, it also works with both 11.2 as well as your build of 11.3.

The math.h is the same in all toolchains. No matter if Linux or Windows.

Since the erroneous prototypes were in the STL port previously, I actually believe the compiler might have some stricter type-checking. Although I#m not entirely sure of this.

[ckormanyos]

namely the replacement of the math.h, I can safely claim that the original math.h of avr-libc

Cool @mikrocoder I am glad you mentioned this.

Oh this all makes sense now. The file you mention has, as it should, int for the return type of ::isnan and similar. In this file, however, isnanf is expressed as a macro. This is why it works in combination with the incorrect signatures in our STL.

I really feel the urge to change our signatures (in the subroutines having C-linkage) to int instead of bool. This is expected to work with the OP and make avr-libstdcpp better.

Cc: @chris-durand and @salkinium and @rleh

[ckormanyos]

the original math.h of avr-libc by Jörg Wunsch

There is really a lot right with that file. The signatures of things like isnan() return (as they should) int. And also the f and l versions are implemented as macros. This is very ISO-C-conformant.

[mikrocoder]

I consider the exchange of the math.h only as a workaround. So that I could use your lib at all. If that compiles sometime without exchange would be very good. I am also interested that everything works as clean as possible.

[ckormanyos]

interested that everything works as clean as possible

Oh definitely. Indeed. That's the goal.

I believe that this PR at the moment strikes a good compromise. It works with the GCC 11.2 in the modm-io project and also works with the GCC 11.3 compiler that was provided for this PR earlier (it works locally for me).

It's not a perfect state yet since there are a few discrepancies among the various im´plementations of <math.h>.

[chris-durand]

Fixed in #20

[mikrocoder]

Thank you very much. I will try it out soon.

[ckormanyos]

will try it out soon

OK sounds good.
Thanks.

If you happen to run into any inconsistencies remaining in the ISO C99 <math.h> signatures, please feel free to either re-start this thread or start a new one.

[mikrocoder]

Hi,

I have not forgotten you. :-) Found time today and may announce that your changes work. The old code compiles without errors. Just good. :-) Thanks a lot.

Could it be that the new math.h is independent of the toolchain?
I have avr-gcc-11.3.0 with replaced Zak Kemble math.h and a new avr-gcc 12.1.0 with original math.h. There are no collisions. Compiled with both without errors.

[ckormanyos]

announce that your changes work

Cool thanks for the feedback @mikrocoder.

Could it be that the new math.h is independent of the toolchain?

In my opinion, the newer versions of <math.h> are getting better in general regarding correctness of ISO C99 math function signatures. So basically this menas, our patches make compatibility better in general. There may still be little problems we find and correct, but the trend is a good one.

[chris-durand]

Could it be that the new math.h is independent of the toolchain?

The <math.h> incompatibilities are related to the avr-libc version. The changes in avr-libc happened in that commit which recently has been released with avr-libc 2.1.0. We are handling this now by checking for the libc version here. For example, some recent builds of avr-gcc 9.4 and 10.3 ship with avr-libc 2.1.0 and the newer <math.h> now.