Skip to content

Commit c67034e

Browse files
authored
Fix C++11 ABI breakage when compiled with C++17 #1668 (#1675)
* ci: suppress Node 20 deprecation and missing python-version warnings - Injects `FORCE_JAVASCRIPT_ACTIONS_TO_NODE24=true` in all workflows to opt-in to Node.js 24 and suppress the deprecation warnings from multiple GitHub Actions. - Specifies `python-version: '3.x'` for `actions/setup-python@v5` in `meson.yml` to fix the missing input warning. * Fix C++11 ABI breakage when compiled with C++17 (#1668) When JSONCPP_HAS_STRING_VIEW was defined, the library dropped the `const char*` and `const String&` overloads for `operator[]`, `get`, `removeMember`, and `isMember`, breaking ABI compatibility for projects consuming the library with C++11. This change unconditionally declares and defines the legacy overloads so they are always exported, restoring compatibility. * ci: add ABI compatibility matrix workflow This adds a new GitHub Actions workflow to verify ABI compatibility across C++ standard boundaries. It explicitly tests the scenario where JsonCpp is built with one standard (e.g., C++11) and consumed by an application built with a newer one (e.g., C++23), and vice versa. To facilitate testing the specific `std::string_view` boundary that is conditionally compiled, a new `stringView` demo application has been added to the `example/` directory and is consumed directly by the CI matrix to ensure standard library symbols link correctly across standard versions, build types (shared/static), and operating systems. * fix: inline std::string_view methods to prevent ABI breaks This commit completely eliminates the ABI breakage that occurs across C++ standard boundaries when using `std::string_view`. Previously, when the library was built with C++17+, CMake would leak `JSONCPP_HAS_STRING_VIEW=1` as a PUBLIC definition. A C++11 consumer would receive this definition, attempt to parse the header, and fail with compiler errors because `std::string_view` is not available in their environment. Conversely, if the library was built in C++11 (without `string_view` symbols), a C++17 consumer would naturally define `JSONCPP_HAS_STRING_VIEW` based on `__cplusplus` inside `value.h`. The consumer would then call the declared `string_view` methods, resulting in linker errors because the methods weren't compiled into the library. By moving all `std::string_view` overloads directly into `value.h` as `inline` methods that delegate to the fundamental `const char*, const char*` methods: 1. The consumer's compiler dictates whether the overloads are visible (via `__cplusplus >= 201703L`). 2. The consumer compiles the inline wrappers locally, removing any reliance on the library's exported symbols for `std::string_view`. 3. CMake no longer needs to pollute the consumer's environment with PUBLIC compile definitions. * run clang format * finish clang format
1 parent 2c2754f commit c67034e

File tree

7 files changed

+151
-93
lines changed

7 files changed

+151
-93
lines changed
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
name: ABI Compatibility
2+
3+
on: [check_run, push, pull_request]
4+
5+
env:
6+
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true
7+
8+
jobs:
9+
abi-compatibility:
10+
runs-on: ${{ matrix.os }}
11+
12+
strategy:
13+
fail-fast: false
14+
matrix:
15+
os: [ubuntu-latest, windows-latest, macos-latest]
16+
shared_libs: [ON, OFF]
17+
include:
18+
- jsoncpp_std: 11
19+
app_std: 23
20+
- jsoncpp_std: 23
21+
app_std: 11
22+
23+
steps:
24+
- name: checkout project
25+
uses: actions/checkout@v4
26+
27+
- name: build and install JsonCpp (C++${{ matrix.jsoncpp_std }})
28+
shell: bash
29+
run: |
30+
mkdir build-jsoncpp
31+
cd build-jsoncpp
32+
cmake .. -DCMAKE_CXX_STANDARD=${{ matrix.jsoncpp_std }} \
33+
-DCMAKE_CXX_STANDARD_REQUIRED=ON \
34+
-DCMAKE_INSTALL_PREFIX=$GITHUB_WORKSPACE/install-jsoncpp \
35+
-DBUILD_SHARED_LIBS=${{ matrix.shared_libs }} \
36+
-DJSONCPP_WITH_TESTS=OFF
37+
cmake --build . --config Release
38+
cmake --install . --config Release
39+
40+
- name: create example app
41+
shell: bash
42+
run: |
43+
mkdir example-app
44+
cat << 'EOF' > example-app/CMakeLists.txt
45+
cmake_minimum_required(VERSION 3.10)
46+
project(abi_test)
47+
48+
find_package(jsoncpp REQUIRED CONFIG)
49+
50+
add_executable(abi_test stringView.cpp)
51+
target_link_libraries(abi_test PRIVATE JsonCpp::JsonCpp)
52+
EOF
53+
54+
cp $GITHUB_WORKSPACE/example/stringView/stringView.cpp example-app/stringView.cpp
55+
56+
- name: build example app (C++${{ matrix.app_std }})
57+
shell: bash
58+
run: |
59+
cd example-app
60+
mkdir build
61+
cd build
62+
cmake .. -DCMAKE_CXX_STANDARD=${{ matrix.app_std }} \
63+
-DCMAKE_CXX_STANDARD_REQUIRED=ON \
64+
-DCMAKE_PREFIX_PATH=$GITHUB_WORKSPACE/install-jsoncpp
65+
cmake --build . --config Release
66+
67+
- name: run example app
68+
shell: bash
69+
run: |
70+
if [ "$RUNNER_OS" == "Windows" ]; then
71+
export PATH=$GITHUB_WORKSPACE/install-jsoncpp/bin:$PATH
72+
./example-app/build/Release/abi_test.exe
73+
elif [ "$RUNNER_OS" == "macOS" ]; then
74+
export DYLD_LIBRARY_PATH=$GITHUB_WORKSPACE/install-jsoncpp/lib:$DYLD_LIBRARY_PATH
75+
./example-app/build/abi_test
76+
else
77+
export LD_LIBRARY_PATH=$GITHUB_WORKSPACE/install-jsoncpp/lib:$LD_LIBRARY_PATH
78+
./example-app/build/abi_test
79+
fi

example/BUILD.bazel

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,9 @@ cc_binary(
3333
srcs = ["stringWrite/stringWrite.cpp"],
3434
deps = ["//:jsoncpp"],
3535
)
36+
37+
cc_binary(
38+
name = "stringView",
39+
srcs = ["stringView/stringView.cpp"],
40+
deps = ["//:jsoncpp"],
41+
)

example/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ set(EXAMPLES
44
readFromStream
55
stringWrite
66
streamWrite
7+
stringView
78
)
89
add_definitions(-D_GLIBCXX_USE_CXX11_ABI)
910

example/stringView/stringView.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
#include "json/json.h"
2+
#include <cstdlib>
3+
#include <iostream>
4+
5+
#if defined(JSONCPP_HAS_STRING_VIEW)
6+
#include <string_view>
7+
#endif
8+
9+
/**
10+
* \brief Example using std::string_view with JsonCpp.
11+
*/
12+
int main() {
13+
Json::Value root;
14+
root["key"] = "value";
15+
16+
#if defined(JSONCPP_HAS_STRING_VIEW)
17+
std::cout << "Has string_view support" << std::endl;
18+
std::string_view sv("key");
19+
if (root.isMember(sv)) {
20+
std::cout << root[sv].asString() << std::endl;
21+
}
22+
#else
23+
std::cout << "No string_view support" << std::endl;
24+
if (root.isMember("key")) {
25+
std::cout << root["key"].asString() << std::endl;
26+
}
27+
#endif
28+
return EXIT_SUCCESS;
29+
}

include/json/value.h

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,8 @@ class JSON_API Value {
357357
Value(const StaticString& value);
358358
Value(const String& value);
359359
#ifdef JSONCPP_HAS_STRING_VIEW
360-
Value(std::string_view value);
360+
inline Value(std::string_view value)
361+
: Value(value.data(), value.data() + value.length()) {}
361362
#endif
362363
Value(bool value);
363364
Value(std::nullptr_t ptr) = delete;
@@ -405,7 +406,14 @@ class JSON_API Value {
405406
/** Get string_view of string-value.
406407
* \return false if !string. (Seg-fault if str is NULL.)
407408
*/
408-
bool getString(std::string_view* str) const;
409+
inline bool getString(std::string_view* str) const {
410+
char const* begin;
411+
char const* end;
412+
if (!getString(&begin, &end))
413+
return false;
414+
*str = std::string_view(begin, static_cast<size_t>(end - begin));
415+
return true;
416+
}
409417
#endif
410418
Int asInt() const;
411419
UInt asUInt() const;
@@ -496,12 +504,19 @@ class JSON_API Value {
496504
#ifdef JSONCPP_HAS_STRING_VIEW
497505
/// Access an object value by name, create a null member if it does not exist.
498506
/// \param key may contain embedded nulls.
499-
Value& operator[](std::string_view key);
507+
inline Value& operator[](std::string_view key) {
508+
return resolveReference(key.data(), key.data() + key.length());
509+
}
500510
/// Access an object value by name, returns null if there is no member with
501511
/// that name.
502512
/// \param key may contain embedded nulls.
503-
const Value& operator[](std::string_view key) const;
504-
#else
513+
inline const Value& operator[](std::string_view key) const {
514+
Value const* found = find(key.data(), key.data() + key.length());
515+
if (!found)
516+
return nullSingleton();
517+
return *found;
518+
}
519+
#endif
505520
/// Access an object value by name, create a null member if it does not exist.
506521
/// \note Because of our implementation, keys are limited to 2^30 -1 chars.
507522
/// Exceeding that will cause an exception.
@@ -516,7 +531,6 @@ class JSON_API Value {
516531
/// that name.
517532
/// \param key may contain embedded nulls.
518533
const Value& operator[](const String& key) const;
519-
#endif
520534
/** \brief Access an object value by name, create a null member if it does not
521535
* exist.
522536
*
@@ -533,16 +547,17 @@ class JSON_API Value {
533547
#ifdef JSONCPP_HAS_STRING_VIEW
534548
/// Return the member named key if it exist, defaultValue otherwise.
535549
/// \note deep copy
536-
Value get(std::string_view key, const Value& defaultValue) const;
537-
#else
550+
inline Value get(std::string_view key, const Value& defaultValue) const {
551+
return get(key.data(), key.data() + key.length(), defaultValue);
552+
}
553+
#endif
538554
/// Return the member named key if it exist, defaultValue otherwise.
539555
/// \note deep copy
540556
Value get(const char* key, const Value& defaultValue) const;
541557
/// Return the member named key if it exist, defaultValue otherwise.
542558
/// \note deep copy
543559
/// \param key may contain embedded nulls.
544560
Value get(const String& key, const Value& defaultValue) const;
545-
#endif
546561
/// Return the member named key if it exist, defaultValue otherwise.
547562
/// \note deep copy
548563
/// \note key may contain embedded nulls.
@@ -588,27 +603,29 @@ class JSON_API Value {
588603
/// \pre type() is objectValue or nullValue
589604
/// \post type() is unchanged
590605
#if JSONCPP_HAS_STRING_VIEW
591-
void removeMember(std::string_view key);
592-
#else
606+
inline void removeMember(std::string_view key) {
607+
removeMember(key.data(), key.data() + key.length(), nullptr);
608+
}
609+
#endif
593610
void removeMember(const char* key);
594611
/// Same as removeMember(const char*)
595612
/// \param key may contain embedded nulls.
596613
void removeMember(const String& key);
597-
#endif
598614
/** \brief Remove the named map member.
599615
*
600616
* Update 'removed' iff removed.
601617
* \param key may contain embedded nulls.
602618
* \return true iff removed (no exceptions)
603619
*/
604620
#if JSONCPP_HAS_STRING_VIEW
605-
bool removeMember(std::string_view key, Value* removed);
606-
#else
621+
inline bool removeMember(std::string_view key, Value* removed) {
622+
return removeMember(key.data(), key.data() + key.length(), removed);
623+
}
624+
#endif
607625
bool removeMember(String const& key, Value* removed);
608626
/// Same as removeMember(const char* begin, const char* end, Value* removed),
609627
/// but 'key' is null-terminated.
610628
bool removeMember(const char* key, Value* removed);
611-
#endif
612629
/// Same as removeMember(String const& key, Value* removed)
613630
bool removeMember(const char* begin, const char* end, Value* removed);
614631
/** \brief Remove the indexed array element.
@@ -622,15 +639,16 @@ class JSON_API Value {
622639
#ifdef JSONCPP_HAS_STRING_VIEW
623640
/// Return true if the object has a member named key.
624641
/// \param key may contain embedded nulls.
625-
bool isMember(std::string_view key) const;
626-
#else
642+
inline bool isMember(std::string_view key) const {
643+
return isMember(key.data(), key.data() + key.length());
644+
}
645+
#endif
627646
/// Return true if the object has a member named key.
628647
/// \note 'key' must be null-terminated.
629648
bool isMember(const char* key) const;
630649
/// Return true if the object has a member named key.
631650
/// \param key may contain embedded nulls.
632651
bool isMember(const String& key) const;
633-
#endif
634652
/// Same as isMember(String const& key)const
635653
bool isMember(const char* begin, const char* end) const;
636654

src/lib_json/CMakeLists.txt

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,6 @@ if(BUILD_SHARED_LIBS)
131131

132132
target_compile_features(${SHARED_LIB} PUBLIC ${REQUIRED_FEATURES})
133133

134-
if(JSONCPP_HAS_STRING_VIEW)
135-
target_compile_definitions(${SHARED_LIB} PUBLIC JSONCPP_HAS_STRING_VIEW=1)
136-
endif()
137134

138135
target_include_directories(${SHARED_LIB} PUBLIC
139136
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
@@ -168,9 +165,6 @@ if(BUILD_STATIC_LIBS)
168165

169166
target_compile_features(${STATIC_LIB} PUBLIC ${REQUIRED_FEATURES})
170167

171-
if(JSONCPP_HAS_STRING_VIEW)
172-
target_compile_definitions(${STATIC_LIB} PUBLIC JSONCPP_HAS_STRING_VIEW=1)
173-
endif()
174168

175169
target_include_directories(${STATIC_LIB} PUBLIC
176170
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
@@ -198,9 +192,6 @@ if(BUILD_OBJECT_LIBS)
198192

199193
target_compile_features(${OBJECT_LIB} PUBLIC ${REQUIRED_FEATURES})
200194

201-
if(JSONCPP_HAS_STRING_VIEW)
202-
target_compile_definitions(${OBJECT_LIB} PUBLIC JSONCPP_HAS_STRING_VIEW=1)
203-
endif()
204195

205196
target_include_directories(${OBJECT_LIB} PUBLIC
206197
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>

0 commit comments

Comments
 (0)