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

-Wdouble-promotion clean API #1365

Merged
merged 1 commit into from
Dec 14, 2021
Merged

-Wdouble-promotion clean API #1365

merged 1 commit into from
Dec 14, 2021

Conversation

SoapGentoo
Copy link
Contributor

We have a GPU codebase that we use with htslib where we have to be very careful with float<->double conversions, for which we enable -Wdouble-promotion. Unfortunately, the publicly facing API has two lines that trigger this, mainly due to the fact that printf doesn't have float specifiers. This PR has two commits:

  • The first one makes the public API -Wdouble-promotion clean by adding explicit casts.
  • The second one fixes a simple -Wdouble-promotion issue. This one can be dropped, but it's not very invasive.

I tried fixing the whole codebase, but this required more casts. If desired, I can do those too.

@jkbonfield
Copy link
Contributor

Thanks for raising this. We'll discuss it on our weekly meeting (Tuesday).

Fixing the whole code base is probably quite an undertaking and I'm not sure we really wish to have the maintenance overhead of avoiding automatic float to double promotion everywhere (I count 81 one of them, and likely many more in samtools/bcftools if we touch those), but if this is all that's needed for your work then it seems reasonable to me.

errmod.c Outdated Show resolved Hide resolved
@jkbonfield jkbonfield merged commit e769401 into samtools:develop Dec 14, 2021
@SoapGentoo SoapGentoo deleted the Wdouble-promotion branch December 14, 2021 15:22
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

Successfully merging this pull request may close these issues.

2 participants