From e3ce06a7611bc6f3f9439440dd305aca68797106 Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:16:14 +0700 Subject: [PATCH 1/6] Write an alias that is a reserved word in accent quotes The engine reserves ON, so MATCH (f:Flag) RETURN f.on AS on stopped parsing and two frame tests went red against the engine at HEAD. The property is still named on and reads fine, because a property is looked up by name rather than parsed as one. It is the alias that is a name being written, and the way to write one that is also a keyword is in accent quotes, which is what the engine's own message says and what its corpus already covers. --- test/test_frame.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/test_frame.cpp b/test/test_frame.cpp index 3607598..f05735a 100644 --- a/test/test_frame.cpp +++ b/test/test_frame.cpp @@ -156,7 +156,10 @@ ZU_TEST(a_bitmap_is_a_column_of_booleans) { frame.bools("on", bitmap, 4); conn.register_frame(frame); - auto r = conn.query("MATCH (f:Flag) RETURN f.on AS on"); + /* The alias in accent quotes because ON is a reserved word. The + property is still named on, and a property is read by name rather + than parsed as one, which is why only the alias needs them. */ + auto r = conn.query("MATCH (f:Flag) RETURN f.on AS `on`"); CHECK_EQ(r.rows(), 4u); CHECK_EQ(r.row(0).get(0), true); CHECK_EQ(r.row(1).get(0), false); @@ -215,7 +218,9 @@ ZU_TEST(a_date_goes_in_as_the_days_it_is_and_comes_back_as_a_date) { frame.column("on", on, 1, zu::TemporalKind::date); conn.register_frame(frame); - auto r = conn.query("MATCH (e:Event) RETURN e.on AS on"); + /* Accent quotes for the same reason they are up in the bitmap test: + ON is a reserved word and an alias is a name being written. */ + auto r = conn.query("MATCH (e:Event) RETURN e.on AS `on`"); const zu::Temporal first = r.row(0).get(0); CHECK_EQ(first.kind, zu::TemporalKind::date); CHECK_EQ(first.count, 19782); From 8a37866af31fdc31f517dbcec5965d737411efa9 Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:25:25 +0700 Subject: [PATCH 2/6] Ask whether the library has std::expected, not whether the standard is C++23 The floor build's #error fired on clang, which does not have std::expected at -std=c++23 and never claimed to. Clang reports __cpp_concepts as 201907L, libstdc++ gates every C++23 library feature on 202002L, so the library never switches on. The gate was asking about the standard when the bug it guards is about the library, so it now reads __cpp_lib_expected with included on the spot, which is the only thing that can tell a toolchain without std::expected from a header that mislaid it. --- test/test_expected.cpp | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/test/test_expected.cpp b/test/test_expected.cpp index 131826d..7d86e6e 100644 --- a/test/test_expected.cpp +++ b/test/test_expected.cpp @@ -6,8 +6,11 @@ * error, what succeeds returns a value, and the error carries the same * GQLSTATUS either way. * - * The whole file compiles to nothing under C++20, because std::expected - * arrived in 23 and the throwing half is complete on its own. */ + * The whole file compiles to nothing on a toolchain with no + * std::expected, which is every C++20 build and also clang against + * libstdc++, where the two disagree about concepts and the C++23 + * library never switches on. The throwing half is complete on its own, + * so that is a narrower API rather than a broken one. */ #include #include "fixture.hpp" @@ -184,16 +187,24 @@ ZU_TEST(a_frame_registers_without_throwing) { * half of the API a caller who builds without exceptions depends on * had not been compiled anywhere for months. * - * So the floor build says so out loud. Below C++23 there is nothing to - * run here and the throwing half is complete on its own; at C++23 and - * above, this half not being there is a broken build rather than a - * quiet one, and the compiler is the only thing positioned to notice. + * So the floor build says so out loud, and what it asks is the + * question the bug was: does this library have std::expected, and did + * zu.hpp fail to see it. Not whether the standard is C++23, which is a + * different question with a different answer. Clang reports + * __cpp_concepts as 201907L rather than 202002L, and libstdc++ gates + * every C++23 library feature on the later number, so clang against + * libstdc++ at -std=c++23 has no std::expected at all. That is a + * toolchain without it and not a header that mislaid it, and asking + * about the standard alone called it the second. * - * MSVC without /Zc:__cplusplus reports 199711L, which skips the check - * rather than firing it, and a gate that is off is better than a gate - * that is wrong. */ -#if __cplusplus > 202002L -#error "C++23 or later and no std::expected: zu.hpp turned the try_ half off. Check that is included before the feature tests in zu.hpp rather than deleting this line." + * is included here rather than left to zu.hpp, because zu.hpp + * having included it is the thing being checked. If it stops, the macro + * is undefined while zu.hpp reads it and defined by the time this does, + * which is the whole of the original bug and is what fires this. */ +#include + +#if defined(__cpp_lib_expected) && __cpp_lib_expected >= 202202L +#error "std::expected is in this library and zu.hpp turned the try_ half off anyway. Check that is included before the feature tests in zu.hpp rather than deleting this line." #endif ZU_TEST(this_toolchain_has_no_std_expected_and_the_throwing_half_is_enough) { From 2707ef7a1eb5ccaaa373983ed6e4f5ba7dda62c1 Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:47:16 +0700 Subject: [PATCH 3/6] An exact decimal is a zu::Decimal ABI 0.15 added tag 14 and zu_value_decimal, and this wrapper still said 0.14, so the check that the two agree failed before anything was built. A decimal is an unscaled integer and a scale, and the integer is 128 bits, which holds the thirty eight digits DECIMAL(p, s) may declare. C++ has no standard type that wide and MSVC has no extension either, so it is kept as the two halves the ABI hands over. unscaled() rebuilds it where the compiler has __int128, unscaled64() answers for the values that fit a plain one, and to_string writes the number out with the point where the scale puts it and every nought the scale asked for. Not a double, and reading a decimal cell as one is refused rather than answered approximately. Decimal::as_double is the conversion, spelled as a call with a name on it so that giving up the digits is something a program is seen to do. It takes the magnitude before it adds the halves up: a negative decimal has a high half of -1 and a low half a hair under two to the sixty four, and adding those as doubles cancels down to nothing and takes every digit with it. Doxygen needed the int128 pair in PREDEFINED. Without it the two members behind that switch are thrown away before it sees them, so they would be missing from the reference and from api/surface.txt while being present in the header on every compiler this builds on. --- CMakeLists.txt | 6 +- api/surface.txt | 16 +++ docs/Doxyfile.in | 20 ++- include/zu.hpp | 274 ++++++++++++++++++++++++++++++++++++++++- test/test_expected.cpp | 16 +++ test/test_query.cpp | 2 +- test/test_values.cpp | 118 ++++++++++++++++++ 7 files changed, 440 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 05c4dd6..6e3fdf0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -29,7 +29,7 @@ if(POLICY CMP0144) endif() project(zu-cpp - VERSION 0.14.0 + VERSION 0.15.0 DESCRIPTION "The header-only C++ wrapper over libzu" HOMEPAGE_URL "https://github.com/tamnd/zu-c" LANGUAGES C CXX) @@ -83,9 +83,9 @@ if(TARGET zu::zu) # is not fatal here, because the two numbers are counts and a header # newer than this wrapper is usually only wider, but it is worth # saying out loud rather than finding at the first struct. - if(NOT ZU_ABI_VERSION VERSION_EQUAL "0.14") + if(NOT ZU_ABI_VERSION VERSION_EQUAL "0.15") message(WARNING - "zu.h declares ABI ${ZU_ABI_VERSION} and this wrapper was written against 0.14") + "zu.h declares ABI ${ZU_ABI_VERSION} and this wrapper was written against 0.15") endif() else() message(STATUS diff --git a/api/surface.txt b/api/surface.txt index c457ab1..46de997 100644 --- a/api/surface.txt +++ b/api/surface.txt @@ -1,5 +1,7 @@ define ZU_HAS_EXPECTED define ZU_HAS_FORMAT +define ZU_HAS_INT128 +formatter std::formatter< zu::Decimal > formatter std::formatter< zu::Error > formatter std::formatter< zu::Node > formatter std::formatter< zu::Position > @@ -121,6 +123,16 @@ func static zu::Database::try_create(std::string_view, const Config & = Config{} func static zu::Database::try_memory(const Config & = Config{}) -> expected< Database > func static zu::Database::try_open(std::string_view, const Config & = Config{}) -> expected< Database > func zu::Database::try_path() const -> expected< std::string_view > +struct zu::Decimal +func zu::Decimal::as_double() const noexcept -> double +var zu::Decimal::hi -> std::int64_t +var zu::Decimal::lo -> std::uint64_t +func static zu::Decimal::of(std::int64_t, std::int32_t) noexcept -> Decimal +friend zu::Decimal::operator==(const Decimal &, const Decimal &) = default -> bool +var zu::Decimal::scale -> std::int32_t +func zu::Decimal::unscaled() const noexcept -> __int128 +func zu::Decimal::unscaled64() const noexcept -> std::optional< std::int64_t > +func static zu::Decimal::wide(__int128, std::int32_t) noexcept -> Decimal class zu::Error func zu::Error::Error() = default func zu::Error::code() const noexcept -> std::optional< std::string_view > @@ -382,6 +394,7 @@ enum zu::Type -> int enum value zu::Type::binding_table -> ZU_TYPE_BINDING_TABLE enum value zu::Type::boolean -> ZU_TYPE_BOOL enum value zu::Type::bytes -> ZU_TYPE_BYTES +enum value zu::Type::decimal -> ZU_TYPE_DECIMAL enum value zu::Type::floating -> ZU_TYPE_FLOAT enum value zu::Type::graph -> ZU_TYPE_GRAPH enum value zu::Type::integer -> ZU_TYPE_INT @@ -398,6 +411,7 @@ func explicit zu::Value::Value(const zu_value *) noexcept func zu::Value::Value() = default func zu::Value::as_bool() const -> bool func zu::Value::as_bytes() const -> std::span< const std::uint8_t > +func zu::Value::as_decimal() const -> Decimal func zu::Value::as_double() const -> double func zu::Value::as_int() const -> std::int64_t func zu::Value::as_node() const -> Node @@ -415,6 +429,7 @@ func zu::Value::raw() const noexcept -> const zu_value * func zu::Value::size() const noexcept -> std::uint64_t func zu::Value::try_as_bool() const -> expected< bool > func zu::Value::try_as_bytes() const -> expected< std::span< const std::uint8_t > > +func zu::Value::try_as_decimal() const -> expected< Decimal > func zu::Value::try_as_double() const -> expected< double > func zu::Value::try_as_int() const -> expected< std::int64_t > func zu::Value::try_as_node() const -> expected< Node > @@ -461,6 +476,7 @@ func zu::to_string(Status) noexcept -> std::string_view func zu::to_string(Temporal) -> std::string func zu::to_string(TemporalKind) noexcept -> std::string_view func zu::to_string(Type) noexcept -> std::string_view +func zu::to_string(const Decimal &) -> std::string func zu::to_string(const Error &) -> std::string func zu::to_string(const Value &) -> std::string func zu::version() -> std::string_view diff --git a/docs/Doxyfile.in b/docs/Doxyfile.in index 9665430..adea4f7 100644 --- a/docs/Doxyfile.in +++ b/docs/Doxyfile.in @@ -86,13 +86,25 @@ EXPAND_ONLY_PREDEF = YES # reader's problem and not Doxygen's, so they are written out here as # the values the reference is generated at. # -# ZU_FORMATTER expands to nothing so that its ten invocations do not -# arrive as ten functions nobody declared. The macro itself is +# ZU_HAS_INT128 is the same pair for the same reason, and __SIZEOF_INT128__ +# is the compiler macro it is defined from rather than a library one. +# The value is 16, which is what GCC and Clang set it to, though nothing +# here reads it: what matters is that it is defined, since that is the +# whole of the test. Without the pair, Decimal::wide and +# Decimal::unscaled are thrown away before Doxygen sees them, and two +# published names would then be missing from the reference and from +# api/surface.txt while being present in the header on every compiler +# this project builds on. +# +# ZU_FORMATTER expands to nothing so that its eleven invocations do not +# arrive as eleven functions nobody declared. The macro itself is # documented where it is defined and names what it specializes, and -# docs/surface.py puts the ten back, because a specialization a caller -# formats against is published whether Doxygen sees it or not. +# docs/surface.py puts the eleven back, because a specialization a +# caller formats against is published whether Doxygen sees it or not. PREDEFINED = __cpp_lib_expected=202202L \ __cpp_lib_format=201907L \ + __SIZEOF_INT128__=16 \ ZU_HAS_EXPECTED=1 \ ZU_HAS_FORMAT=1 \ + ZU_HAS_INT128=1 \ ZU_FORMATTER(TYPE)= diff --git a/include/zu.hpp b/include/zu.hpp index d9eb552..e5195c9 100644 --- a/include/zu.hpp +++ b/include/zu.hpp @@ -54,6 +54,7 @@ #include +#include #include #include #include @@ -115,6 +116,19 @@ #define ZU_HAS_FORMAT 0 #endif +#if defined(__SIZEOF_INT128__) +/** 1 when this compiler has __int128 and zu::Decimal hands the unscaled + * integer over as one, 0 when it does not and the two halves are the + * whole of it. There is no standard feature test to read here, because + * a 128 bit integer is not a standard type: it is an extension GCC and + * Clang offer on every 64 bit target and MSVC offers on none, so the + * question is which compiler this is rather than which standard. + * Everything else Decimal does is defined either way. */ +#define ZU_HAS_INT128 1 +#else +#define ZU_HAS_INT128 0 +#endif + namespace zu { /* ---- what a call answered ---- */ @@ -162,9 +176,14 @@ enum class Type : int { graph = ZU_TYPE_GRAPH, binding_table = ZU_TYPE_BINDING_TABLE, /** Octets rather than text, so nothing here is validated as UTF-8 and - * nothing is decoded on the way out. Last in the list because the + * nothing is decoded on the way out. Late in the list because the * order is the ABI's numbering and this is what ABI 0.14 added. */ bytes = ZU_TYPE_BYTES, + /** An exact number, read as a zu::Decimal and not as a double. The + * point of the type is the digits the caller wrote, and binary + * floating point is where those go, so as_double is not the reader + * for this and does not answer for it. ABI 0.15 added it. */ + decimal = ZU_TYPE_DECIMAL, }; /** Which temporal a temporal is. The unit follows the kind: days for a @@ -303,6 +322,150 @@ struct Temporal { ///@} }; +namespace detail { + +/** A 128 bit two's complement integer taken apart into how big it is + * and which way it points, which is the shape both of the things done + * with a decimal's unscaled integer want it in. + * + * The sign has to come off first either way. Printing wants the digits + * and a minus in front of them, and adding the halves up as a double + * wants two numbers that point the same way: a negative value has a + * high half of -1 and a low half a hair under two to the sixty four, + * and adding those two as doubles is one enormous negative plus one + * enormous positive, which cancels down to nothing and takes every + * digit that mattered with it. */ +struct Magnitude { + /** The top 64 bits of how big it is. */ + std::uint64_t hi = 0; + /** The bottom 64 bits of how big it is. */ + std::uint64_t lo = 0; + /** Which way it pointed before the sign came off. */ + bool negative = false; +}; + +/** Splits one apart. The most negative value has no positive + * counterpart, so negating it gives the same bits back, which is + * exactly the magnitude wanted: it is read as unsigned from here on and + * never asked to be a signed number again. */ +inline Magnitude magnitude128(std::int64_t hi, std::uint64_t lo) noexcept { + Magnitude out{static_cast(hi), lo, hi < 0}; + if (out.negative) { + out.hi = ~out.hi; + out.lo = ~out.lo; + if (++out.lo == 0) { + out.hi++; + } + } + return out; +} + +} // namespace detail + +/** An exact number: an integer of unscaled units, and how many of its + * digits stand to the right of the point. The value is the unscaled + * integer times ten to the minus scale, so 1234 at scale 2 is 12.34 and + * 1234 at scale 0 is 1234. + * + * It is not a double and it does not become one on the way here. A + * tenth is not a binary fraction, so a price read as binary floating + * point is not the price that was written, and the whole reason the + * engine has this type is to keep the digits the statement asked for + * all the way to the caller. to_string writes them out; as_double is + * the conversion that gives them up, and it is spelled as a call so + * that giving them up is something a program is seen to do. + * + * The unscaled integer is 128 bits, which holds thirty eight digits and + * so holds every decimal the engine will declare. It is kept as the two + * halves the ABI hands over, because C++ has no standard type that wide + * and MSVC has no extension either. unscaled() rebuilds it on a + * compiler that does, and unscaled64() answers for the values that fit + * a plain int64_t, which is most of them. */ +struct Decimal { + /** The top 64 bits of the unscaled integer, two's complement, so this + * is 0 or -1 for every value an int64_t would hold. */ + std::int64_t hi = 0; + /** The bottom 64 bits of the same integer, unsigned. */ + std::uint64_t lo = 0; + /** How many of the digits are after the point, from 0 to 38. Never + * negative: a decimal with no fraction has a scale of nought. */ + std::int32_t scale = 0; + + /** Equal when all three agree, so 12.30 and 12.3 are two decimals + * here. They are one number and the engine compares them equal, but + * they are not the same value: each prints the way it was written and + * a test that expected two places should not pass on one. */ + friend bool operator==(const Decimal&, const Decimal&) = default; + + /** Builds one from an unscaled integer that fits an int64_t and a + * scale. The sign is extended into the high half, which is what makes + * -1 at scale 0 minus one rather than a very large positive. */ + static Decimal of(std::int64_t unscaled, std::int32_t scale) noexcept { + return Decimal{unscaled < 0 ? -1 : 0, static_cast(unscaled), scale}; + } + +#if ZU_HAS_INT128 + ///@{ + /** The full 128 bit integer, taken and given back, on a compiler with + * a type that wide. + * + * A second name rather than an overload of of(), because a literal + * written in a call is an int and converts to either width equally + * well, so two overloads would make Decimal::of(1234, 2) ambiguous on + * exactly the compilers that have both. */ + static Decimal wide(__int128 unscaled, std::int32_t scale) noexcept { + return Decimal{static_cast(static_cast(unscaled) >> 64), + static_cast(unscaled), scale}; + } + __int128 unscaled() const noexcept { + return static_cast<__int128>((static_cast(static_cast(hi)) + << 64) | + lo); + } + ///@} +#endif + + /** The unscaled integer when it fits an int64_t, and nothing when it + * does not. Nineteen digits is where that line falls and the engine's + * decimals go to thirty eight, so a caller who reads this has to say + * what to do with the ones that do not fit, which is the point of + * answering an optional rather than a wrapped number. */ + std::optional unscaled64() const noexcept { + const std::int64_t narrow = static_cast(lo); + if ((narrow < 0 ? -1 : 0) != hi) { + return std::nullopt; + } + return narrow; + } + + /** The number as a double, which is where the exactness stops. + * + * Almost every decimal loses something here, which is why it is a call + * with a name on it rather than a conversion the compiler will do + * behind a caller's back. It is offered because a program that is + * about to draw a chart wants a double and is entitled to say so. */ + double as_double() const noexcept { + /* The magnitude first and the sign at the end. Two to the sixty + * four is what the high half counts, and adding the two halves up + * is only the value when both point the same way, which is what + * detail::Magnitude is for. + * + * Ten to the scale is built up and divided by once rather than + * dividing by ten as many times as the scale says, because each + * division rounds and doing thirty eight of them rounds thirty + * eight times. */ + const detail::Magnitude m = detail::magnitude128(hi, lo); + const double whole = + static_cast(m.hi) * 18446744073709551616.0 + static_cast(m.lo); + double ten = 1.0; + for (std::int32_t i = 0; i < scale; i++) { + ten *= 10.0; + } + const double out = whole / ten; + return m.negative ? -out : out; + } +}; + /* ---- errors ---- */ /** Everything a failure has to say, read out of the zu_error before it @@ -871,6 +1034,11 @@ class Value { * be valid UTF-8 is still a blob and a caller who wanted text should * be told the column is not text. */ std::span as_bytes() const { return detail::unwrap(bytes_impl()); } + /** The exact number, digits and scale both. as_double does not answer + * for a decimal cell and is not meant to: a caller who wants the + * float has Decimal::as_double and asks for it there, where the + * losing of the digits is written down. */ + Decimal as_decimal() const { return detail::unwrap(decimal_impl()); } Temporal as_temporal() const { return detail::unwrap(temporal_impl()); } Node as_node() const { return detail::unwrap(node_impl()); } Rel as_rel() const { return detail::unwrap(rel_impl()); } @@ -911,6 +1079,9 @@ class Value { [[nodiscard]] expected> try_as_bytes() const { return detail::to_expected(bytes_impl()); } + [[nodiscard]] expected try_as_decimal() const { + return detail::to_expected(decimal_impl()); + } [[nodiscard]] expected try_as_temporal() const { return detail::to_expected(temporal_impl()); } [[nodiscard]] expected try_as_node() const { return detail::to_expected(node_impl()); } [[nodiscard]] expected try_as_rel() const { return detail::to_expected(rel_impl()); } @@ -965,6 +1136,14 @@ class Value { } return std::span(p, len); } + detail::Outcome decimal_impl() const { + Decimal d; + if (auto e = detail::checked(zu_value_decimal(v_, &d.hi, &d.lo, &d.scale), + "zu_value_decimal")) { + return std::move(*e); + } + return d; + } detail::Outcome temporal_impl() const { std::int32_t kind = 0; std::int64_t count = 0; @@ -1725,6 +1904,8 @@ T Row::get(std::uint32_t col) const { return std::vector(v.begin(), v.end()); } else if constexpr (std::is_same_v) { return result_->cell(row_, col).as_bool(); + } else if constexpr (std::is_same_v) { + return result_->cell(row_, col).as_decimal(); } else if constexpr (std::is_same_v) { return result_->cell(row_, col).as_temporal(); } else if constexpr (std::is_same_v) { @@ -3383,6 +3564,51 @@ inline std::string printed(double d) { return std::string(buf, len < sizeof buf ? len : sizeof buf - 1); } +/** The digits of a 128 bit two's complement integer held as its two + * halves, with a minus in front where there is one. + * + * Written on the halves rather than on __int128 so that there is one + * implementation and not a portable one nobody exercises beside a fast + * one everybody does. Thirty nine divisions of a number this wide is + * nothing next to the query that produced it. + * + * The division is by ten, one digit at a time, and each round divides + * the high half and carries its remainder into the low. The low half is + * then done in two thirty two bit steps, because a remainder below ten + * shifted up by thirty two still fits a uint64_t while the same + * remainder shifted up by sixty four would not, and this header has no + * wider type to borrow. */ +inline std::string digits128(std::int64_t hi, std::uint64_t lo) { + const Magnitude m = magnitude128(hi, lo); + const bool negative = m.negative; + std::uint64_t high = m.hi; + std::uint64_t low = m.lo; + std::string digits; + while (high != 0 || low != 0) { + std::uint64_t carry = high % 10; + high /= 10; + std::uint64_t top = (carry << 32) | (low >> 32); + const std::uint64_t q_top = top / 10; + carry = top % 10; + const std::uint64_t bottom = (carry << 32) | (low & 0xffffffffu); + const std::uint64_t q_bottom = bottom / 10; + carry = bottom % 10; + low = (q_top << 32) | q_bottom; + digits.push_back(static_cast('0' + carry)); + } + if (digits.empty()) { + return "0"; + } + if (negative) { + digits.push_back('-'); + } + /* Built least significant first, which is the direction the division + * hands them over, so the string is turned round at the end rather + * than each digit being pushed to the front of it. */ + std::reverse(digits.begin(), digits.end()); + return digits; +} + } // namespace detail ///@{ @@ -3455,6 +3681,7 @@ inline std::string_view to_string(Type t) noexcept { case Type::graph: return "graph"; case Type::binding_table: return "binding_table"; case Type::bytes: return "bytes"; + case Type::decimal: return "decimal"; } return "unknown"; } @@ -3503,6 +3730,43 @@ inline std::string to_string(Temporal t) { return out; } +/** The number written out, with the point where the scale puts it and + * no exponent, which is the spelling every other client of this engine + * writes a decimal in and the one the conformance corpus expects. + * + * The scale is honoured rather than trimmed. 12.30 at scale 2 prints + * with both places and 12.3 at scale 1 prints with one, because the + * places are the thing the type is carrying and a printer that dropped + * a trailing zero would be dropping the answer to how precise this is. + * + * A number with fewer digits than its scale is all fraction, and the + * zeros in front of it belong to the value: 5 at scale 3 is 0.005 and + * not 5.000. */ +inline std::string to_string(const Decimal& d) { + std::string out = detail::digits128(d.hi, d.lo); + const bool negative = !out.empty() && out.front() == '-'; + std::string_view digits = out; + if (negative) { + digits.remove_prefix(1); + } + if (d.scale <= 0) { + return out; + } + const std::size_t places = static_cast(d.scale); + std::string front(negative ? "-" : ""); + if (places >= digits.size()) { + front += "0."; + front.append(places - digits.size(), '0'); + front += digits; + return front; + } + const std::size_t cut = digits.size() - places; + front += digits.substr(0, cut); + front += '.'; + front += digits.substr(cut); + return front; +} + /** A failure on one line, which is what a log wants. Error::report is * the other spelling, three lines with a caret under the column, for * the program that is showing somebody a statement to fix. @@ -3554,6 +3818,7 @@ inline std::string to_string(const Value& v) { case Type::rel: return to_string(v.as_rel()); case Type::temporal: return to_string(v.as_temporal()); case Type::bytes: return std::to_string(v.as_bytes().size()) + " bytes"; + case Type::decimal: return to_string(v.as_decimal()); default: break; } return std::string(to_string(v.type())) + " of " + std::to_string(v.size()); @@ -3565,10 +3830,10 @@ inline std::string to_string(const Value& v) { #if ZU_HAS_FORMAT /** std::format over the same text. * - * Defines std::formatter for the ten types zu::to_string prints: + * Defines std::formatter for the eleven types zu::to_string prints: * Status, Severity, Type, TemporalKind, Position, Node, Rel, Temporal, - * Error and Value. Each specialization is one line over the to_string - * overload of the same type, so std::format("{}", v) and + * Decimal, Error and Value. Each specialization is one line over the + * to_string overload of the same type, so std::format("{}", v) and * zu::to_string(v) are the same bytes by construction rather than by * two pieces of code being kept in step. * @@ -3614,6 +3879,7 @@ ZU_FORMATTER(zu::Position); ZU_FORMATTER(zu::Node); ZU_FORMATTER(zu::Rel); ZU_FORMATTER(zu::Temporal); +ZU_FORMATTER(zu::Decimal); ZU_FORMATTER(zu::Error); ZU_FORMATTER(zu::Value); diff --git a/test/test_expected.cpp b/test/test_expected.cpp index 7d86e6e..753f940 100644 --- a/test/test_expected.cpp +++ b/test/test_expected.cpp @@ -121,6 +121,22 @@ ZU_TEST(the_calls_abi_0_14_added_have_the_expected_spelling_too) { CHECK(!absent->has_value()); } +ZU_TEST(the_calls_abi_0_15_added_have_the_expected_spelling_too) { + auto conn = zu::Connection::memory(); + auto r = conn.query("RETURN CAST('1.20' AS DECIMAL(5, 2)) AS d, 1 AS i"); + + const auto d = r.cell(0, 0).try_as_decimal(); + CHECK(d.has_value()); + CHECK_EQ(d->scale, 2); + CHECK_EQ(d->unscaled64().value(), 120); + + /* An integer is not a decimal, and here that comes back rather than + * throws, on the same terms as every other reader on this class. */ + const auto wrong = r.cell(0, 1).try_as_decimal(); + CHECK(!wrong.has_value()); + CHECK_EQ(wrong.error().status(), zu::Status::misuse); +} + ZU_TEST(the_bulk_paths_have_the_expected_spelling_too) { zt::TempDir dir("expected"); const std::string path = dir.file("people.zu"); diff --git a/test/test_query.cpp b/test/test_query.cpp index 46c9d6a..a74757a 100644 --- a/test/test_query.cpp +++ b/test/test_query.cpp @@ -21,7 +21,7 @@ ZU_TEST(a_database_in_memory_answers_a_statement) { } ZU_TEST(the_wrapper_and_the_library_agree_about_the_abi) { - CHECK_EQ(zu::abi_version(), "0.14"); + CHECK_EQ(zu::abi_version(), "0.15"); CHECK(!zu::version().empty()); } diff --git a/test/test_values.cpp b/test/test_values.cpp index 6a11dc0..4cac9a0 100644 --- a/test/test_values.cpp +++ b/test/test_values.cpp @@ -121,6 +121,124 @@ ZU_TEST(octets_and_text_are_not_read_as_one_another) { CHECK_THROWS_AS(zu::Exception, r.cell(0, 1).as_bytes()); } +ZU_TEST(a_decimal_comes_back_with_the_digits_it_was_written_with) { + auto conn = zu::Connection::memory(); + /* CAST is the only way to reach a decimal today. There is no literal + * spelling for one and no column may be declared DECIMAL, so this is + * where they come from and the reason every case here asks this way. */ + auto r = conn.query("RETURN CAST('1.20' AS DECIMAL(5, 2)) AS v"); + CHECK_EQ(r.type(0, 0), zu::Type::decimal); + + const zu::Decimal d = r.cell(0, 0).as_decimal(); + CHECK(d.unscaled64().has_value()); + CHECK_EQ(*d.unscaled64(), 120); + CHECK_EQ(d.scale, 2); + /* Both places, which is the whole point of the type: a double would + * have had neither the value nor the count of digits. */ + CHECK_EQ(zu::to_string(d), std::string("1.20")); + + /* The same value through the typed reader, and equal to one written + * here, because a Decimal is compared member by member. */ + CHECK(r.row(0).get(0) == zu::Decimal::of(120, 2)); + CHECK(r.row(0).get("v") == d); +} + +ZU_TEST(a_decimal_keeps_its_sign_and_its_noughts) { + struct Case { + const char* text; + std::int32_t places; + }; + /* Written out rather than derived from the text, so that the case + * says what scale it expects instead of agreeing with itself. */ + const Case cases[] = { + {"0", 0}, {"1.20", 2}, {"-0.05", 2}, {"1234", 0}, + {"0.005", 3}, {"0.000", 3}, {"-1234.5678", 4}, + }; + + auto conn = zu::Connection::memory(); + for (const Case& one : cases) { + const std::string statement = std::string("RETURN CAST('") + one.text + + "' AS DECIMAL(38, " + std::to_string(one.places) + ")) AS v"; + auto r = conn.query(statement); + const zu::Decimal d = r.cell(0, 0).as_decimal(); + CHECK_EQ(d.scale, one.places); + /* Nothing is normalised on the way through, so 0.000 keeps its + * three places and -0.05 keeps the nought it needs to have a + * hundredth at all. */ + CHECK_EQ(zu::to_string(d), std::string(one.text)); + } +} + +ZU_TEST(a_decimal_wider_than_an_int64_arrives_whole) { + auto conn = zu::Connection::memory(); + /* Thirty eight digits, which is the widest DECIMAL(p, s) may be + * declared and the widest the 128 bit integer behind it holds. */ + const std::string digits(38, '1'); + auto r = conn.query("RETURN CAST('" + digits + "' AS DECIMAL(38, 0)) AS v"); + + const zu::Decimal d = r.cell(0, 0).as_decimal(); + CHECK_EQ(zu::to_string(d), digits); + /* Nineteen digits is where an int64_t stops, so this one says so + * rather than handing back a number that is not the number. */ + CHECK(!d.unscaled64().has_value()); + CHECK(d.hi != 0); +#if ZU_HAS_INT128 + CHECK(zu::Decimal::wide(d.unscaled(), d.scale) == d); +#endif +} + +ZU_TEST(a_decimal_is_not_a_double_and_does_not_read_as_one) { + auto conn = zu::Connection::memory(); + auto r = conn.query("RETURN CAST('0.1' AS DECIMAL(5, 1)) AS a, " + "CAST('0.2' AS DECIMAL(5, 1)) AS b"); + /* Reading it as a float is refused at the call rather than answered + * approximately, which is what makes the exactness a property of the + * API and not of how carefully the caller reads the docs. */ + CHECK_THROWS_AS(zu::Exception, r.cell(0, 0).as_double()); + + const double a = r.cell(0, 0).as_decimal().as_double(); + const double b = r.cell(0, 1).as_decimal().as_double(); + /* And the loss, said out loud. A tenth is not a binary fraction, so + * this is the sum a program that went through double would get. */ + CHECK(a + b != 0.3); + CHECK(a > 0.09 && a < 0.11); + + /* A negative one, where the two halves point opposite ways: the high + * half is -1 and the low is a hair under two to the sixty four, and + * adding those up as doubles is an enormous negative plus an enormous + * positive that cancels down to nothing. The sign has to come off + * before the sum, and this is the case that says it did. */ + CHECK_EQ(zu::Decimal::of(-12345678, 4).as_double(), -1234.5678); +} + +ZU_TEST(a_decimal_written_here_prints_as_the_number_it_is) { + /* No engine in this one. to_string is arithmetic over the two halves + * and the scale, and it is worth checking on the edges rather than + * only on what a CAST happens to produce. */ + CHECK_EQ(zu::to_string(zu::Decimal::of(0, 0)), std::string("0")); + CHECK_EQ(zu::to_string(zu::Decimal::of(0, 3)), std::string("0.000")); + CHECK_EQ(zu::to_string(zu::Decimal::of(1200, 3)), std::string("1.200")); + CHECK_EQ(zu::to_string(zu::Decimal::of(-5, 2)), std::string("-0.05")); + CHECK_EQ(zu::to_string(zu::Decimal::of(-1, 0)), std::string("-1")); + + /* Ten to the nineteen, which is past an int64_t and so past the half + * the halves are usually all of, and the carry from the low half into + * the high one is what this checks. */ + const zu::Decimal wide{0, 10000000000000000000ull, 0}; + CHECK_EQ(zu::to_string(wide), std::string("10000000000000000000")); + + /* The most negative 128 bit number, whose magnitude has no positive + * counterpart. Negating it gives the same bits back, which is what + * the digits are read out of. */ + const zu::Decimal floor{static_cast(0x8000000000000000ull), 0, 0}; + CHECK_EQ(zu::to_string(floor), + std::string("-170141183460469231731687303715884105728")); + + /* Two scales of one number are two decimals here, because each prints + * the way it was written. */ + CHECK(zu::Decimal::of(120, 2) != zu::Decimal::of(12, 1)); +} + ZU_TEST(a_node_is_a_table_and_a_row) { zt::TempDir dir("node"); const std::string path = dir.file("people.zu"); From 81e41e1c1770a322a71a5e26317b407a3f127de6 Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:47:26 +0700 Subject: [PATCH 4/6] Let the slow fixture be slow under memcheck The valgrind job set ZU_TEST_ROWS to 300, which shrank the two fixtures that exist in order to be slow, on the reasoning that memcheck runs the machine forty times slower and that is another way of getting a statement long enough to be watched and interrupted. The arithmetic does not work. The statement counts pairs, so a tenth of the rows is a hundredth of the work, and a hundredth slowed by forty is four tenths: under memcheck at three hundred rows the statement was shorter than it is here at three thousand. The progress watcher asks to be called every millisecond and the statement ended before the thread carrying it got that far, so it never fired, and the case that asserts it fired has been red since this job was added. The interrupt case in the threads suite is the same shape and was only spared by being second in a loop that stopped at the first failure. So the knob goes. The number is written where it is used, and the two cases cost this job the minute they cost it. --- .github/workflows/ci.yml | 17 ++++++++++------- test/harness.h | 39 +++++++++++++++------------------------ test/misuse.c | 2 +- test/threads.c | 2 +- 4 files changed, 27 insertions(+), 33 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ad9cc7..b03c5a7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -254,11 +254,16 @@ jobs: # thread-local arena looks to a leak checker rather than how a # leak looks. # - # ZU_TEST_ROWS shrinks the two fixtures that exist to be slow. - # Memcheck already runs the machine forty times slower, which is - # another way of getting a statement that lasts long enough to be - # interrupted, so the smaller number tests the same thing in a - # fifth of the wall clock. + # This used to set ZU_TEST_ROWS to 300, to shrink the two fixtures + # that exist in order to be slow, on the reasoning that memcheck + # runs the machine forty times slower and that is another way of + # getting a statement long enough to be watched and interrupted. + # The arithmetic does not work: the statement counts pairs, so a + # tenth of the rows is a hundredth of the work, and a hundredth + # slowed by forty is shorter than the whole fixture is natively. + # The watcher never fired, and the case that says it fired is the + # one that has been red here since this job was added. The whole + # fixture runs now and this job costs a minute more. - name: The C suites under memcheck run: | set -eu @@ -268,8 +273,6 @@ jobs: --errors-for-leak-kinds=definite,indirect \ --track-origins=yes "./build-vg/test/$t" done - env: - ZU_TEST_ROWS: 300 # The reference, built the same way the release builds it, and graded # rather than glanced at. diff --git a/test/harness.h b/test/harness.h index 85ba4dc..b5aa9ae 100644 --- a/test/harness.h +++ b/test/harness.h @@ -230,7 +230,7 @@ static inline int zt_next_fd(void) { return fd; } -/* How many rows a fixture that exists in order to be slow should hold. +/* How long a fixture that exists in order to be slow has to last. * * Two cases here need a statement that is still running when something * else happens to it: the progress watcher has to fire while it runs, @@ -238,30 +238,21 @@ static inline int zt_next_fd(void) { * counting pairs over three thousand people, which takes about a third * of a second and is nothing. * - * Under valgrind it is not nothing. Memcheck runs somewhere between - * twenty and fifty times slower, and a third of a second becomes ten - * seconds or more of a job whose other cases finish instantly. So the - * valgrind runs pass a smaller number, and get the same behaviour for - * the same reason: what those two cases need is a statement that lasts - * long enough to be interrupted, and slowing the machine down by forty - * is another way of arriving at one. + * There used to be a ZU_TEST_ROWS here that shrank the number, and the + * valgrind job set it to three hundred on the reasoning that memcheck + * already runs the machine forty times slower and that is another way + * of getting a statement that lasts. It is not. The work is pairs, so + * a tenth of the rows is a hundredth of the statement, and a hundredth + * slowed by forty is four tenths: under memcheck at three hundred rows + * the statement was shorter than it is here at three thousand. The + * watcher never fired, the case that asserts it fired failed, and the + * case that asserts an interrupt lands was next in the same job and + * would have failed the same way. * - * The default is the whole number, so nothing changes for anyone who - * does not set it, and the value is clamped at the whole number so a - * larger one cannot overrun the array the caller sized. */ -static inline uint64_t zt_rows(uint64_t whole) { - const char *set = getenv("ZU_TEST_ROWS"); - unsigned long asked; - char *end = NULL; - if (set == NULL || *set == '\0') { - return whole; - } - asked = strtoul(set, &end, 10); - if (end == set || asked == 0) { - return whole; - } - return (uint64_t)asked > whole ? whole : (uint64_t)asked; -} + * So there is no knob. The number is written where it is used, the two + * cases cost the valgrind job the minute they cost it, and what they + * check is a thing that happened rather than a thing that had time to. + */ typedef struct zt_case { const char *name; diff --git a/test/misuse.c b/test/misuse.c index 7a78bf5..9ff1ef9 100644 --- a/test/misuse.c +++ b/test/misuse.c @@ -570,7 +570,7 @@ ZT_TEST(a_call_back_into_the_library_from_the_watcher_is_refused_rather_than_rac * call made from inside it is a call made from a second thread at a * moment when the first is certainly inside the executor. */ static int64_t ids[3000]; - const uint64_t rows = zt_rows(3000); + const uint64_t rows = 3000; zu_database *db = NULL; zu_conn *conn = NULL; zu_frame *frame = NULL; diff --git a/test/threads.c b/test/threads.c index c4c8813..d992d48 100644 --- a/test/threads.c +++ b/test/threads.c @@ -414,7 +414,7 @@ ZT_TEST(an_interrupt_from_another_thread_stops_the_statement_and_not_the_connect * the connection has to run the next statement normally, which is * what the header says makes this different from closing it. */ static int64_t many[3000]; - const uint64_t rows = zt_rows(3000); + const uint64_t rows = 3000; zu_database *db = NULL; zu_conn *conn = NULL; zu_frame *frame = NULL; From 0f4817677771463b36b4e787869b5ba470e4d292 Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Tue, 25 Aug 2026 16:04:34 +0700 Subject: [PATCH 5/6] Say the 128 bit calls are an extension, where the header says it GCC at -Wpedantic says "ISO C++ does not support __int128", which is true, and with -Werror on top it says it five times and stops. Clang says nothing, which is why this passed twice on three of the four compilers and failed on the fourth. The two calls stay. The alternative is handing a caller who has a 128 bit type the two halves and letting them shift and or them back together, which is the work the type exists to save. So the warning is turned off across those two declarations and back on directly after. It is turned off in the header rather than in this project's flags, because this header is installed and a caller compiles it with theirs. A project that builds at -Wpedantic -Werror, which is a reasonable thing to do, would otherwise fail on a header it only included. What the caller writes in their own file is still theirs to hear about: the pragma covers two declarations and nothing past them, which is what the check with g++ 16 shows, where the header is clean at both -std= c++20 and -std=c++23 and a scratch file that writes __int128 itself still gets told. --- include/zu.hpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/zu.hpp b/include/zu.hpp index e5195c9..732ebd6 100644 --- a/include/zu.hpp +++ b/include/zu.hpp @@ -405,6 +405,21 @@ struct Decimal { } #if ZU_HAS_INT128 +/* __int128 is an extension and -Wpedantic is the flag that says so, in + * so many words: "ISO C++ does not support __int128". It is right, and + * the two calls below are here anyway, because the alternative is + * handing a caller who has a 128 bit type the two halves and letting + * them put it back together. + * + * The silence is written here rather than left to the build, because + * this header is installed and a caller compiles it with their flags + * and not ours. A project that builds at -Wpedantic -Werror, which is a + * reasonable thing to do, would otherwise fail on a header it only + * included. The pragma covers these two declarations and nothing else: + * whatever the caller writes in their own file is still theirs to hear + * about. */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" ///@{ /** The full 128 bit integer, taken and given back, on a compiler with * a type that wide. @@ -423,6 +438,7 @@ struct Decimal { lo); } ///@} +#pragma GCC diagnostic pop #endif /** The unscaled integer when it fits an int64_t, and nothing when it From 420c5e1555da3b955a289826c1805efeb37f6018 Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Tue, 25 Aug 2026 16:04:52 +0700 Subject: [PATCH 6/6] Let the second thread have a turn, and name the five blocks the engine keeps The memcheck job ran misuse and stopped, because misuse failed, so the threads suite has not run under valgrind since the job was added. It runs now, and it had two things wrong with it. The first is ours. one_connection_in_two_threads_is_refused_rather_ than_raced asserts the intruding thread made at least one call, and natively it makes thousands. Under memcheck it made none: valgrind runs one thread at a time, the first thread never blocks, and it got through its five hundred statements and set the stop flag before the thread it started was let go once. The assertion was false about the scheduler rather than about the engine. So the first thread waits on a condition the second raises after its first call, which cannot hang, because the only way past the wait is a call that has already happened. The second is the engine's. Five allocations survive the close that should give them back: the database, the connection, the frame and the statement in the interrupt case, and the whole connection in the sharing case, holding 14,349 bytes more between them. The misuse suite opens and closes hundreds of the same handles the same way and reports nothing at all, so this is not closing a connection leaking. It is closing a connection leaking after it has been interrupted or shared, and it wants an unstripped build of the library to say which. That is tamnd/zu#778. Until 778 closes, those five are suppressed by name, one rule each, each naming the call the block came in through and the case that made it. Not a rule about libzu.so, which would be four lines instead of forty and would suppress the bug this job exists to catch: a connection zu-c forgot to close, or a result it forgot to free, is a different stack and is still an error. Indirectly lost stops counting as an error along with it, because an indirectly lost block is one hanging off a definitely lost block, so counting the root counts the children, and it is what lets the roots be suppressed by name at all. It is still shown. --- .github/workflows/ci.yml | 19 +++++++---- test/memcheck.supp | 71 ++++++++++++++++++++++++++++++++++++++++ test/threads.c | 37 +++++++++++++++++++++ 3 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 test/memcheck.supp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b03c5a7..6b355dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -248,11 +248,17 @@ jobs: # The two C files, because they are the ones that hand raw # pointers across and give everything back by hand. # - # Definitely and indirectly lost are errors. Possibly lost is not: - # it is one block of 180 bytes, it belongs to a thread the library - # started, and a pointer into the middle of a block is how a - # thread-local arena looks to a leak checker rather than how a - # leak looks. + # Definitely lost is an error. Indirectly lost is shown and not + # counted, because an indirectly lost block is by definition one + # hanging off a definitely lost block: count the root and the + # children come with it, and a leak that had no root would not be + # indirect. It is the spelling that lets the five roots the engine + # keeps be suppressed by name in test/memcheck.supp rather than by + # a rule about libzu.so wide enough to hide this job's own point. + # Possibly lost is not an error either: it is one block of 180 + # bytes, it belongs to a thread the library started, and a pointer + # into the middle of a block is how a thread-local arena looks to + # a leak checker rather than how a leak looks. # # This used to set ZU_TEST_ROWS to 300, to shrink the two fixtures # that exist in order to be slow, on the reasoning that memcheck @@ -270,7 +276,8 @@ jobs: for t in misuse threads; do valgrind --error-exitcode=99 --leak-check=full \ --show-leak-kinds=definite,indirect \ - --errors-for-leak-kinds=definite,indirect \ + --errors-for-leak-kinds=definite \ + --suppressions=test/memcheck.supp \ --track-origins=yes "./build-vg/test/$t" done diff --git a/test/memcheck.supp b/test/memcheck.supp new file mode 100644 index 0000000..d16573a --- /dev/null +++ b/test/memcheck.supp @@ -0,0 +1,71 @@ +# The five blocks the engine keeps, named one at a time. +# +# tamnd/zu#778. Closing a connection gives everything back, except +# after the connection has been interrupted or shared with a second +# thread, and then five allocations survive it: the database, the +# connection, the frame, the statement, and the connection the other +# case made without a database of its own. Between them they hold +# 14,349 more bytes indirectly, which is what a live connection would +# be expected to hold. +# +# The other suite in this job, misuse, opens and closes hundreds of the +# same handles the same way and reports nothing at all, so this is not +# closing a connection leaking. It is closing a connection leaking +# after something has happened to it, which is the engine's to find +# with an unstripped build of its own. +# +# Each of these names the call the block came in through and the case +# that made it, so the suppression covers those two cases and nothing +# else. A connection zu-c forgot to close anywhere else in this suite, +# or a result it forgot to free, is a different stack and is still an +# error. That is the whole reason these are written out one at a time +# rather than as one rule about libzu.so: a rule that broad would +# suppress the bug this job exists to catch. +# +# They come out when 778 closes. + +{ + 778: the database an interrupted connection was opened on + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:zu_database_memory + fun:an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection +} + +{ + 778: the connection that was interrupted + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:zu_connect + fun:an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection +} + +{ + 778: the frame registered on it + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:zu_frame_new + fun:zu_frame_new_z + fun:an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection +} + +{ + 778: the statement the interrupt stopped + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:zu_query + fun:an_interrupt_from_another_thread_stops_the_statement_and_not_the_connection +} + +{ + 778: the connection two threads shared + Memcheck:Leak + match-leak-kinds: definite + fun:malloc + fun:zu_memory + fun:one_connection_in_two_threads_is_refused_rather_than_raced +} diff --git a/test/threads.c b/test/threads.c index d992d48..ce8b4d5 100644 --- a/test/threads.c +++ b/test/threads.c @@ -273,6 +273,40 @@ static void set_stop(int value) { pthread_mutex_unlock(&stop_lock); } +/* The second thread having got a turn, which is a thing to wait for + * rather than a thing to hope for. + * + * The case below asserts that the intruding thread made at least one + * call, and natively it makes thousands. Under memcheck it made none: + * valgrind runs one thread at a time and hands the turn over at points + * of its own choosing, and the first thread never blocks, so it can run + * its five hundred statements and set the stop flag before the thread + * it started has been let go once. The assertion was then false about + * the scheduler rather than about the engine. + * + * So the first thread blocks on this instead, and the second thread is + * let go because something is waiting for it. It cannot hang: the only + * way past the wait is a call the second thread has already made, and + * the second thread makes one before it looks at the stop flag a second + * time. */ +static pthread_cond_t ran = PTHREAD_COND_INITIALIZER; +static int has_run; + +static void note_a_run(void) { + pthread_mutex_lock(&stop_lock); + has_run = 1; + pthread_cond_signal(&ran); + pthread_mutex_unlock(&stop_lock); +} + +static void wait_for_a_run(void) { + pthread_mutex_lock(&stop_lock); + while (!has_run) { + pthread_cond_wait(&ran, &stop_lock); + } + pthread_mutex_unlock(&stop_lock); +} + static void *share_the_connection(void *arg) { struct sharer *s = (struct sharer *)arg; while (!should_stop()) { @@ -280,6 +314,7 @@ static void *share_the_connection(void *arg) { zu_status st = zu_query_z(s->conn, "RETURN 1 AS one", &res, NULL); zu_result_free(res); s->calls++; + note_a_run(); if (st == ZU_MISUSE_CONCURRENT) { s->refused++; } else if (st == ZU_OK) { @@ -312,6 +347,7 @@ ZT_TEST(one_connection_in_two_threads_is_refused_rather_than_raced) { memset(&state, 0, sizeof state); set_stop(0); + has_run = 0; ZT_CHECK_EQ(zu_memory(&conn, NULL), ZU_OK); state.conn = conn; @@ -347,6 +383,7 @@ ZT_TEST(one_connection_in_two_threads_is_refused_rather_than_raced) { zu_result_free(res); } + wait_for_a_run(); set_stop(1); pthread_join(other, NULL);