facebook / zstd

Zstandard - Fast real-time compression algorithm
http://www.zstd.net
Other
23.75k stars 2.11k forks source link

C++ Builder and mem.h ambiguity #3911

Open encode84 opened 9 months ago

encode84 commented 9 months ago

It's very weird issue, but currently we have ambiguity between "mem.h" from ZSTD and from Embarcadero RAD Studio (C++ Builder which is actually based on clang) See: https://docwiki.embarcadero.com/RADStudio/Alexandria/en/Memset,_wmemset Currently it is not possible to compile ZSTD with RAD Studio unless rename "mem.h" to, say, "zstd_mem.h"

All other your programs like LZ4 and XXHASH have no compile issues with C++ Builder.

Cyan4973 commented 9 months ago

Indeed, that's unexpected. zstd is now using relative #include path to avoid such issues. So the source code should state #include "path/to/mem.h", and not #include <mem.h>, which should be enough to avoid any confusion between 2 header files with the same name.

Which version of zstd is being used ?

encode84 commented 9 months ago

It's the latest ZSTD - 1.5.5. Also, many compiler headers use - so we got tons of nightmare errors form e.g. winnt.h

Cyan4973 commented 9 months ago

So the source code should state #include "path/to/mem.h", and not #include ,

I checked, and there is no place in the source code where <mem.h> is included.

It's the latest ZSTD - 1.5.5. Also, many compiler headers use - so we got tons of nightmare errors form e.g. winnt.h

Do you mean it happens with some other programs and libraries too ?

encode84 commented 9 months ago

Do you mean it happens with some other programs and libraries too ?

I have never experienced any problems with C++ Builder and other libraries like libdeflate, zlib, xxhash, lz4, etc. The use case - my CHK project there I use e.g. LZ4 as a quick file compression test. Lots of things going on inside, including the Windows API. What I mean is that some Compiler's system headers use which cause troubles. At first, I even not really get what’s going on – why I can’t compile ZSTD here, maybe the project is too heavy or we have some compiler incompatibilities?! I was surprised why I get errors like “use of undeclared identifier ‘memset’” from locked system headers like “winnt.h”, “sysset.h”, “xscan.h”, “xutility.h”, “xmemory.h”, etc.? Now I got it, all these got catched by ZSTD's "mem.h", and didn’t include the actual system

Cyan4973 commented 8 months ago

So, if I do understand properly, you mean other parts linked by the project, which do #include <mem.h>, are actually accessing Zstandard's "zstd/lib/common/mem.h" instead, which obviously messes the declaration and linking stages.

That's pretty weird. The environment must be altered pretty strongly to reach such a situation. Even if zstd/lib/common/ was part of your #include path (which is unnecessary), it should still come after the system include path when the code requests #include <mem.>. That's the meaning of the angle brackets < > : look for system include directory first, whereas " " means : look for user-defined include directories first.

As a first investigation step, let's check: is zstd/lib/common/ defined as part of your include path ? And if yes, can this setting be removed ?

CyberGonzo commented 8 months ago

I seem to have a similar problem. Not really the same, but similar (I think related) in that C++ Builder (Clang actually) says that needs to be included.

I'm running in circles trying to add includes etc. Using v 1.5.5, downloaded today.

To elaborate. I created a new static library project with C++ Builder 12 (clang for both 32 and 64 bit) and I added all .c files from /common/ and /decompress/ to the project. I then built the .dll and *.a files semi successfully. Semi because I get these weird warnings, yet the dll/a files get built.

[C++ Warning] mem.h(208, 13):  implicitly declaring library function 'memcpy' with type 'void *(void *, const void *, unsigned int)'
..
[C++ Warning] bitstream.h(254, 23):  implicitly declaring library function 'memset' with type 'void *(void *, int, unsigned int)'
...
[C++ Warning] zstd_internal.h(193, 4):  implicitly declaring library function 'memmove' with type 'void *(void *, const void *, unsigned int)'

I get a lot more warnings (but always memcpy, memset and memmove), you get the gist.

When I add the library to a project and use a function, for instance ZSTD_decompress(), after successfully building I get linker issues:

[Linker Error] Error: Unresolved external '_ZSTD_memcpy' referenced from MyLibLocation\ZSTD.LIB|huf_decompress

Which goes straight to the heart of the warnings ..

PS. #include <string.h> is present in zstd_deps.h where ZSTD_memcpy and the two other functions are defined

What am I missing ? (aside from a good night's sleep ;)

PS. prototyping the functions in zstd_deps.h, like so:

void *memcpy(void *dest, const void *src, size_t n) ;
void *memset(void *str, int c, size_t n) ;
void *memmove(void *str1, const void *str2, size_t n) ;

gets rid of the lib building warnings, but the linker problems in the main project that uses the static lib remain

CyberGonzo commented 8 months ago

Before I move on, and for others, should they be in the same situation, the solution is to rename mem.h (I used zstd_mem.h) and apply that change to all #includes that reference mem.h It's also important to delete mem.h so that it cannot be loaded

I hope next version of the library can take that in account

The problem is a circular include issue as mem.h gets processed before it should. I think it's an Embarcadero issue (that you probably should work around), because I believe the #include <string.h> in zstd_deps.h is what causes the issue. It causes mem.h to be included again and because of the compile guards in zstd_deps.h ZSTD_memcpy gets processed before it's properly defined in zstd_deps.h It's messy .. I stopped looking, we all have better things to do ;)

OMG. I should have read the original post better, it's basically the same conclusion -lol-.