Code Review

by Brown, Friday, January 15, 2021, 10:11 (324 days ago) @ Brown
edited by Brown, Friday, January 15, 2021, 10:15

I gathered some more information and collected it into a form others might find useful.

There are 32 Proofgold modules, though two of them (config and proofgold) are a bit different. config.ml is created by the configure script and has values specific to the node setup. proofgold.ml is the "main" file that starts all the threads and then runs the console or daemon. (Technically there is a 33rd module proofgoldcli, but this is just a command line interface to communicate with the daemon if it's running).

Each module may define types, values and exceptions. Some values are functions and some of these functions are recursive. Generally I think a code review/code documentation should report the following about each item.

Each type should be given a short text description.

Each exception should be given a short text description, indicate which functions raise the exception and indicate which functions catch the exception.

Each value should be given a short text description. If the value is a function, then the following should be reported:

1. Are there bugs or potential bugs?
2. Does the function terminate and is it intended to?
3. What exceptions can be raised directly by the function?
4. What exceptions can be raised indirectly by the function via an internal function call?
5. What exceptions can be caught by the function?
6. Are there any critical sections in the code? What could go wrong if two threads entered the critical section at onece?

If the function is recursive, then the following should be reported:

7. Is the function tail recursive? If not, could it be modified to be tail recursive?

Here is some lisp code collecting the names of the modules, what modules each depend upon and what items are defined in the module:

http://s000.tinyupload.com/?file_id=19399511542592928349

In total there are almost 1600 items, so a full review and documentation will take some time and is more than I can do by myself. The review can be done in 17 phases where the modules with no dependencies are reviewed first (in parallel in principle) and later modules can be done after previous phases are complete. Calling (review-outline) lists the "plan" with 17 phases. Calling (review-outline t) gives the "plan" with the 1600 items included under each module where it is declared. This full outline is available here:

http://s000.tinyupload.com/?file_id=84028890324646711754

I'll post the outline without items in a comment below.

The "short text descriptions" of many items can be taken from the Qeditas documentation:

http://qeditas.org/docs/QeditasTechDoc.pdf

Note however that the Qeditas documentation is about 5 years out of date, so it will be up to the reviewer/documenter to determine if the old description still applies.