attempt two

CMakeLists.txt

@@ -32,7 +32,7 @@ endif()
 # set the project name
 project(
 	libcifpp
-	VERSION 9.0.3
+	VERSION 9.0.4
 	LANGUAGES CXX C)
 
 list(PREPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
@@ -178,6 +178,18 @@ if(FMT EQUAL "FMT-NOTFOUND")
 	endif()
 endif()
 
+# Using fast_float for float parsing, but only if needed
+# try_compile(STD_CHARCONV_COMPILING
+# 	SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/cmake/test-charconv.cpp)
+
+if(NOT STD_CHARCONV_COMPILING)
+	message(NOTICE "libcifpp: Using fast_float for std::from_chars")
+	FetchContent_Declare(fastfloat
+		GIT_REPOSITORY "https://github.com/fastfloat/fast_float"
+		GIT_TAG v8.0.2)
+	FetchContent_MakeAvailable(fastfloat)
+endif()
+
 find_package(Threads)
 find_package(ZLIB QUIET)
 
@@ -343,6 +355,10 @@ else()
 	target_link_libraries(cifpp PRIVATE $<BUILD_INTERFACE:pcre2s>)
 endif()
 
+if(NOT STD_CHARCONV_COMPILING)
+	target_link_libraries(cifpp PUBLIC FastFloat::fast_float)
+endif()
+
 if(fmt_FOUND)
 	target_link_libraries(cifpp PUBLIC fmt)
 endif()

changelog

@@ -1,3 +1,6 @@
+Version 9.0.4
+- Fix various stopping and reconstruction errors
+
 Version 9.0.3
 - Reconstruction fixed when some entity ids are missing
 

cmake/test-charconv.cpp

@@ -0,0 +1,17 @@
+#include <charconv>
+#include <cassert>
+#include <cstring>
+
+int main()
+{
+	float v;
+	char s[] = "1.0";
+
+	auto r = std::from_chars(s, s + strlen(s), v);
+
+	assert(r.ec == std::errc{});
+	assert(r.ptr = s + strlen(s));
+	assert(v == 1.0f);
+
+	return 0;
+}

include/cif++/item.hpp

@@ -53,12 +53,12 @@ namespace cif
 // --------------------------------------------------------------------
 /** @brief item is a transient class that is used to pass data into rows
  * but it also takes care of formatting data.
- * 
- * 
- * 
+ *
+ *
+ *
  * The class cif::item is often used implicitly when creating a row in a category
  * using the emplace function.
- * 
+ *
  * @code{.cpp}
  * cif::category cat("my-cat");
  * cat.emplace({
@@ -68,12 +68,12 @@ namespace cif
  *   { "item-4", std::make_optional<int>(42) },   // <- stores an item with value 42
  *   { "item-5" }                                 // <- stores an item with value .
  * });
- * 
+ *
  * std::cout << cat << '\n';
  * @endcode
- * 
+ *
  * Will result in:
- * 
+ *
  * @code{.txt}
  * _my-cat.item-1 1
  * _my-cat.item-2 1.00
@@ -176,7 +176,7 @@ class item
 
 	/// \brief constructor for an item with name \a name and as
 	/// content value \a value
-	template<typename T, std::enable_if_t<std::is_same_v<T, std::string>, int> = 0>
+	template <typename T, std::enable_if_t<std::is_same_v<T, std::string>, int> = 0>
 	item(const std::string_view name, T &&value)
 		: m_name(name)
 		, m_value(std::move(value))
@@ -221,8 +221,8 @@ class item
 	item &operator=(item &&rhs) noexcept = default;
 	/** @endcond */
 
-	std::string_view name() const { return m_name; }   ///< Return the name of the item
-	std::string_view value() const & { return m_value; } ///< Return the value of the item
+	std::string_view name() const { return m_name; }            ///< Return the name of the item
+	std::string_view value() const & { return m_value; }        ///< Return the value of the item
 	std::string value() const && { return std::move(m_value); } ///< Return the value of the item
 
 	/// \brief replace the content of the stored value with \a v
@@ -560,7 +560,9 @@ struct item_handle::item_value_as<T, std::enable_if_t<std::is_arithmetic_v<T> an
 			auto b = txt.data();
 			auto e = txt.data() + txt.size();
 
-			std::from_chars_result r = (b + 1 < e and *b == '+' and std::isdigit(b[1])) ? selected_charconv<value_type>::from_chars(b + 1, e, result) : selected_charconv<value_type>::from_chars(b, e, result);
+			std::from_chars_result r = (b + 1 < e and *b == '+' and std::isdigit(b[1])) //
+			                               ? from_chars(b + 1, e, result)
+			                               : from_chars(b, e, result);
 
 			if ((bool)r.ec or r.ptr != e)
 			{
@@ -595,7 +597,9 @@ struct item_handle::item_value_as<T, std::enable_if_t<std::is_arithmetic_v<T> an
 			auto b = txt.data();
 			auto e = txt.data() + txt.size();
 
-			std::from_chars_result r = (b + 1 < e and *b == '+' and std::isdigit(b[1])) ? selected_charconv<value_type>::from_chars(b + 1, e, v) : selected_charconv<value_type>::from_chars(b, e, v);
+			std::from_chars_result r = (b + 1 < e and *b == '+' and std::isdigit(b[1]))
+			                               ? from_chars(b + 1, e, v)
+			                               : from_chars(b, e, v);
 
 			if ((bool)r.ec or r.ptr != e)
 			{

include/cif++/text.hpp

@@ -355,279 +355,35 @@ std::string cif_id_for_number(int number);
 std::vector<std::string> word_wrap(const std::string &text, std::size_t width);
 
 // --------------------------------------------------------------------
-/// \brief std::from_chars for floating point types.
-///
-/// These are optional, there's a selected_charconv class below that selects
-/// the best option to use based on support by the stl library.
-///
-/// I.e. that in case of GNU < 12 (or something) the cif implementation will
-/// be used, all other cases will use the stl version.
 
-template <typename FloatType, std::enable_if_t<std::is_floating_point_v<FloatType>, int> = 0>
-std::from_chars_result from_chars(const char *first, const char *last, FloatType &value)
-{
-	std::from_chars_result result{ first, {} };
-
-	enum State
-	{
-		IntegerSign,
-		Integer,
-		Fraction,
-		ExponentSign,
-		Exponent
-	} state = IntegerSign;
-	int sign = 1;
-	unsigned long long vi = 0;
-	int fl = 0, tz = 0;
-	int exponent_sign = 1;
-	int exponent = 0;
-	bool done = false;
-
-	while (not done and not (bool)result.ec)
-	{
-		char ch = result.ptr != last ? *result.ptr : 0;
-		++result.ptr;
-
-		switch (state)
-		{
-			case IntegerSign:
-				if (ch == '-')
-				{
-					sign = -1;
-					state = Integer;
-				}
-				else if (ch == '+')
-					state = Integer;
-				else if (ch >= '0' and ch <= '9')
-				{
-					vi = ch - '0';
-					state = Integer;
-				}
-				else if (ch == '.')
-					state = Fraction;
-				else
-					result.ec = std::errc::invalid_argument;
-				break;
-
-			case Integer:
-				if (ch >= '0' and ch <= '9')
-					vi = 10 * vi + (ch - '0');
-				else if (ch == 'e' or ch == 'E')
-					state = ExponentSign;
-				else if (ch == '.')
-					state = Fraction;
-				else
-				{
-					done = true;
-					--result.ptr;
-				}
-				break;
-
-			case Fraction:
-				if (ch >= '0' and ch <= '9')
-				{
-					vi = 10 * vi + (ch - '0');
-
-					if (ch == '0')
-						tz += 1;
-					else
-					{
-						fl += tz + 1;
-						tz = 0;
-					}
-				}
-				else if (ch == 'e' or ch == 'E')
-					state = ExponentSign;
-				else
-				{
-					done = true;
-					--result.ptr;
-				}
-				break;
-
-			case ExponentSign:
-				if (ch == '-')
-				{
-					exponent_sign = -1;
-					state = Exponent;
-				}
-				else if (ch == '+')
-					state = Exponent;
-				else if (ch >= '0' and ch <= '9')
-				{
-					exponent = ch - '0';
-					state = Exponent;
-				}
-				else
-					result.ec = std::errc::invalid_argument;
-				break;
-
-			case Exponent:
-				if (ch >= '0' and ch <= '9')
-					exponent = 10 * exponent + (ch - '0');
-				else
-				{
-					done = true;
-					--result.ptr;
-				}
-				break;
-		}
-	}
-
-	if (not (bool)result.ec)
-	{
-		while (tz-- > 0)
-			vi /= 10;
-
-		long double v = std::pow(10, -fl) * vi * sign;
-		if (exponent != 0)
-			v *= std::pow(10, exponent * exponent_sign);
-
-		if (std::isnan(v))
-			result.ec = std::errc::invalid_argument;
-		else if (std::abs(v) > std::numeric_limits<FloatType>::max())
-			result.ec = std::errc::result_out_of_range;
-
-		value = static_cast<FloatType>(v);
-	}
-
-	return result;
-}
-
-/// \brief duplication of std::chars_format for deficient STL implementations
-enum class chars_format
-{
-	scientific = 1,
-	fixed = 2,
-	// hex,
-	general = fixed | scientific
-};
-
-/// \brief a simplistic implementation of std::to_chars for old STL implementations
-template <typename FloatType, std::enable_if_t<std::is_floating_point_v<FloatType>, int> = 0>
-std::to_chars_result to_chars(char *first, char *last, FloatType &value, chars_format fmt)
-{
-	int size = static_cast<int>(last - first);
-	int r = 0;
-
-	switch (fmt)
-	{
-		case chars_format::scientific:
-			if constexpr (std::is_same_v<FloatType, long double>)
-				r = snprintf(first, last - first, "%le", value);
-			else
-				r = snprintf(first, last - first, "%e", value);
-			break;
-
-		case chars_format::fixed:
-			if constexpr (std::is_same_v<FloatType, long double>)
-				r = snprintf(first, last - first, "%lf", value);
-			else
-				r = snprintf(first, last - first, "%f", value);
-			break;
-
-		case chars_format::general:
-			if constexpr (std::is_same_v<FloatType, long double>)
-				r = snprintf(first, last - first, "%lg", value);
-			else
-				r = snprintf(first, last - first, "%g", value);
-			break;
-	}
-
-	std::to_chars_result result;
-	if (r < 0 or r >= size)
-		result = { first, std::errc::value_too_large };
-	else
-		result = { first + r, std::errc() };
-
-	return result;
-}
-
-/// \brief a simplistic implementation of std::to_chars for old STL implementations
-template <typename FloatType, std::enable_if_t<std::is_floating_point_v<FloatType>, int> = 0>
-std::to_chars_result to_chars(char *first, char *last, FloatType &value, chars_format fmt, int precision)
-{
-	int size = static_cast<int>(last - first);
-	int r = 0;
-
-	switch (fmt)
-	{
-		case chars_format::scientific:
-			if constexpr (std::is_same_v<FloatType, long double>)
-				r = snprintf(first, last - first, "%.*le", precision, value);
-			else
-				r = snprintf(first, last - first, "%.*e", precision, value);
-			break;
-
-		case chars_format::fixed:
-			if constexpr (std::is_same_v<FloatType, long double>)
-				r = snprintf(first, last - first, "%.*lf", precision, value);
-			else
-				r = snprintf(first, last - first, "%.*f", precision, value);
-			break;
-
-		case chars_format::general:
-			if constexpr (std::is_same_v<FloatType, long double>)
-				r = snprintf(first, last - first, "%.*lg", precision, value);
-			else
-				r = snprintf(first, last - first, "%.*g", precision, value);
-			break;
-	}
-
-	std::to_chars_result result;
-	if (r < 0 or r >= size)
-		result = { first, std::errc::value_too_large };
-	else
-		result = { first + r, std::errc() };
-
-	return result;
-}
-
-/// \brief class that uses our implementation of std::from_chars and std::to_chars
 template <typename T>
-struct my_charconv
-{
-	/// @brief Simply call our version of std::from_chars
-	static std::from_chars_result from_chars(const char *a, const char *b, T &d)
-	{
-		return cif::from_chars(a, b, d);
-	}
+using from_chars_function = decltype(std::from_chars(std::declval<const char *>(), std::declval<const char *>(), std::declval<T &>()));
 
-	/// @brief Simply call our version of std::to_chars
-	static std::to_chars_result to_chars(char *first, char *last, T &value, chars_format fmt)
-	{
-		return cif::to_chars(first, last, value, fmt);
-	}
-};
-
-/// \brief class that uses the STL implementation of std::from_chars and std::to_chars
 template <typename T>
 struct std_charconv
 {
-	/// @brief Simply call std::from_chars
 	static std::from_chars_result from_chars(const char *a, const char *b, T &d)
 	{
 		return std::from_chars(a, b, d);
 	}
-
-	/// @brief Simply call std::to_chars
-	static std::to_chars_result to_chars(char *first, char *last, T &value, chars_format fmt)
-	{
-		return std::to_chars(first, last, value, fmt);
-	}
 };
 
-/// \brief helper to find a from_chars function
-template <typename T>
-using from_chars_function = decltype(std::from_chars(std::declval<const char *>(), std::declval<const char *>(), std::declval<T &>()));
+template <typename T, typename = void>
+struct ff_charconv;
 
-/**
- * @brief Helper to select the best implementation of charconv based on availability of the
- * function in the std:: namespace
- *
- * @tparam T The type for which we want to find a from_chars/to_chars function
- */
 template <typename T>
-using selected_charconv = typename std::conditional_t<std_experimental::is_detected_v<from_chars_function, T>, std_charconv<T>, my_charconv<T>>;
+struct ff_charconv<T, typename std::enable_if_t<std::is_floating_point_v<T>>>
+{
+	static std::from_chars_result from_chars(const char *a, const char *b, T &v);
+};
+
+template <typename T>
+using charconv = typename std::conditional_t<std_experimental::is_detected_v<from_chars_function, T>, std_charconv<T>, ff_charconv<T>>;
+
+template <typename T>
+constexpr auto from_chars(const char *s, const char *e, T &v)
+{
+	return charconv<T>::from_chars(s, e, v);
+}
 
 } // namespace cif

src/datablock.cpp

@@ -25,8 +25,11 @@
  */
 
 #include "cif++/datablock.hpp"
+
 #include "cif++/validate.hpp"
 
+#include <exception>
+
 namespace cif
 {
 
@@ -42,7 +45,16 @@ datablock::datablock(const datablock &db)
 void datablock::load_dictionary()
 {
 	if (auto *audit_conform = get("audit_conform"); audit_conform and not audit_conform->empty())
-		set_validator(&validator_factory::instance().get(*audit_conform));
+	{
+		try
+		{
+			set_validator(&validator_factory::instance().get(*audit_conform));
+		}
+		catch (const std::exception &ex)
+		{
+			std::clog << ex.what() << '\n';
+		}
+	}
 }
 
 void datablock::set_validator(const validator *v)
@@ -96,7 +108,8 @@ bool datablock::strip()
 	bool result = true;
 
 	// remove all categories that have no validator
-	erase(std::remove_if(begin(), end(), [](category &c) {
+	erase(std::remove_if(begin(), end(), [](category &c)
+			  {
 		bool result = false;
 		if (c.get_cat_validator() == nullptr)
 		{
@@ -104,8 +117,8 @@ bool datablock::strip()
 				std::clog << "Dropping category " << c.name() << '\n';
 			result = true;
 		}
-		return result;
-	}), end());
+		return result; }),
+		end());
 
 	// then strip the remaining categories
 	for (auto &cat : *this)

src/model.cpp

@@ -2898,8 +2898,8 @@ static int compare_numbers(std::string_view a, std::string_view b)
 
 	std::from_chars_result ra, rb;
 
-	ra = selected_charconv<double>::from_chars(a.data(), a.data() + a.length(), da);
-	rb = selected_charconv<double>::from_chars(b.data(), b.data() + b.length(), db);
+	ra = from_chars(a.data(), a.data() + a.length(), da);
+	rb = from_chars(b.data(), b.data() + b.length(), db);
 
 	if (not(bool) ra.ec and not(bool) rb.ec)
 	{

src/pdb/reconstruct.cpp

@@ -295,11 +295,13 @@ void createEntityIDs(datablock &db)
 					{ "id", entity_id },
 					{ "type", "water" } });
 			else
+			{
 				entity.emplace({ //
 					{ "id", entity_id },
 					{ "type", "non-polymer" } });
 
-			newEntitiesForCompound[comp_id] = entity_id;
+				newEntitiesForCompound[comp_id] = entity_id;
+			}
 		}
 
 		entity_ids[i] = entity_id;
@@ -648,7 +650,7 @@ void checkAtomRecords(datablock &db)
 			{
 				char b[12];
 
-				if (auto [ptr, ec] = cif::to_chars(b, b + sizeof(b), v, cif::chars_format::fixed, prec); ec == std::errc{})
+				if (auto [ptr, ec] = std::to_chars(b, b + sizeof(b), v, std::chars_format::fixed, prec); ec == std::errc{})
 					row.assign(item_name, { b, static_cast<std::string::size_type>(ptr - b) }, false, false);
 			}
 		}

