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

zstd does #include within extern "C" scopes. This breaks trying to build a clang module. #4187

Open
alecazam opened this issue Nov 4, 2024 · 6 comments
Assignees

Comments

@alecazam
Copy link

alecazam commented Nov 4, 2024

Describe the bug
#include should never be inside of scopes or namepsaces. But zstd seems to extern "C" and then include various headers within them. This then causes clang module building to fail. I will trying moving all of these outside of these scopes, but in general this is bad practice for code used in so many applications..

import of C++ module 'std_stdint_h' appears within extern "C" language linkage specification

#if  !defined (__VMS) && (defined (__cplusplus) || (defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) /* C99 */) )
#  if defined(_AIX)
#    include <inttypes.h>
#  else
#    include <stdint.h> /* intptr_t */ <-
#  endif

Another example from the concatenated zstddeclib.cpp

#if defined (__cplusplus)
extern "C" {
#endif

/*-****************************************
*  Dependencies
******************************************/
#include <stddef.h>  /* size_t, ptrdiff_t */

To Reproduce
Steps to reproduce the behavior:

  1. turn on -fmodules and -fcxx-modules
  2. setup a module.modulemap with zstd in it
  3. Watch compile fail trying to build zstd

Expected behavior
All headers should just be included first and outside of any scoping. Then any concatenation should just work. extern "C" should only wrap function calls and structs for naming reasons.

Screenshots and charts
None

Desktop (please complete the following information):

  • OS: macOS
  • Version 15.0
  • Compiler clang
  • Flags any
  • Other relevant hardware specs 8/4 cores
  • Build system Xcode

Additional context
None

@alecazam
Copy link
Author

alecazam commented Nov 4, 2024

Alternatively, just have one file with all #include and then prepend that at the top of the concatenated files.

@Cyan4973
Copy link
Contributor

Cyan4973 commented Nov 5, 2024

example from the concatenated zstddeclib.cpp

Is the issue specific to the amalgamated single-file library ?

@alecazam
Copy link
Author

alecazam commented Nov 5, 2024

Yes, from what I see one concatenated header will start an extern "C" scope, and then do #include of other zstd files. So end up with #include nested inside of these scopes. I've commented out the includes here.

This is from the top of zstd.h. I've just commented out the includes. But zstd.h, zstddeclib.cpp, and zstd.cpp all have similar patterns.

#if defined (__cplusplus)
extern "C" {
#endif

#ifndef ZSTD_H_235446
#define ZSTD_H_235446

/* ======   Dependency   ======*/
//#include <limits.h>   /* INT_MAX */
//#include <stddef.h>   /* size_t */

@Cyan4973
Copy link
Contributor

Cyan4973 commented Nov 5, 2024

The amalgamated single file libraries are supposed to be *.c.
Why are they renamed as *.cpp?

@Victor-C-Zhang
Copy link
Contributor

Hey @alecazam, can you share the contents of your module.modulemap and the exact commands you used to repro the error?

@Victor-C-Zhang
Copy link
Contributor

Update: I was unable to repro the build failure with the info provided.
Here's what I did:

  1. Generate zstd.c from create_single_file_library.sh
  2. Copy zstd.c and lib/zstd.h, lib/zdict.h, lib/zstd_error.h, lib/module.modulemap to a fresh directory (outside zstd repo) ~/module_test
  3. In the directory ~/module_test use the following main.cpp:
#include "zzz.h"
#include <iostream>
#include <string>

int main() {
  std::cout << "Hello, world!" << std::endl;
  std::string input = "abcdabcdabcdabcdabcdabcdabcdabcd";
  std::string output('\0', input.size() * 100);
  auto res = ZSTD_compress(output.data(), output.size(),
                           input.data(), input.size(), 3);
  std::cout << res << std::endl;

  return 0;
}
  1. For safety, rename the module libzstd -> zzz, and rename zstd.h -> zzz.h, zstd.c -> zzz.c. Also do this in the module.modulemap.
  2. Build with module
clang -shared -o libzzz.dylib zzz.c
clang++ -std=c++20 main.cpp -fmodules -fcxx-modules -L. -lzzz
  1. It runs:
% ./a.out
Hello, world!
18446744073709551546

Apple clang version 15.0.0 (clang-1500.3.9.4)
Target: arm64-apple-darwin24.1.0
Thread model: posix

@alecazam can you share your repro steps?

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

No branches or pull requests

3 participants