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

export missed symbol #2293

Merged
merged 1 commit into from
May 19, 2021
Merged

export missed symbol #2293

merged 1 commit into from
May 19, 2021

Conversation

sergiud
Copy link
Contributor

@sergiud sergiud commented May 18, 2021

When compiling with -fvisibility=hidden, linker errors occur because a symbol was not marked as being exported. The final keyword is not necessary for a class that is already marked as final.

How to reproduce:

CXXFLAGS=-fvisibility=hidden cmake -S . -B build/ -DBUILD_SHARED_LIBS:BOOL=ON -GNinja
cmake --build build/

Log output

@sergiud sergiud marked this pull request as draft May 18, 2021 18:25
@sergiud sergiud force-pushed the export branch 2 times, most recently from 45c037f to 5a6edc3 Compare May 18, 2021 21:46
@sergiud
Copy link
Contributor Author

sergiud commented May 18, 2021

@vitaut I've updated the error description and marked other missed symbols for export.

It might be a good idea to set the visibility to hidden by default because this leaves more room for compiler optimizations.

In CMake, the preferred way for generating export macros is to use the GenerateExportHeader module which creates a header with toolchain specific export defines. On a downside, this introduces an additional header. Is this something fmt would still consider?

@vitaut
Copy link
Contributor

vitaut commented May 18, 2021

Is this something fmt would still consider?

No, we try to keep the number of headers small and especially avoid any generated headers.

@vitaut
Copy link
Contributor

vitaut commented May 19, 2021

There are some build failures.

@sergiud
Copy link
Contributor Author

sergiud commented May 19, 2021

I could reproduce the problem with -flto enabled (clang 11.0 and gcc 11.1).

Enable `-fvisibility=hidden` and `-fvisibility-inlines-hidden` by default in CI builds to ensure all public symbols are exported correctly.
@sergiud sergiud marked this pull request as ready for review May 19, 2021 22:18
@vitaut vitaut merged commit 13e6529 into fmtlib:master May 19, 2021
@vitaut
Copy link
Contributor

vitaut commented May 19, 2021

Thank you!

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

Successfully merging this pull request may close these issues.

2 participants