Speak ABI 0.15, and make the three things that were red green - #13
Merged
Merged
Conversation
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.
…s 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 <version> included on the spot, which is the only thing that can tell a toolchain without std::expected from a header that mislaid it.
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.
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.
tamnd
changed the title
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.
…e 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mainhas been red since the valgrind and TSan job landed, and three separate things were wrong. This is all of them.The engine moved to ABI 0.15 and this wrapper still said 0.14, so the check that the two agree failed before anything was built. 0.15 added tag 14 and
zu_value_decimal, and a wrapper that claimed the number without reading the value would be claiming a version it does not speak, so the decimal comes with it.zu::Decimalis an unscaled 128 bit integer and a scale, kept as the two halves the ABI hands over because C++ has no standard type that wide, withunscaled()where the compiler has__int128,unscaled64()for the values that fit a plain one, andto_stringwriting the number out with the point where the scale puts it. Reading a decimal cell as a double is refused rather than answered approximately:Decimal::as_doubleis the conversion and is spelled as a call so that giving up the digits is something a program is seen to do.ONis a reserved word now, soRETURN f.on AS onis a syntax error and the alias goes in accent quotes. The property is still namedon, because a property is read by name rather than parsed as one.The
std::expectedfloor build fired its#erroron clang, which does not havestd::expectedat-std=c++23and never claimed to: clang reports__cpp_conceptsas 201907L, libstdc++ gates every C++23 library feature on 202002L, and the library never switches on. The gate was asking whether the standard is C++23 when the bug it guards is about the library, so it now reads__cpp_lib_expectedwith<version>included on the spot.And the valgrind job shrank the two fixtures that exist in order to be slow. The statement counts pairs, so a tenth of the rows is a hundredth of the work, and a hundredth slowed by memcheck's forty is shorter than the whole fixture is natively. The progress watcher never fired and the case that asserts it fired is the one that has been failing. The knob goes and the number is written where it is used.
Two more came out from under those. GCC at
-Wpedantic -Werrorsays "ISO C++ does not support__int128" and stops, which clang does not say at all, so the decimal compiled on three of the four compilers. The two 128 bit calls stay and the warning is turned off across their declarations, in the header rather than in this project's flags, because the header is installed and a caller compiles it with theirs.And with misuse passing, the threads suite ran under memcheck for the first time. Its sharing case asserts the intruding thread made at least one call, which natively it makes thousands of and under valgrind it made none, because valgrind runs one thread at a time and the first thread never blocks. It waits on a condition now.
What is left is the engine's, and it is filed as tamnd/zu#778. 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. The misuse suite opens and closes hundreds of the same handles the same way and reports nothing, so this is not closing a connection leaking, it is closing a connection leaking after it has been interrupted or shared. The five are suppressed by name in
test/memcheck.supp, one rule each naming the call the block came in through and the case that made it, so that the job can go green over everything else it checks. Not a rule aboutlibzu.so, which would suppress the bug this job exists to catch. They come out when 778 closes.