Extending the Priority Cross-referencer

The other day I added some new code to a procedure that I had written several months ago. The purpose of the code was to sum the quantity of a raw material found in the kititems of the work orders belonging to a given order line. The actual query for a given line worked correctly when I checked it in WINDBI, but inside the procedure it didn't work properly. The code itself was within a cursor loop and it was clear that the loop did not complete.

Eventually I realised that I had left out a few of the commands necessary for a cursor, primarily missing checking RETVAL after fetching a new tuple (to see whether there are more tuples). The procedure worked correctly once I added the missing commands. 

Just to be clear, the complete order of commands should be

1. Declare
2. Open
3. Retval 
4. Fetch
5. Retval
6. Loop
7. Close

Once I had the procedure working, I decided to add more checks to PrioXRef. 'Retval' and 'Loop' differ from the other cursor commands in that the others are always followed by the cursor name, whereas 'Retval' and 'Loop' aren't. Just to complicate matters, 'Retval' is a variable (it has the colon prefix).

After some thinking, I figured out that I needed to store cursor names in a stack (specifically, after 'declare'), so that 'Retval' and 'Loop' can reference the current cursor name. 'Close' 'pops' the current cursor off the stack. This part eventually worked well but the analysis of the nodes in the parse tree for a cursor became more complicated, primarily because 'Retval' has to appear twice in specific places. After working on this for a while, I wrote new code that checks that all seven commands appear in the correct order. This code can't check in a strictly linear fashion: let's say that the first 'retval' is missing; the third node in the cursor tree will be 'fetch' which is not the third identifier ('retval'), but it is the fourth identifier. 

I've only checked the new version against a simple, toy, procedure, but next week I'll check the version against real code with multiple cursors, some of which are cursors within cursors (this is the reason for the stack).

Edit from 18/12/21: It seems that I was premature in stating that LOOP had been handled. I woke up this morning, realising that there are cases of LOOP that I had not handled: in about 20% of the procedures that I write, there are multiple LOOP statements for the same cursor. Very occasionally there is a procedure that has a LOOP with no cursor - this generally happens when iterating over a series of dates. Both of these cases would break what I wrote yesterday.

Edit from 03/01/22: Unfortunately I have to rewrite the code for RETVAL. Quite often I write code with a cursor (or even two) embedded within a cursor. Whilst the first cursor would have 'goto 500 where :retval <= 0' after the 'open' command, the internal cursor would have 'loop 100 where :retval <= 0' if there is only one cursor. There wouldn't be a problem if there were two internal cursors as presumably the second has to execute even if the first fails (and so after 'open' would be written 'goto <label> where :retval <= 0). It looks like I am going to have to write some kind of state machine with much more detail than I have done so far (e.g. the only cursor command that can come after 'declare' is 'open'; anything else is a mistake). 

Further work on the fault analyser (PrioXRef) - beware of linked files

I mentioned the other day a 270 line Priority procedure that I used as a test for the cross referencer. This procedure was at the proof of concept stage: it did what it was supposed to do but made several assumptions. I improved it to be more general, a conceptual change that required many changes throughout the procedure, that amongst other things builds a new purchase part with specific characteristics. At the end of the procedure, a purchase order is created that includes this new part. As such, the part needs to have a price in the supplier's price list based on the base part of the new part - whilst creation of the purchase order was part of the POC procedure, addition of the part to the price list wasn't.

After adding the appropriate code for the price list, I saw that it wasn't working, leading to a painful debugging session before the penny dropped. In the query that retrieved the price of the base part, I was using the table PART and this was what was causing the problem - PART was linked and of cause the actual part that I was using wasn't in the linked table. What made this error particularly galling was that I had already documented this problem.

As happens frequently these days, I woke up in the middle of the night. After dealing with my bladder, the first thought that popped into my head was that PrioXRef doesn't deal with 'UPDATE' and 'DELETE' statements; the second thought was how I could get the program to deal with the linked table problem. Unfortunately, this problem occupied my mind for quite some time and I had to take cognitive steps to banish it so that I could resume sleeping.

