estraier / tkrzw

a set of implementations of DBM
Apache License 2.0
164 stars 20 forks source link

Make it compile with LLVM #32

Closed puffetto closed 2 years ago

puffetto commented 2 years ago

Hello, Nowadays the official compiler on FreeBSD is LLVM; LLVM on FreeBSD is a bit picky about standard conformance: if you say "-D_POSIX_C_SOURCE=999999L" it means to compile making visible ONLY what is available in "the latest version on POSIX". In example "isascii()" is currecntly deprecated. The PR should not affect any other platform, but I could not test on Linux. With this change and the PR I submitted a few hours ago for tkrzw-rpc both things compile happily on FreeBSD/LLVM, link with systems-provided gRPC, abseil and protobuf ports, and so far seem to be quite stable.

estraier commented 2 years ago

In that case, I'd like to make my code be compatible to the LLVM requirement with -D_POSIX_C_SOURCE=999999L. I myself has been making effort to avoid deprecated APIs. What errors did you observe with -D_POSIX_C_SOURCE=999999L ?

puffetto commented 2 years ago

Yes, Clang+LLVM.

Point is that this is considered normal behaviour, some library functions that tkrzw uses are not in the posix standard (like memmem()) or have been removed in the most recent versions (like isalpha()); while they are provided on FreeBSD the system headers hide them if you ask to have current posix conformance.

For sure with D_POSIX_C_SOURCE=999999L on FreeBSD CLANG/LLVM hides isalpha() and memmem(); if you care I investigate more, but I am quite confident that at least on this platform not setting _POSIX_C_SOURCE is the best option.

Please notice that the proposed PR does not change anything on the other platforms.

puffetto commented 2 years ago

I confirm that there are only these two issues: isascii() and memmem(). Leaving the memmem() patch in place and adding a wild "int isascii(int);" implementation the code compiles and runs.

puffetto commented 2 years ago

Final note: after digging a bit it is not even tkrzw using isascii(), it is the STL library itself; there has been a similar "bug report" in the past on FreeBSD's, https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=257827 The general consensus seem to be that _POSIX_C_SOURCE is only valid for 'C' code, not 'C++', as it requires a C99 compiler and as C++ bindings are not defined for POSIX. Also on Solaris using the feature test macros in C++ is defined as undefined behaviour.

estraier commented 2 years ago

Now I understand the situation. One bothering issue is that _POSIX_C_SOURCE is necessary on some platforms including MacOS to enable some POSIX API functions. To me, saying "_POSIX_C_SOURCE is only valid for 'C' code, not 'C++'" sounds too much. Calling POSIX "C" API from C++ code is a daily necessity for UNIX programmers.

Anyway, working code is much better than non-working code. So, I'd like to take your suggestion.

puffetto commented 2 years ago

Calling POSIX "C" API from C++ code is a daily necessity for UNIX programmers.

Sure it is, the problem is just including STL or other "c++" headers with that macro defined.

I am not sure it's the cleaner way, but have seen things like this done:

#ifdef _POSIX_C_SOURCE
#define __MY_PRIVATE_PCS _POSIX_C_SOURCE
#undef _POSIX_C_SOURCE
#endif 

#define _POSIX_C_SOURCE 999999L
#include "someposix.h"
#include "anotherposix.h"
#include "onemoreposix.h"
#undef _POSIX_C_SOURCE

#ifdef __MY_PRIVATE_PCS
#define _POSIX_C_SOURCE __MY_PRIVATE_PCS
#undef __MY_PRIVATE_PCS
#endif

... or the other way around to isolate inclusion of STL/c++ headers. It's a dirty hack but it works and it is pretty portable.

Maybe on platforms where posix stuff "disappears" using in example<cstdio> instead of <stdio.h> solves the issue (basides having the stuff land in the std:: namespace) ? Just guessing, never digged into that.

All the best,

A.

estraier commented 1 year ago

Thanks for the suggestion. I wish that the standard C++ library takes such a measure to encapsulate messy things from application programmers... Anyway, blacklisting some system names seems enough for the time being. Let's see how it goes.

On Fri, Apr 1, 2022 at 4:13 AM Andrea Cocito @.***> wrote:

Calling POSIX "C" API from C++ code is a daily necessity for UNIX programmers.

Sure it is, the problem is just including STL or other "c++" headers with that macro defined.

I am not sure it's the cleaner way, but have seen things like this done:

ifdef _POSIX_C_SOURCE

define __MY_PRIVATE_PCS _POSIX_C_SOURCE

undef _POSIX_C_SOURCE

endif

define _POSIX_C_SOURCE 999999L

include "someposix.h"

include "anotherposix.h"

include "onemoreposix.h"

undef _POSIX_C_SOURCE

ifdef __MY_PRIVATE_PCS

define _POSIX_C_SOURCE __MY_PRIVATE_PCS

undef __MY_PRIVATE_PCS

endif

... or the other way around to isolate inclusion of STL/c++ headers. It's a dirty hack but it works and it is pretty portable.

Maybe on platforms where posix stuff "disappears" using in example instead of solves the issue (basides having the stuff land in the std:: namespace) ? Just guessing, never digged into that.

All the best,

A.

— Reply to this email directly, view it on GitHub https://github.com/estraier/tkrzw/pull/32#issuecomment-1085002196, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQGJVRFFXHDAIXMN3QY5NTDVCX2NZANCNFSM5SBV2VLA . You are receiving this because you modified the open/close state.Message ID: @.***>