diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ad9cc7..6b355dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -248,28 +248,38 @@ 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. # - # 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 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 - env: - ZU_TEST_ROWS: 300 # The reference, built the same way the release builds it, and graded # rather than glanced at. 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..732ebd6 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,166 @@ 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 +/* __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. + * + * 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); + } + ///@} +#pragma GCC diagnostic pop +#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 +1050,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 +1095,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 +1152,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 +1920,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 +3580,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 +3697,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 +3746,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 +3834,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 +3846,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 +3895,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/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/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/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/test_expected.cpp b/test/test_expected.cpp index 131826d..753f940 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" @@ -118,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"); @@ -184,16 +203,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) { 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); 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"); diff --git a/test/threads.c b/test/threads.c index c4c8813..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); @@ -414,7 +451,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;