Skip to content

Hide most of non-public symbols by default #984

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Sep 25, 2021
Merged

Hide most of non-public symbols by default #984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25ed134
Export YAML::detail::node::m_amount
pinotree Apr 18, 2021
762e6f6
CMake: use GenerateExportHeader
pinotree Apr 18, 2021
5fe6157
CMake: hide all the symbols by default
pinotree Apr 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ include(CMakeDependentOption)
include(CheckCXXCompilerFlag)
include(GNUInstallDirs)
include(CTest)
include(GenerateExportHeader)

set(CMAKE_C_VISIBILITY_PRESET hidden)
set(CMAKE_CXX_VISIBILITY_PRESET hidden)
set(CMAKE_VISIBILITY_INLINES_HIDDEN 1)

find_program(YAML_CPP_CLANG_FORMAT_EXE NAMES clang-format)

Expand Down Expand Up @@ -81,6 +86,7 @@ set_property(TARGET yaml-cpp
target_include_directories(yaml-cpp
PUBLIC
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
$<BUILD_INTERFACE:${PROJECT_BINARY_DIR}/include>
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
PRIVATE
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src>)
Expand Down Expand Up @@ -136,6 +142,12 @@ write_basic_package_version_file(
"${PROJECT_BINARY_DIR}/yaml-cpp-config-version.cmake"
COMPATIBILITY AnyNewerVersion)

generate_export_header(yaml-cpp
BASE_NAME YAML_CPP
EXPORT_FILE_NAME "${PROJECT_BINARY_DIR}/include/yaml-cpp/dll.h"
EXPORT_MACRO_NAME YAML_CPP_API
)

configure_file(yaml-cpp.pc.in yaml-cpp.pc @ONLY)

if (YAML_CPP_INSTALL)
Expand All @@ -145,6 +157,9 @@ if (YAML_CPP_INSTALL)
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
install(DIRECTORY ${PROJECT_SOURCE_DIR}/include/
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
FILES_MATCHING PATTERN "*.h")
install(DIRECTORY ${PROJECT_BINARY_DIR}/include/
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
FILES_MATCHING PATTERN "*.h")
install(EXPORT yaml-cpp-targets
Expand Down
33 changes: 0 additions & 33 deletions include/yaml-cpp/dll.h
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion src/node_data.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace YAML {
namespace detail {
std::atomic<size_t> node::m_amount{0};
YAML_CPP_API std::atomic<size_t> node::m_amount{0};
Copy link

@egorpugin egorpugin Sep 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't same YAML_CPP_API should be in header too?

Copy link
Contributor Author

@pinotree pinotree Sep 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, in the definition is enough, for this internal symbol.


const std::string& node_data::empty_scalar() {
static const std::string svalue;
Expand Down