At the moment, I don't think that there is a way of showing this as a fault. How is PrioXRef supposed to know that the part that I am using as a parameter is not in the linked table? Of course, the problem would not exist if I had the internal part number of :SEARCHNAME and then I wouldn't have to include the PART table in this query. 

/* Get price from supplier's pricelist */                          
SELECT SPARTPRICE.PRICE, SUPPRICELIST.SUPPLDATE, 
SUPPRICELIST.SUPPLIST 
INTO :PRICE, :SDATE, :SLIST                                                                                       
FROM SUPPRICELIST, SPARTPRICE, PART, SUPPLIERS                      
AND SUPPRICELIST.SUP = SUPPLIERS.SUP 
WHERE SUPPRICELIST.SUPPLIST = SPARTPRICE.SUPPLIST                   
AND SPARTPRICE.PART = PART.PART                                     
AND SUPPLIERS.SUPNAME = :SUPNAME                                    
AND PART.PARTNAME = :SEARCHNAME 
ORDER BY 2 DESC;  

The only idea that I have at the moment is to display a warning like "[line number] warning: PART is linked". In fact, whilst PrioXRef knows that on line X that table PART is linked and is then unlinked on line Y, it can't use that information as it ignores SELECT statements. That's not exactly true: SELECT is not a keyword that triggers some handling, but INTO and FROM are such keywords. I should add some code to the FROM handling that checks whether any of the following tables are stored in the binary tree of identifiers. The FROM code should continue to be in effect until the next keyword is found (there may not be a WHERE clause, ORDER BY or GROUP BY in the query). PrioXRef has to check that the token following each part is not an alias (the above problem was solved by aliasing PART which causes the real table to be joined).

This might require a rewrite of the tokeniser as at the moment it does not save the token terminator (this would be a comma in the above example) and so getting the alias may cause the next table name to be obscured. On the other hand, there is code that pushes the next token back into the current text line to be parsed if it's not the expected token. Rewriting the alias detection code should be rewritten and changed to be a function so that it can be called at several different places.

A few people have received PrioXRef, but I have yet to receive any comment bar 'Thanks' and 'Nice idea'.

Using the cross referencer in the wild

So far, I've been using the cross referencer to check programs that have been debugged. Even so, I am finding problems here and there, primarily forgetting to unlink tables.

This morning I wrote a new procedure that updates bills of materials; the details aren't particularly important. Before running it "in the wild", I ran the code through the cross referencer and received the following analysis (to my chagrin): :ANAME [Variable]:  used before initialised; used only once. This is exactly the sort of problem that the program is supposed to find. I then look in the references section for this variable and discover that its sole reference is on line 44. I then look in the code window and find this snippet

37: :FANAME = '';
38: SELECT ACT.ACTNAME INTO :FANAME
39: FROM PARTARC, ACT
40: WHERE PARTARC.ACT = ACT.ACT
41: AND PARTARC.PART = :FMP
42: AND PARTARC.SON = :SMP;
43: LOOP 100 WHERE :ORIGNAME = :ANAME; /* no change, so skip */

Apart from the fact that :ANAME is referenced in line 43 and not 44, the error is obvious: the code finds a value and saves it in :FANAME, but the rest of the code blithely uses :ANAME. ERROR!!

Actually, in this program that's not too bad an error and will cause lines in the bill of materials to be updated unnecessarily, but that's not the point.

Introducing the Priority procedure cross referencer and fault analyser

 Over the months, I have collected various mistakes that I have made whilst programming procedures in Priority and that were not noted by the in-built syntax checker. I want to write a program that will check things that are not caught by the internal program, to supplement it and not replace it. Simply put, I want to write an external syntax checker that will check things like matched LINK/UNLINK pairs, uninitialised variables and a few other problems. I've been devoting a fair amount of thought as to how to store the data of such a program; traditional cross reference programs in Pascal used linked lists, and indeed I found such a program yesterday evening. But I would like to have a much more modern interface and use types such as stringlists and similar. A stringlist is ideal for storing what would have been an array of identifiers but isn't so useful when additional data regarding those identifiers is required.

