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

[api][visibility] Make the visibility macros more consistent #3363

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

terrelln
Copy link
Contributor

@terrelln terrelln commented Dec 15, 2022

  1. Follow the scheme introduced in PR Explicitly hide static symbols #2501 for both zdict.h and zstd_errors.h.
  2. If the *_VISIBLE macro isn't set, but the *_VISIBILITY macro is, use that. Also make this change for zstd.h, since we probably shouldn't have changed that macro name without backward compatibility in the first place.
  3. Change all references to *_VISIBILITY to *_VISIBLE.
  4. Add ZDICTLIB_STATIC_API to be consistent with zstd.h, this is a no-op change by default.
  5. Document these macros in lib/README.md.

Fixes #3359.

@Cyan4973
Copy link
Contributor

Cyan4973 commented Dec 16, 2022

OK, so switch to *_VISIBLE, only keep *_VISIBILITY for backward compatibility purposes.

Also : we should probably document these build macros,
since they influence the scope of libzstd library,
and are meant to be employed by 3rd party library builders.
Such documentation should be part of lib/README.md.

1. Follow the scheme introduced in PR facebook#2501 for both `zdict.h` and `zstd_errors.h`.
2. If the `*_VISIBLE` macro isn't set, but the `*_VISIBILITY` macro is, use that.
   Also make this change for `zstd.h`, since we probably shouldn't have changed
   that macro name without backward compatibility in the first place.
3. Change all references to `*_VISIBILITY` to `*_VISIBLE`.

Fixes facebook#3359.
@terrelln
Copy link
Contributor Author

Update README

@terrelln terrelln merged commit 358a237 into facebook:dev Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistency: ZSTDLIB_VISIBLE vs ZSTDLIB_VISIBILITY
3 participants