Opened 9 years ago

Closed 9 years ago

#6230 closed bug (fixed)

math.h uses an evil definition for HUGE_VALF

Reported by: TechnoMancer Owned by: nobody
Priority: normal Milestone: R1
Component: System/POSIX Version: R1/Development
Keywords: HUGE_VALF __builtin_huge_valf Cc: plmdvy@…
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

math.h uses a definition of HUGE_VALF that relies on a gcc extension even though it should work in all standards and C++. The solution is to use the builtin that returns it __builtin_huge_valf() however only gcc 3.3 and newer, and clang have this so it will have to fall back to the old one for gcc2. I discovered this while building llvm and had to fix it when building it with clang. I shall make a patch for this and submit it, hopefully it fares better than my other tickets.

Attachments (4)

clang-math.patch (1.1 KB ) - added by kaliber 9 years ago.
Simple math.h changes for clang
math.h.diff (1.3 KB ) - added by scottmc 9 years ago.
updated version of kaliber's patch. Needs to be tested.
math.h.2.diff (1.9 KB ) - added by TechnoMancer 9 years ago.
my version of the patch as I think it should be, tested quickly to make sure nothing complains when changed bits were used
math.h.3.diff (1.8 KB ) - added by TechnoMancer 9 years ago.
Updated path, modified as requested.

Download all attachments as: .zip

Change History (15)

comment:1 by TechnoMancer, 9 years ago

Cc: plmdvy@… added

by kaliber, 9 years ago

Attachment: clang-math.patch added

Simple math.h changes for clang

comment:3 by kaliber, 9 years ago

Has a Patch: set

by scottmc, 9 years ago

Attachment: math.h.diff added

updated version of kaliber's patch. Needs to be tested.

by TechnoMancer, 9 years ago

Attachment: math.h.2.diff added

my version of the patch as I think it should be, tested quickly to make sure nothing complains when changed bits were used

comment:4 by TechnoMancer, 9 years ago

I have provided a patch for this, this will cause gcc4 to also use the builtins instead of that horrible old method, I also added the one for HUGE_VALL for gcc4 and clang. If people dislike the repeated #if.... #else....#endif blocks I can change it to be one #if and the defs then an #else and the old section. Please review the patch.

in reply to:  4 comment:5 by bonefish, 9 years ago

Replying to TechnoMancer:

I have provided a patch for this, this will cause gcc4 to also use the builtins instead of that horrible old method, I also added the one for HUGE_VALL for gcc4 and clang. If people dislike the repeated #if.... #else....#endif blocks I can change it to be one #if and the defs then an #else and the old section. Please review the patch.

A single #if __GNUC__ >= 4 block would indeed be nicer. The comment on the following seems to be off:

44	/* TODO: define HUGE_VALL for long doubles for gcc2 */ 
45	#if __GNUC__ >= 4 
46	#   define HUGE_VALL        __builtin_huge_vall() 
47	#endif 

Otherwise the patch looks OK.

BTW, any clue why, unlike for the other macros, there's a check for a pre-existing definition of HUGE_VAL?

comment:6 by TechnoMancer, 9 years ago

I will fix it up into the two sections then, I changed the comment to put for gcc2 on the end since I implemented it for gcc4 and clang.

No idea why only HUGE_VAL does this, I will make all of them do this too since its good practise.

in reply to:  6 comment:7 by bonefish, 9 years ago

Replying to TechnoMancer:

No idea why only HUGE_VAL does this, I will make all of them do this too since its good practise.

Ah, no, please don't. That just makes the header less readable and after all this header is the place where those macros should be defined.

comment:8 by TechnoMancer, 9 years ago

Must I remove the #ifndef check from HUGE_VAL then or leave it? Should I duplicate it for the gcc4 block?

in reply to:  8 ; comment:9 by bonefish, 9 years ago

Replying to TechnoMancer:

Must I remove the #ifndef check from HUGE_VAL then or leave it? Should I duplicate it for the gcc4 block?

Apparently I introduced that #ifdef myself in hrev24412. It's something gcc 4 seems to consider necessary, so I would at least leave it in that case, though it certainly wouldn't do harm to leave it in both cases.

by TechnoMancer, 9 years ago

Attachment: math.h.3.diff added

Updated path, modified as requested.

in reply to:  9 comment:10 by TechnoMancer, 9 years ago

Replying to bonefish:

Apparently I introduced that #ifdef myself in hrev24412. It's something gcc 4 seems to consider necessary, so I would at least leave it in that case, though it certainly wouldn't do harm to leave it in both cases.

I have changed the file as requested and provided an updated path. If all is satisfactory can you commit it so that clang can be used without known hinderance on future versions of haiku.

comment:11 by bonefish, 9 years ago

Resolution: fixed
Status: newclosed

Thanks! Applied in hrev39307 with some small changes: I removed a few blank lines, indented the comments and added missing spaces in them.

Note: See TracTickets for help on using tickets.