Skip to content

[libcxx] Do not redeclare lgamma_r when targeting the LLVM C library #102036

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 27, 2024
Merged

[libcxx] Do not redeclare lgamma_r when targeting the LLVM C library #102036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
abbae3a
[libcxx] Do not redeclare `lgamma_r` when targeting the LLVM C library
jhuber6 Aug 5, 2024
219c923
Address comments
jhuber6 Aug 14, 2024
a291d3f
Merge branch 'main' into features
jhuber6 Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions libcxx/include/__configuration/platform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,18 @@
// ... add new file formats here ...
#endif

// Need to detect which libc we're using if we're on Linux.
#if defined(__linux__)
// To detect which libc we're using
#if __has_include(<features.h>)
Comment on lines +33 to +34
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When pre-qualifying LLVM 20, we just found out that this breaks some code. Indeed, folks are free to have a header named <features.h> available on their include paths, and in fact it seems not uncommon to have that. So the __has_include(<features.h>) is going to lie, and the #include <features.h> is going to pick up the wrong one anyway.

We'd need to find another header we can include to provide this version macro. Since C lacks a standardized header to provide version stuff (what a shame), we often rely on using some minimal header like <iso646.h> to get implementation-specific values. That should work for glibc, and llvm-libc could be changed to provide a <iso646.h> header. Also, all of llvm-libc's headers should define the feature macros like __LLVM_LIBC__, which I think is not the case right now.

CCing @var-const @jhuber6 @michaelrj-google

# include <features.h>
#endif

#if defined(__linux__)
# if defined(__GLIBC_PREREQ)
# define _LIBCPP_GLIBC_PREREQ(a, b) __GLIBC_PREREQ(a, b)
# else
# define _LIBCPP_GLIBC_PREREQ(a, b) 0
# endif // defined(__GLIBC_PREREQ)
#endif // defined(__linux__)
#endif

#ifndef __BYTE_ORDER__
# error \
Expand Down
3 changes: 2 additions & 1 deletion libcxx/include/__random/binomial_distribution.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ class _LIBCPP_TEMPLATE_VIS binomial_distribution {
}
};

#ifndef _LIBCPP_MSVCRT_LIKE
// The LLVM C library provides this with conflicting `noexcept` attributes.
#if !defined(_LIBCPP_MSVCRT_LIKE) && !defined(__LLVM_LIBC__)
extern "C" double lgamma_r(double, int*);
#endif

Expand Down
Loading