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 only public symbols #565

Merged
merged 1 commit into from
Mar 9, 2025

Conversation

jwnimmer-tri
Copy link
Contributor

@jwnimmer-tri jwnimmer-tri commented Mar 9, 2025

Unless a function is declared by an installed header, it should be marked either LCM_NO_EXPORT or static.

Running 'nm --extern-only --defined-only liblcm.so' will show the exported symbols. As of these commit, the only text (i.e., function) symbols are the 16 necessary API functions; previously, there were 83.

Unless function is declared by an installed header, it should be
marked either LCM_NO_EXPORT or static.

Running 'nm --extern-only --defined-only liblcm.so' will show the
exported symbols. As of these commit, the only text (i.e., function)
symbols are the 16 necessary API functions; previously, there were 83.
@nosracd
Copy link
Contributor

nosracd commented Mar 9, 2025

Running 'nm --extern-only --defined-only liblcm.so' will show the exported symbols. As of these commit, the only text (i.e., function) symbols are the 16 necessary API functions; previously, there were 83.

Curiously, this doesn't reproduce on the cmake build (I tried macOS and Linux, clang and gcc, debug and release build). But this is definitely true for the meson build

@nosracd nosracd merged commit f6f09bc into lcm-proj:master Mar 9, 2025
58 checks passed
@jwnimmer-tri
Copy link
Contributor Author

jwnimmer-tri commented Mar 9, 2025

Ah, interesting, I hadn't noticed that. Looking now, I see that the generated Makefile from CMake includes a flags.make which sets -fvisibility=hidden by default. I suppose that's coming from here:

# Enable ELF hidden visibility
set(CMAKE_C_VISIBILITY_PRESET "hidden")
set(CMAKE_CXX_VISIBILITY_PRESET "hidden")
set(CMAKE_VISIBILITY_INLINES_HIDDEN 1)

@jwnimmer-tri jwnimmer-tri deleted the no-export-internal branch March 9, 2025 16:26
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