Address review: simplify from_chars migration to net reduction in code

- buildInt: parse once as unsigned long long (input is never negative),
  strip suffixes with remove_suffix, eliminate two-pass fallback path
- json.hpp parse_number: scan number boundaries in-place, call from_chars
  directly on the string range — no intermediate strings or std::pow
- parse_num: remove dead integral overload, collapse to single template
- Remove unused <cmath> includes from both files

Requested by @lefticus in PR #669 review.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
leftibot 2026-04-12 16:29:31 -06:00
parent 6517bb38a7
commit da7a1e043b
3 changed files with 59 additions and 115 deletions

View File

@ -70,7 +70,6 @@ static_assert(_MSC_FULL_VER >= 190024210, "Visual C++ 2015 Update 3 or later req
#endif
#include <charconv>
#include <cmath>
#include <memory>
#include <string>
@ -124,21 +123,8 @@ namespace chaiscript {
};
template<typename T>
[[nodiscard]] constexpr auto parse_num(const std::string_view t_str) noexcept -> typename std::enable_if<std::is_integral<T>::value, T>::type {
T t = 0;
for (const auto c : t_str) {
if (c < '0' || c > '9') {
return t;
}
t *= 10;
t += c - '0';
}
return t;
}
template<typename T>
[[nodiscard]] auto parse_num(const std::string_view t_str) -> typename std::enable_if<!std::is_integral<T>::value, T>::type {
T t = 0;
[[nodiscard]] auto parse_num(const std::string_view t_str) {
T t{};
std::from_chars(t_str.data(), t_str.data() + t_str.size(), t);
return t;
}

View File

@ -736,30 +736,29 @@ namespace chaiscript {
bool long_ = false;
bool longlong_ = false;
auto i = t_val.size();
for (; i > 0; --i) {
const char val = t_val[i - 1];
if (val == 'u' || val == 'U') {
while (!t_val.empty()) {
const char c = t_val.back();
if (c == 'u' || c == 'U') {
unsigned_ = true;
} else if (val == 'l' || val == 'L') {
if (long_) {
longlong_ = true;
}
} else if (c == 'l' || c == 'L') {
if (long_) { longlong_ = true; }
long_ = true;
} else {
break;
}
t_val.remove_suffix(1);
}
if (prefixed) {
t_val.remove_prefix(2);
}
const auto *const first = t_val.data();
const auto *const last = first + i - (prefixed ? 2 : 0);
unsigned long long uu = 0;
const auto [ptr, ec] = std::from_chars(t_val.data(), t_val.data() + t_val.size(), uu, base);
if (ec != std::errc()) {
return const_var(std::numeric_limits<long long>::max());
}
#ifdef __GNUC__
#pragma GCC diagnostic push
@ -767,47 +766,24 @@ namespace chaiscript {
#ifdef CHAISCRIPT_CLANG
#pragma GCC diagnostic ignored "-Wtautological-compare"
#pragma GCC diagnostic ignored "-Wtautological-unsigned-zero-compare"
#pragma GCC diagnostic ignored "-Wtautological-type-limit-compare"
#pragma GCC diagnostic ignored "-Wsign-conversion"
#endif
#endif
{
long long u = 0;
auto [ptr, ec] = std::from_chars(first, last, u, base);
if (ec == std::errc()) {
if (!unsigned_ && !long_ && u >= std::numeric_limits<int>::min() && u <= std::numeric_limits<int>::max()) {
return const_var(static_cast<int>(u));
} else if ((unsigned_ || base != 10) && !long_ && u >= std::numeric_limits<unsigned int>::min()
&& u <= std::numeric_limits<unsigned int>::max()) {
return const_var(static_cast<unsigned int>(u));
} else if (!unsigned_ && !longlong_ && u >= std::numeric_limits<long>::min() && u <= std::numeric_limits<long>::max()) {
return const_var(static_cast<long>(u));
} else if ((unsigned_ || base != 10) && !longlong_ && u >= std::numeric_limits<unsigned long>::min()
&& u <= std::numeric_limits<unsigned long>::max()) {
return const_var(static_cast<unsigned long>(u));
} else if (!unsigned_ && u >= std::numeric_limits<long long>::min() && u <= std::numeric_limits<long long>::max()) {
return const_var(static_cast<long long>(u));
} else {
return const_var(static_cast<unsigned long long>(u));
}
}
unsigned long long uu = 0;
const auto result = std::from_chars(first, last, uu, base);
if (result.ec == std::errc()) {
if (!longlong_ && uu >= std::numeric_limits<unsigned long>::min() && uu <= std::numeric_limits<unsigned long>::max()) {
return const_var(static_cast<unsigned long>(uu));
} else {
return const_var(static_cast<unsigned long long>(uu));
}
}
return const_var(std::numeric_limits<long long>::max());
if (!unsigned_ && !long_ && uu <= static_cast<unsigned long long>(std::numeric_limits<int>::max())) {
return const_var(static_cast<int>(uu));
} else if ((unsigned_ || base != 10) && !long_ && uu <= std::numeric_limits<unsigned int>::max()) {
return const_var(static_cast<unsigned int>(uu));
} else if (!unsigned_ && !longlong_ && uu <= static_cast<unsigned long long>(std::numeric_limits<long>::max())) {
return const_var(static_cast<long>(uu));
} else if ((unsigned_ || base != 10) && !longlong_ && uu <= std::numeric_limits<unsigned long>::max()) {
return const_var(static_cast<unsigned long>(uu));
} else if (!unsigned_ && uu <= static_cast<unsigned long long>(std::numeric_limits<long long>::max())) {
return const_var(static_cast<long long>(uu));
} else {
return const_var(static_cast<unsigned long long>(uu));
}
#ifdef __GNUC__

View File

@ -8,7 +8,7 @@
#include "../chaiscript_defines.hpp"
#include "quick_flat_map.hpp"
#include <cctype>
#include <cmath>
#include <charconv>
#include <cstdint>
#include <initializer_list>
#include <iostream>
@ -503,65 +503,47 @@ namespace chaiscript::json {
}
static JSON parse_number(const std::string &str, size_t &offset) {
std::string val, exp_str;
char c = '\0';
const auto start = offset;
bool isDouble = false;
bool isNegative = false;
std::int64_t exp = 0;
bool isExpNegative = false;
if (offset < str.size() && str.at(offset) == '-') {
isNegative = true;
if (offset < str.size() && str[offset] == '-') { ++offset; }
while (offset < str.size() && str[offset] >= '0' && str[offset] <= '9') { ++offset; }
if (offset < str.size() && str[offset] == '.') {
isDouble = true;
++offset;
while (offset < str.size() && str[offset] >= '0' && str[offset] <= '9') { ++offset; }
}
for (; offset < str.size();) {
c = str.at(offset++);
if (c >= '0' && c <= '9') {
val += c;
} else if (c == '.' && !isDouble) {
val += c;
isDouble = true;
} else {
break;
}
}
if (offset < str.size() && (c == 'E' || c == 'e')) {
c = str.at(offset++);
if (c == '-') {
isExpNegative = true;
} else if (c == '+') {
// do nothing
} else {
--offset;
}
for (; offset < str.size();) {
c = str.at(offset++);
if (c >= '0' && c <= '9') {
exp_str += c;
} else if (!isspace(c) && c != ',' && c != ']' && c != '}') {
throw std::runtime_error(std::string("JSON ERROR: Number: Expected a number for exponent, found '") + c + "'");
} else {
break;
}
if (offset < str.size() && (str[offset] == 'e' || str[offset] == 'E')) {
isDouble = true;
++offset;
if (offset < str.size() && (str[offset] == '+' || str[offset] == '-')) { ++offset; }
if (offset >= str.size() || str[offset] < '0' || str[offset] > '9') {
throw std::runtime_error(std::string("JSON ERROR: Number: Expected a number for exponent, found '") + str[offset] + "'");
}
exp = chaiscript::parse_num<std::int64_t>(exp_str) * (isExpNegative ? -1 : 1);
} else if (offset < str.size() && (!isspace(c) && c != ',' && c != ']' && c != '}')) {
throw std::runtime_error(std::string("JSON ERROR: Number: unexpected character '") + c + "'");
while (offset < str.size() && str[offset] >= '0' && str[offset] <= '9') { ++offset; }
}
--offset;
if (isDouble || !exp_str.empty()) {
std::string full_num;
if (isNegative) { full_num += '-'; }
full_num += val;
if (!exp_str.empty()) {
full_num += 'e';
if (isExpNegative) { full_num += '-'; }
full_num += exp_str;
if (offset < str.size()) {
const char c = str[offset];
if (!isspace(c) && c != ',' && c != ']' && c != '}') {
throw std::runtime_error(std::string("JSON ERROR: Number: unexpected character '") + c + "'");
}
return JSON(chaiscript::parse_num<double>(full_num));
}
const auto *const first = str.data() + start;
const auto *const last = str.data() + offset;
if (isDouble) {
double val = 0;
std::from_chars(first, last, val);
return JSON(val);
} else {
return JSON((isNegative ? -1 : 1) * chaiscript::parse_num<std::int64_t>(val));
std::int64_t val = 0;
std::from_chars(first, last, val);
return JSON(val);
}
}