Skip to content

Commit

Permalink
Remove double-conversion (#116)
Browse files Browse the repository at this point in the history
* adding new `to_string` implementation and cleaning up test cases

* remove double-conversion as an ASL dependency

* adding const qualifier

* Yet another build break

* replacing dconv-based parsing with `std::from_chars` in `json.hpp`

* falling back to boost lexical_cast

* Yet another build break

* adding test case for `-Infinity`

* Yet another build break

* using `std::stod` instead of `boost::lexical_cast<double>`

* Switching _back_ to `lexical_cast<double>` as we can parse without allocations

* adding json floating point parsing tests, and _now_ using `std::strtod`

* docs tweak

* Yet another build break

* Yet another build break
  • Loading branch information
fosterbrereton authored May 13, 2024
1 parent 5a98d06 commit 2e788b8
Show file tree
Hide file tree
Showing 12 changed files with 158 additions and 865 deletions.
20 changes: 2 additions & 18 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,8 @@ project(adobe-source-libraries CXX)
# add_definitions ("-Wall")
# add_definitions ("-Werror")

# There are two big choices this file makes - how to include Boost, and how to
# include double-conversion, respectively. Build environments vary and we're
# trying to support all of them.
#
# For double-conversion, the unix makefile setup is able to download the git
# repo and build it

option(USE_STLAB_DOUBLECONV "use stlab mirror of double-conversion repository" OFF)
# There is a big choice this file makes, namely how to include Boost. Build environments
# vary and we're trying to support all of them.

set(root_path ${CMAKE_SOURCE_DIR}/..)

Expand Down Expand Up @@ -68,16 +62,6 @@ function(setup_dep)

endfunction()

if (USE_STLAB_DOUBLECONV)
setup_dep(URL https://github.com/stlab/double-conversion.git BRANCH master IS_CMAKE)
target_include_directories(double-conversion PUBLIC $<BUILD_INTERFACE:${root_path}>)
else()
setup_dep(URL https://github.com/google/double-conversion.git BRANCH master IS_CMAKE)
target_include_directories(double-conversion PUBLIC $<BUILD_INTERFACE:${root_path}/double-conversion>)
# poorly named flag to fix include path
target_compile_definitions(double-conversion PUBLIC ADOBE_BUILT_WITH_CMAKE)
endif()

function(target_link_boost target)
target_link_libraries(${target} PUBLIC Boost::system)
endfunction(target_link_boost)
Expand Down
19 changes: 6 additions & 13 deletions adobe/json.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@
#include <unordered_map>
#include <vector>

#ifdef ADOBE_BUILT_WITH_CMAKE
#include <double-conversion/double-conversion.h>
#else
#include <double-conversion/src/double-conversion.h>
#endif

#include <adobe/cassert.hpp>
#include <adobe/string/to_string.hpp>

Expand Down Expand Up @@ -69,8 +63,7 @@ class json_parser {
allow 'junk' as it is the end of this token and on to the next one.
*/
explicit json_parser(const char* p)
: p_(p), s2d_(double_conversion::StringToDoubleConverter::ALLOW_TRAILING_JUNK, kNaN, kNaN,
nullptr, nullptr) {}
: p_(p) {}

value_type parse() {
value_type result;
Expand Down Expand Up @@ -207,13 +200,14 @@ class json_parser {
frac();
exp();

int count = 0;
double value = s2d_.StringToDouble(p, static_cast<int>(p_ - p), &count);

const auto p_len = p_ - p;
char* p_end = nullptr;
double value = std::strtod(p, &p_end);
require(std::isfinite(value), "finite number");
ADOBE_ASSERT(count == p_ - p && "StringToDouble() failure");
ADOBE_ASSERT(p_len == (p_end - p) && "std::strtod() failure");

t = value_type(value);

return true;
}

Expand Down Expand Up @@ -350,7 +344,6 @@ class json_parser {
}

const char* p_;
double_conversion::StringToDoubleConverter s2d_;

typedef char table_t_[256];

Expand Down
16 changes: 2 additions & 14 deletions adobe/serializable.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,8 @@
#include <ostream>
#include <type_traits>

#ifdef ADOBE_BUILT_WITH_CMAKE
#include <double-conversion/double-conversion.h>
#else
#include <double-conversion/src/double-conversion.h>
#endif

#include <adobe/typeinfo.hpp>
#include <adobe/string/to_string.hpp>

/**************************************************************************************************/

Expand Down Expand Up @@ -75,14 +70,7 @@ inline void ostream_insertion<bool>(std::ostream& s, const bool& x) {

template <>
inline void ostream_insertion<double>(std::ostream& s, const double& x) {
using namespace double_conversion;

const DoubleToStringConverter& c(DoubleToStringConverter::EcmaScriptConverter());
char buf[32] = {0};
StringBuilder builder(buf, sizeof(buf));
c.ToShortest(x, &builder);

s << builder.Finalize();
s << adobe::to_string(x);
}

/**************************************************************************************************/
Expand Down
53 changes: 53 additions & 0 deletions adobe/string/to_string.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@
#define ADOBE_STRING_TO_STRING_HPP

#include <algorithm>
#include <array>
#include <cassert>
#include <charconv>
#include <cmath>
#include <cstdio>
#include <string>
#include <system_error>

#ifndef NDEBUG
#include <cfloat>
Expand Down Expand Up @@ -118,6 +123,54 @@ O to_string(double x, O out, bool precise = false) {
return std::copy(&buf[0], &buf[0] + size, out);
}

/**************************************************************************************************/
/*!
\ingroup string_algorithm
\brief Convert double precision floating point numbers to ascii representation.
\return A `std::string` with the representation, similar to the `std::to_string` routines.
Google's `double-conversion` library was originally used by ASL to serialize floating-point
values (mostly indirectly when serializing `any_regular_t`s that held a double.) In an effort
to reduce our dependencies, we have removed ASL's use of `double-conversion` in favor of this
variant of `adobe::to_string`, which itself is a light wrapping around `std::to_chars`.
`double-conversion`'s serialization class (`DoubleToStringConverter`) is highly configurable.
There is a comment detailing the all the knobs one can twist and their effects here:
https://github.com/google/double-conversion/blob/4f7a25d8ced8c7cf6eee6fd09d6788eaa23c9afe/double-conversion/double-to-string.h#L86-L164
`double-conversion` also comes with a handful of predefined settings for these knobs to make it
easier for library consumers to serialize floating-point values consistently. ASL used one of
these, called `EcmaScriptConverter`, and would then invoke its `ToShortest` API. The knob
values for the `EcmaScriptConverter` are defined here:
https://github.com/google/double-conversion/blob/4f7a25d8ced8c7cf6eee6fd09d6788eaa23c9afe/double-conversion/double-to-string.cc#L42-L51
`EcmaScriptConverter` sets the `DoubleToStringConverter` knobs a way that is not exactly
reproducible by `std::to_chars`. `adobe::to_string` gets us as close as possible to
`EcmaScriptConverter` without inundating the implementation with special cases. (There are
special cases for nan and +/-infinity so this variant's output matches `EcmaScriptConverter`'s
for those values.) The ways in which `EcmaScriptConverter` and `adobe::to_string` still differ
are considered to be acceptable tradeoffs in light of the eliminated dependency. Some of those
differences can be seen in ASL's `to_string` tests.
*/
inline std::string to_string(double x) {
if (std::isnan(x)) return "NaN";
if (x == std::numeric_limits<double>::infinity()) return "Infinity";
if (x == -std::numeric_limits<double>::infinity()) return "-Infinity";

std::array<char, 64> str;
char* first = &str[0];
char* last = first + str.size();
const std::to_chars_result tcr = std::to_chars(first, last, x);

return tcr.ec == std::errc() ?
std::string(first, tcr.ptr - first) :
std::make_error_code(tcr.ec).message();
}

/**************************************************************************************************/

} // namespace adobe
Expand Down
12 changes: 0 additions & 12 deletions configure.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,6 @@ BOOST_TAR=$BOOST_DIR.tar.gz

cd ..

#
# fetch the double-conversion library.
#

if [ ! -e 'double-conversion' ]; then
echo "INFO : double-conversion not found: setting up."

echo_run git clone --depth=50 --branch=master git://github.com/stlab/double-conversion.git
else
echo "INFO : double-conversion found: skipping setup."
fi

#
# If need be, download Boost and unzip it, moving it to the appropriate location.
#
Expand Down
18 changes: 0 additions & 18 deletions jamroot.jam
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,6 @@ else

use-project /boost : $(boost-path) ;

#
# Permits overriding the double-conversion directory by specifying DBL_CONV_PATH
# in the OS environment table.
#

local double-conversion-path = [ os.environ DBL_CONV_PATH ] ;

if $(double-conversion-path)
{
double-conversion-path = $(double-conversion-path) ;
}
else
{
double-conversion-path = ../ ;
}

# Set up c++11 support as a feature so it will propagate into the
# boost dependencies

Expand Down Expand Up @@ -95,7 +79,6 @@ asl_requirements =

# Include directories
<include>$(boost-path)
<include>$(double-conversion-path)
<include>.

# Linker specializations
Expand Down Expand Up @@ -156,7 +139,6 @@ project
lib asl
: #sources
[ glob source/*.cpp ]
[ glob $(double-conversion-path)/double-conversion/src/*.cc ]
/boost/filesystem
/boost/system
/boost/thread//boost_thread
Expand Down
6 changes: 3 additions & 3 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ We use [Travis CI](https://travis-ci.org/stlab/adobe_source_libraries) to valida
There are multiple ways of building the Adobe Source Libraries.

## `configure && build`
From a UNIX terminal you should be able to execute `./configure.sh` and `./build.sh`, which should download all the necessary dependencies (Boost and double-conversion) and begin building.
From a UNIX terminal you should be able to execute `./configure.sh` and `./build.sh`, which should download all the necessary dependencies (Boost) and begin building.

## CMake

CMake support also exists for Unix Makefiles and XCode. CMake will generate separate debug and release projects in a `build_asl` folder (which will be a sibling to the top-level `adobe_source_libraries` folder.)

CMake will download Boost and double-conversion from git repositories if not found, respectively in ../boost_libraries and ../double-conversion.
If not found, CMake will download Boost from its git repository into ../boost_libraries.

(The `./configure` script can also be used to download and place the Boost and double-conversion libraries where they need to go.)
(The `./configure` script can also be used to download and place the Boost library where it needs to go.)

### Makefiles

Expand Down
2 changes: 0 additions & 2 deletions source/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ if (${CMAKE_CXX_COMPILER_ID} STREQUAL "Clang" OR
target_compile_options(asl PUBLIC -ftemplate-depth=1024)
endif()

target_link_libraries(asl PUBLIC double-conversion)

target_link_boost(asl)

# thread linking
Expand Down
6 changes: 0 additions & 6 deletions source/any_regular.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@
#include <cassert>
#include <string>

#ifdef ADOBE_BUILT_WITH_CMAKE
#include <double-conversion/double-conversion.h>
#else
#include <double-conversion/src/double-conversion.h>
#endif

#include <adobe/algorithm/lower_bound.hpp>
#include <adobe/algorithm/sorted.hpp>
#include <adobe/array.hpp>
Expand Down
23 changes: 23 additions & 0 deletions test/json/asl_json_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,29 @@

/**************************************************************************************************/

BOOST_AUTO_TEST_CASE(asl_json_floating_point_parsing) {
struct test_t {
const char* str;
double expected;
};

test_t tests[] = {
{"[42]", 42},
{"[12.536]", 12.536},
{"[-20.5]", -20.5},
{"[-1.375e+112]", -1.375e+112},
{"[3.1415926535897932384626433832795028841971693993751058209]", 3.1415926535897932384626433832795028841971693993751058209},
};

for (const auto& test : tests) {
adobe::any_regular_t x = adobe::json_parse(test.str);
double d = x.cast<adobe::array_t>()[0].cast<double>();
BOOST_CHECK_CLOSE(d, test.expected, 0.000001);
}
}

/**************************************************************************************************/

BOOST_AUTO_TEST_CASE(asl_json_helper_smoke) {
std::cout << "-=-=- asl_json_helper_smoke -=-=-\n";
adobe::any_regular_t x = adobe::json_parse(u8R"raw(
Expand Down
Loading

0 comments on commit 2e788b8

Please sign in to comment.