Having stated the principle in the previous commit, I should apply it consistently. A source file linked into the Puzzles library of common support code should not also define a main() under ifdef. This commit only goes as far as the _library_ support modules. It would be a much bigger job to do the same for all the actual _puzzles_ that have test main()s or standalone-solver main()s. And it's not necessary, because modifying one of those source files only triggers a rebuild of _one_ puzzle, not absolutely everything. (Not to mention that it's quite likely the puzzle and the test main() will need to be modified in conjunction anyway.) As in the previous commit, this has required exposing a few internal API functions as global, and maybe editing them a bit. In particular, the one-shot internal function that divvy_rectangle() loops on until it succeeds is now exposed as divvy_rectangle_attempt(), which means the test program doesn't have to condition a failure counter into the real function. I've thrown away penrose-vector-test completely, because that didn't look like a test program with any ongoing use at all - it was surely vestigial, while James was getting the vector representation up and running in the first place.
After Ben fixed all the unwanted global functions by using gcc's
-Wmissing-declarations to spot any that were not predeclared, I
remembered that clang has -Wmissing-variable-declarations, which does
the same job for global objects. Enabled it in -DSTRICT=ON, and made
the code clean under it.
Mostly this was just a matter of sticking 'static' on the front of
things. One variable was outright removed ('verbose' in signpost.c)
because after I made it static clang was then able to spot that it was
also unused.
The more interesting cases were the ones where declarations had to be
_added_ to header files. In particular, in COMBINED builds, puzzles.h
now arranges to have predeclared each 'game' structure defined by a
puzzle backend. Also there's a new tiny header file gtk.h, containing
the declarations of xpm_icons and n_xpm_icons which are exported by
each puzzle's autogenerated icon source file and by no-icon.c. Happily
even the real XPM icon files were generated by our own Perl script
rather than being raw xpm output from ImageMagick, so there was no
difficulty adding the corresponding #include in there.
I noticed commit db3b531e2cab765a00475054d2e9046c9d0437d3 in the history where Simon added a bunch of "static" qualifiers. That suggested that consistently marking internal functions "static" is desirable, so I tried a build using GCC's -Wmissing-declarations, which requires prior declaration (presumed to be in a header file) of all global functions. This commit makes the GTK build clean under GCC's -Wmissing-declarations. I've also adding "static" to a few obviously internal objects, but GCC doesn't complain about those so I certainly haven't got them all.
In the setup phase of the centralised latin.c solver, we start by going over the input grid containing already-placed clue numbers, and calling latin_solver_place to enter each on into the solver's data structure. This has the side effect of ruling out each number from the rest of the row and column, and _also_ checking by assertion that the number being placed is not ruled out. Those are a bad combination, because it means that if you give an obviously inconsistent input grid to latin_solver_alloc (e.g. with two identical numbers in a row already), it will fail an assertion. In that situation, you want the solver run as a whole to return diff_impossible so that the error is reported cleanly. This assertion failure could be provoked by giving either Towers or Group a manually-constructed game description inconsistent in that way, and hitting Solve. Worse, it could be provoked during live play in Unequal, by filling in a number clashing with a clue and then pressing 'h' to get hints.
I've only just realised that there's a false-positive bug in the latin.c solver framework. It's designed to solve puzzles in which the solution is a latin square but with some additional constraints provided by the individual puzzle, and so during solving, it runs a mixture of its own standard deduction functions that apply to any latin-square puzzle and extra functions provided by the client puzzle to do deductions based on the extra clues or constraints. But what happens if the _last_ move in the solving process is performed by one of the latin.c built-in methods, and it causes a violation of the client puzzle's extra constraints? Nothing will ever notice, and so the solver will report that the puzzle has a solution when it actually has none. An example is the Group game id 12i:m12b9a1zd9i6d10c3y2l11q4r . This was reported by 'groupsolver -g' as being ambiguous. But if you look at the two 'solutions' reported in the verbose diagnostics, one of them is arrant nonsense: it has no identity element at all, and therefore, it fails associativity all over the place. Actually that puzzle _does_ have a unique solution. This bug has been around for ages, and nobody has reported a problem. For recursive solving, that's not much of a surprise, because it would cause a spurious accusation of ambiguity, so that at generation time some valid puzzles would be wrongly discarded, and you'd never see them. But at non-recursive levels, I can't see a reason why this bug _couldn't_ have led one of the games to present an actually impossible puzzle believing it to be soluble. Possibly this never came up because the other clients of latin.c are more forgiving of this error in some way. For example, they might all be very likely to use their extra clues early in the solving process, so that the requirements are already baked in by the time the final grid square is filled. I don't know! Anyway. The fix is to introduce last-minute client-side validation: whenever the centralised latin_solver thinks it's come up with a filled grid, it should present it to a puzzle-specific validator function and check that it's _really_ a legal solution. This commit does the plumbing for all of that: it introduces the new validator function as one of the many parameters to latin_solver, and arranges to call it in an appropriate way during the solving process. But all the per-puzzle validation functions are empty, for the moment.
This is the main bulk of this boolification work, but although it's making the largest actual change, it should also be the least disruptive to anyone interacting with this code base downstream of me, because it doesn't modify any interface between modules: all the inter-module APIs were updated one by one in the previous commits. This just cleans up the code within each individual source file to use bool in place of int where I think that makes things clearer.
This commit removes the old #defines of TRUE and FALSE from puzzles.h, and does a mechanical search-and-replace throughout the code to replace them with the C99 standard lowercase spellings.
latin_check now returns bool, and latin_solver_diff_set takes a bool 'extreme' flag. Should be non-disruptive.
Oops! This was left over from an early development version of commits 4408476b7 and 000ebc507, in which I initially arranged for each adjacency list to be terminated with the sentinel value -1 instead of separately storing an array of the lists' lengths. I later changed the representation to make randomising the algorithm easier (it's much easier to shuffle an array uniformly at random if you _don't_ have to faff endlessly to work out its length). But this write of a no-longer- needed sentinel value in the client code must have survived the rewrite by mistake, and also somehow evaded all my pre-commit testing with valgrind and asan. A user reported that the Towers Javascript version was crashing on startup, and I think this is the cause, because it seems to fix it for me.
The new client code is a lot smaller and nicer, because we can throw away the col[] and num[] permutations entirely. Of course, this also means that every puzzle that incorporated latin.c now has to link against matching.c instead of maxflow.c - but since I centralised that secondary dependency into Recipe a few commits ago, it's trivial to switch them all over at once.