I will no doubt continue to debate the subject in my mind until I commence coding; at the moment, my inclination is to take the old school cross referencer and adapt it to the Priority SQL syntax. This will be a complex task that would have to be done one way or another, so it's probably better to start with something that works so that I can concentrate on the syntax and not on how everything is stored. A cross referencer is a good idea anyway: it makes finding references to a variable much easier. I started writing a long program in Priority yesterday afternoon and finished writing and debugging this morning: this is 270 lines long which is fairly long but not too complicated. During the writing process, I moved pieces from place to place within the program (primarily moving non-variant operations out of loops, to be technical) and sometimes these edits slightly mangled the text. I discovered a new bug: a variable will always start with a colon (e.g. :DAYS); in the course of one of these edits and pastes, I had a variable named ::DAYS which is not the same as :DAYS. 

A cross referencer helps in finding variables that appear only once; this can mean that either the variable is superfluous as it is never used, or more problematic, it is a variable without value (as in the above case of ::DAYS). I wasted an hour yesterday on another procedure, trying to figure out why a value being saved in a variable was not being written later on. Eventually I saw the problem: the value was saved in :PARTCOST but was later accessed as :PARCOST. A cross referencer would find this immediately.

After spending more than a few hours over the weekend working on the cross-referencer, I have completed the first version.

As in the army, everything divides into three. For this program, the first stage is parsing the input file, then displaying the references and finally displaying the analysis. The first stage can also be split into three: the tokeniser, the lexical analysis and the storage. A token is a string extracted from a text file; for example, if the current line is 'select part, partname from part', then there are five tokens: 'select', 'part', 'partname', 'from' and again 'part'. In programming languages with regular syntax, the tokeniser is normally quite straight-forward, but it turns out that the procedural SQL language of Priority does not have regular syntax and cannot be considered to be context free.

Two examples of the ad hoc syntax: I want to note when a variable is initialised and when it is not. Initialisation can occur in one of two forms: either there is an equals sign after the token (e.g. :SEARCHNAME = '12345') or the keyword INTO precedes the token (e.g. SELECT DAY INTO :DAYS). These two opposite options (one prefix and one postfix, to use the technical terms) make it complicated to program. Another syntactic problem is the colon - :. Normally this serves to mark variables, e.g. :DAYS, but it can also be used to separate between two clauses in a ternary comparison (e.g. :DAYS < 7 ? 3 : 5). 

The correct tokenisation of table aliases (e.g. GENERALLOAD F1) took quite a bit of time.

Storage of the identifiers and their references is by means of a binary tree; this part was based on the cross referencer that I found a few days ago which was written in standard Pascal. The references are stored in a queue for each node. I added a few fields to these variable types in order to store further information: the type of identifier (variable, cursor, table) and the operation in progress at the reference (e.g. variable initialisation, opening a cursor, linking a table). This part was simple. Displaying the references was also fairly straight-forward.

The analysis part is dependent on the type of identifier: there are certain checks for variables, certain checks for cursors and certain checks for tables. I found a method to make these checks as stream-lined as possible.

I tested the program by running it alternately on a short test file into which at times included deliberate errors (so that I could check that the errors were being picked up) and on the file for the procedure that I wrote a few days ago. Every time I would look at the references, noting mistakes that had to be fixed. Now I'm 99% confident that I've correctly parsed the files and have correctly denoted variable initisalisation (this was very complicated). Running the finished program on my procedure finds three variables that were initialised and never used. These can be safely deleted from the procedure.

My next step is to publicise the program within a small community, inviting examples of procedures whose analysis appears to be wrong. Maybe there are other checks that need to be added.