src/text.cpp

@@ -29,6 +29,11 @@
 #include <algorithm>
 #include <cassert>
 
+#if __has_include("fast_float/fast_float.h")
+#include "fast_float/fast_float.h"
+#endif
+
+
 namespace cif
 {
 
@@ -512,4 +517,22 @@ std::vector<std::string> word_wrap(const std::string &text, std::size_t width)
 	return result;
 }
 
+#if __has_include("fast_float/fast_float.h")
+
+template<>
+std::from_chars_result ff_charconv<float>::from_chars(const char *a, const char *b, float &v)
+{
+	auto r = fast_float::from_chars(a, b, v);
+	return { r.ptr, r.ec };
+}
+
+template<>
+std::from_chars_result ff_charconv<double>::from_chars(const char *a, const char *b, double &v)
+{
+	auto r = fast_float::from_chars(a, b, v);
+	return { r.ptr, r.ec };
+}
+
+#endif
+
 } // namespace cif

src/validate.cpp

@@ -181,8 +181,8 @@ int type_validator::compare(std::string_view a, std::string_view b) const
 
 				std::from_chars_result ra, rb;
 
-				ra = selected_charconv<double>::from_chars(a.data(), a.data() + a.length(), da);
-				rb = selected_charconv<double>::from_chars(b.data(), b.data() + b.length(), db);
+				ra = from_chars(a.data(), a.data() + a.length(), da);
+				rb = from_chars(b.data(), b.data() + b.length(), db);
 
 				if (not(bool) ra.ec and not(bool) rb.ec)
 				{
@@ -561,8 +561,10 @@ const validator &validator_factory::get(const category &audit_conform)
 	// If the audit conform contains only one record, this is easy
 	if (audit_conform.size() == 1)
 	{
-		const auto &[name, version] = audit_conform.front().get<std::string, std::optional<std::string>>("dict_name", "dict_version");
-		return m_validators.emplace_back(construct_validator(name, version));
+		const auto &[name, version] =
+			audit_conform.front().get<std::string, std::optional<std::string>>("dict_name", "dict_version");
+		if (not name.empty())
+			return m_validators.emplace_back(construct_validator(name, version));
 	}
 
 	// A new, merged dictionary
@@ -570,6 +572,9 @@ const validator &validator_factory::get(const category &audit_conform)
 	std::optional<validator> v;
 	for (const auto &[name, version] : audit_conform.rows<std::string, std::optional<std::string>>("dict_name", "dict_version"))
 	{
+		if (name.empty())
+			continue;
+
 		if (not v)
 			v = construct_validator(name, version);
 		else

test/CMakeLists.txt

@@ -26,12 +26,10 @@ if(NOT (Catch2_FOUND OR TARGET Catch2))
 	find_package(Catch2 3 QUIET)
 
 	if(NOT Catch2_FOUND)
-		include(FetchContent)
-
 		FetchContent_Declare(
 			Catch2
 			GIT_REPOSITORY https://github.com/catchorg/Catch2.git
-            GIT_TAG v3.8.0)
+            GIT_TAG v3.4.0)
 
 		FetchContent_MakeAvailable(Catch2)
 

test/unit-v2-test.cpp

@@ -155,8 +155,8 @@ TEST_CASE("cc_2")
 
 	for (const auto &[val, prec, test] : tests)
 	{
-		char buffer[64];
-		const auto &[ptr, ec] = cif::to_chars(buffer, buffer + sizeof(buffer), val, cif::chars_format::fixed, prec);
+		char buffer[64] = {};
+		const auto &[ptr, ec] = std::to_chars(buffer, buffer + sizeof(buffer), val, std::chars_format::fixed, prec);
 
 		CHECK_FALSE((bool)ec);