Skip to content

Commit 0891eb1

Browse files
Davide Faconticlaude
andcommitted
fix(pj_base): portable float parsing for libc++ (no std::from_chars)
SettingsValue::toDouble instantiated std::from_chars<double>, a deleted overload in Apple Clang's libc++, which broke the macOS build of 0.4.1. Extract a reusable PJ::parseNumber<T> helper (pj_base/number_parse.hpp) that routes floating-point through std::strto* while keeping integers on std::from_chars, and have SettingsValue use it so plugins and PJ4 can reuse the same portable parse. Also add macOS and pre-commit CI workflows: core had Linux and Windows coverage but no macOS, which is why the libc++ break shipped unnoticed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c91d7e4 commit 0891eb1

4 files changed

Lines changed: 138 additions & 15 deletions

File tree

.github/workflows/macos-ci.yml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
name: macOS CI
2+
on:
3+
push:
4+
branches: [development, main]
5+
pull_request:
6+
branches: [development, main]
7+
types: [opened, synchronize, reopened, ready_for_review]
8+
workflow_dispatch: {}
9+
10+
concurrency:
11+
group: ${{ github.workflow }}-${{ github.ref }}
12+
cancel-in-progress: true
13+
14+
jobs:
15+
build:
16+
if: github.event.pull_request.draft == false
17+
runs-on: macos-15-intel
18+
steps:
19+
- uses: actions/checkout@v4
20+
21+
- uses: conan-io/setup-conan@v1
22+
with:
23+
cache_packages: true
24+
25+
- name: Install cmake and ninja
26+
run: pip install cmake ninja
27+
28+
- name: Conan install
29+
run: >
30+
conan install . --output-folder=build --build=missing
31+
-s build_type=RelWithDebInfo -s compiler.cppstd=20
32+
-o "plotjuggler_core/*:with_tests=True"
33+
-o "plotjuggler_core/*:with_parquet_example=False"
34+
35+
- name: Configure
36+
run: >
37+
cmake -S . -B build -G Ninja
38+
-DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/build/conan_toolchain.cmake
39+
-DCMAKE_BUILD_TYPE=RelWithDebInfo
40+
-DPJ_BUILD_PARQUET_IMPORT_EXAMPLE=OFF
41+
42+
- name: Build
43+
run: cmake --build build
44+
45+
- name: Test
46+
env:
47+
QT_QPA_PLATFORM: offscreen
48+
run: ctest --test-dir build --output-on-failure

.github/workflows/pre-commit.yml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
name: pre-commit
2+
on:
3+
push:
4+
branches: [development, main]
5+
pull_request:
6+
branches: [development, main]
7+
types: [opened, synchronize, reopened, ready_for_review]
8+
workflow_dispatch: {}
9+
10+
concurrency:
11+
group: ${{ github.workflow }}-${{ github.ref }}
12+
cancel-in-progress: true
13+
14+
jobs:
15+
pre-commit:
16+
if: github.event.pull_request.draft == false
17+
runs-on: ubuntu-22.04
18+
steps:
19+
- uses: actions/checkout@v4
20+
21+
- uses: actions/setup-python@v5
22+
with:
23+
python-version: '3.12'
24+
25+
- uses: pre-commit/action@v3.0.1
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
#pragma once
2+
3+
#include <cerrno>
4+
#include <charconv>
5+
#include <cstdlib>
6+
#include <optional>
7+
#include <string>
8+
#include <string_view>
9+
#include <system_error>
10+
#include <type_traits>
11+
12+
namespace PJ {
13+
14+
/// Parse the *entire* @p text as a number of type T, returning nullopt unless
15+
/// the whole string is a valid, in-range value (empty input, trailing
16+
/// characters, or overflow all yield nullopt).
17+
///
18+
/// Use this instead of std::from_chars when T may be floating-point: Apple
19+
/// Clang's libc++ does not implement the std::from_chars floating-point
20+
/// overloads, so those are routed through std::strto* here. Integral types go
21+
/// straight to std::from_chars on every toolchain.
22+
template <typename T>
23+
[[nodiscard]] std::optional<T> parseNumber(std::string_view text) {
24+
static_assert(
25+
std::is_arithmetic_v<T> && !std::is_same_v<T, bool>,
26+
"parseNumber supports integral and floating-point types, not bool");
27+
if (text.empty()) {
28+
return std::nullopt;
29+
}
30+
if constexpr (std::is_floating_point_v<T>) {
31+
// std::strto* needs a null-terminated buffer and @p text may be a
32+
// non-terminated view, so copy it. Number strings are short — the
33+
// allocation is negligible for the config/settings paths this serves.
34+
const std::string buffer(text);
35+
const char* begin = buffer.c_str();
36+
char* last = nullptr;
37+
errno = 0;
38+
T out{};
39+
if constexpr (std::is_same_v<T, float>) {
40+
out = std::strtof(begin, &last);
41+
} else if constexpr (std::is_same_v<T, long double>) {
42+
out = std::strtold(begin, &last);
43+
} else {
44+
out = std::strtod(begin, &last);
45+
}
46+
if (errno != 0 || last != begin + buffer.size()) {
47+
return std::nullopt;
48+
}
49+
return out;
50+
} else {
51+
T out{};
52+
const char* begin = text.data();
53+
const char* end = begin + text.size();
54+
const auto [ptr, ec] = std::from_chars(begin, end, out);
55+
if (ec != std::errc{} || ptr != end) {
56+
return std::nullopt;
57+
}
58+
return out;
59+
}
60+
}
61+
62+
} // namespace PJ

pj_base/include/pj_base/sdk/plugin_data_api.hpp

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Copyright 2026 Davide Faconti
33
// SPDX-License-Identifier: Apache-2.0
44

5-
#include <charconv>
65
#include <cstdint>
76
#include <cstring>
87
#include <functional>
@@ -18,6 +17,7 @@
1817

1918
#include "pj_base/builtin/builtin_object.hpp"
2019
#include "pj_base/expected.hpp"
20+
#include "pj_base/number_parse.hpp"
2121
#include "pj_base/plugin_data_api.h"
2222
#include "pj_base/sdk/arrow.hpp"
2323
#include "pj_base/sdk/object_bytes.hpp"
@@ -1309,11 +1309,11 @@ class SettingsValue {
13091309
}
13101310

13111311
[[nodiscard]] std::int64_t toInt(std::int64_t def = 0) const {
1312-
return parse<std::int64_t>(def);
1312+
return raw_.has_value() ? parseNumber<std::int64_t>(*raw_).value_or(def) : def;
13131313
}
13141314

13151315
[[nodiscard]] double toDouble(double def = 0.0) const {
1316-
return parse<double>(def);
1316+
return raw_.has_value() ? parseNumber<double>(*raw_).value_or(def) : def;
13171317
}
13181318

13191319
/// "true"/"1"/"on" → true; "false"/"0"/"off" → false; otherwise @p def.
@@ -1332,18 +1332,6 @@ class SettingsValue {
13321332
}
13331333

13341334
private:
1335-
template <typename T>
1336-
[[nodiscard]] T parse(T def) const {
1337-
if (!raw_.has_value()) {
1338-
return def;
1339-
}
1340-
T out{};
1341-
const char* begin = raw_->data();
1342-
const char* end = begin + raw_->size();
1343-
auto [ptr, ec] = std::from_chars(begin, end, out);
1344-
return (ec == std::errc{} && ptr == end) ? out : def;
1345-
}
1346-
13471335
std::optional<std::string> raw_;
13481336
};
13491337

0 commit comments

Comments
 (0)