facebook / zstd

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

SwiftPM package always produces warnings when `import libzstd` #3328

Open yyaroshevich opened 1 year ago

yyaroshevich commented 1 year ago

Describe the bug Import libzstd into Swift file causes compilation warnings.

To Reproduce Steps to reproduce the behavior:

  1. Clone repro repository git clone https://github.com/yyaroshevich/zstd-warning-swift.git
  2. Compile project (cd zstd-warning-swift; xcodebuild -project ./ZStdWarning.xcodeproj)
  3. Observe compilation is failed due to warnings (warnings as errors are enabled)

Expected behavior No warnings produced when import libzstd.

Screenshots and charts

image

Desktop (please complete the following information):

yyaroshevich commented 1 year ago

I've found out that the warnings are gone if 4 of that macroses are removed from https://github.com/facebook/zstd/blob/dev/lib/module.modulemap This is just an observation I wanted to share.

yyaroshevich commented 1 year ago

Hello @felixhandte! Could you please take a look at this issue (maybe giving hint where to proceed with investigation). It seems like the warnings appeared since changes introduced by https://github.com/facebook/zstd/pull/2953. If I locally revert modulemap to:

module libzstd [extern_c] {
    header "zstd.h"
    export *
}

the warning disappears, but it's likely incorrect per se. So it seems that either problem in modulemap file itself or in way Package.swift is defined. So, could you please help with this issue?

terrelln commented 1 year ago

These are all macros that if they aren't defined, zstd.h and friends define them. I guess that Swift then sees those macros as defined, and errors.

Looking into this.

yyaroshevich commented 1 year ago

Hello there! Is there any updates on this issue (maybe something not reflected here)?

terrelln commented 1 year ago

@yyaroshevich we don't really know how to fix it, and don't have the setup to reproduce it easily. If you find a fix we'd happily accept a PR!

gmilos commented 4 months ago

@terrelln the cause are the macros defined as config_macros in module.modulemap here: https://github.com/facebook/zstd/blob/97291fc5020a8994019ab76cf0cda83a9824374c/lib/module.modulemap#L4-L24. They aren't really config macros, as they are defined here: https://clang.llvm.org/docs/Modules.html#configuration-macros-declaration

(config macros need to be "binary" and cannot be #define-d the way zstd macros are).

AFICT there is no harm in just removing the config_macros block from the module.modulemap. It's broken currently anyway. Would you be able to do it for us?