One convention used in C++ programs are to place each major class into their own header and source file. While the source code for major classes for this project are already in separate files (table, parser and translator), all the class definitions were in a single header file and this header file was getting unwieldy.
So in preparation for all the new classes that will be added for the GUI, the major classes were separated into their own header files including the Token class with its own source file. The translator source file was also getting large, so the token handler and command handler functions were moved into their own source files with associated header files.
As part of this reorganization, many of the dated change comments were removed as these were just cluttering up the code. None of the change comments in the header file were changed, but probably should as they are taking up quite a few lines. The awk scripts also were modified since the sources of some of the enumerations were relocated and the CMake file was updated accordingly for the new files.
Several static access functions were added to the Translator class so that the token and command handler functions have access to the static values in the translator source file, which were previously accessible since these functions were in the same source file. These handlers are also accessing many other Translator internal data members (the reason they are defined as friend functions). This also should be changed, which will be done when the Translator class is transitioned.
[commit 1254b8048b]
Saturday, November 3, 2012
Friday, November 2, 2012
Qt Transition – Parser (Final)
Support for immediate command parsing was implemented in anticipation of the temporary console mode interface, essentially the way GW-Basic works. However, now that the project is being transitioned to Qt for a GUI, these immediate commands are no longer needed. Another factor for removing this was that a lot of modifications to the command parsing functions, which accounted for about a third of the Parser code, would need to be converted to using Qt functions. A waste of time since this would eventually be removed. Therefore, immediate commands support was removed along with parser test 1.
The only code currently using the String class was the Token class. The Parser class generated tokens and therefore the strings inside the tokens. The String class has been removed from the Parser and Token classes, which now uses Qt functions for parsing and no longer uses any of the standard C library functions (the goal for the entire program, but one step at a time).
Since the String class is no longer used, the header and source files for this class were also removed along with the string test program and expected results file. The other three test programs were also removed because these no longer apply, which were for testing conversion of number strings (Qt functions now used), exceptions thrown from constructors (no longer used), and testing operator processing on a stack (Stack class was already removed).
Upon testing on Windows, the range error checking problem reported on October 14 returned. This was due to how numbers are converted in the Qt libraries. Originally on Linux exponents -308 and below caused a range error, but on Windows exponents did cause a range error until -324. Now with Qt, on Linux exponents -324 and below cause a range error and on Windows it takes exponents -509 and below. Therefore, the test value was changed to 1.234e-509.
[commit 6b4afaa538] [commit 348339c7a7] [commit 819fac5185]
The only code currently using the String class was the Token class. The Parser class generated tokens and therefore the strings inside the tokens. The String class has been removed from the Parser and Token classes, which now uses Qt functions for parsing and no longer uses any of the standard C library functions (the goal for the entire program, but one step at a time).
Since the String class is no longer used, the header and source files for this class were also removed along with the string test program and expected results file. The other three test programs were also removed because these no longer apply, which were for testing conversion of number strings (Qt functions now used), exceptions thrown from constructors (no longer used), and testing operator processing on a stack (Stack class was already removed).
Upon testing on Windows, the range error checking problem reported on October 14 returned. This was due to how numbers are converted in the Qt libraries. Originally on Linux exponents -308 and below caused a range error, but on Windows exponents did cause a range error until -324. Now with Qt, on Linux exponents -324 and below cause a range error and on Windows it takes exponents -509 and below. Therefore, the test value was changed to 1.234e-509.
[commit 6b4afaa538] [commit 348339c7a7] [commit 819fac5185]
Thursday, November 1, 2012
String to Number Conversions
As the modified Parser code was being tested, a weird memory issues was reported by valgrind. The problems occurred with the toInt() and toDouble() functions of QString. The problem was duplicated with a very simple program:
No solution was found for this problem. When the program was changed from QString to QByteArray, which also contains these same two functions, no memory issue was reported. Therefore, as a temporary solution, the string to convert is converted to a QByteArray. A QByteArray was declared and the QString to convert was appended to it.
#include <QString>The same issue occurs if the program above is changed to double with toDouble(). No reason for this error could be found. However, when this program is turned into a Qt console application, the error no long occurred. But this same thing applied to the ibcp program did not eliminate the error. Click Continue... for how to build and run this program from the command line to demonstrate the memory issue (requires Linux with Qt and valgrind installed).
int main(void) {
qDebug("%d", QString("123").toInt();
}
No solution was found for this problem. When the program was changed from QString to QByteArray, which also contains these same two functions, no memory issue was reported. Therefore, as a temporary solution, the string to convert is converted to a QByteArray. A QByteArray was declared and the QString to convert was appended to it.
Minor Change – Vector vs. Map
One of the changes made to the test code was to put the names of each test mode into a QMap where the enumeration name for the test mode is associated with the name of the test mode (as a QString). This was the original code (the array is then indexed by the enumeration value) and the names were declared separate so there could be referenced directly (though the name[] could have been used to get the names):
[commit ad08201092]
char parser_name[] = "parser";The problem with this code is that the programmer must insure that the correct names are placed in the array in the correct order matching the enumeration values, otherwise the wrong name will be used. This is also basically why the Code and TokenStatus enumerations are automatically generated - to eliminated possible coding errors. Using QMap was a way to eliminate this possibility. This was the resulting code (the map is still indexed by the enumeration value) and the name map was used to access the names:
char expression_name[] = "expression";
char translator_name[] = "translator";
char *name[] = {
parser_name, expression_name, translator_name
};
QMap<testModeEnum, QString> name;However, the enumeration declaration had to moved outside the function or it would not compile (apparently, local enumerations can be used). In later considering this code, a better method would be to use a pre-sized QVector since a QMap has more overhead that is really not needed here and the enumeration values are in order. The code was changed to the nearly identical:
name[testParser] = "parser";
name[testExpression] = "expression";
name[testTranslator] = "translator";
QVector<QString> name(sizeofTestMode);Where the sizeofTestMode value was added to the end of the enumeration (which was moved back into the function) so that the vector could be allocated ahead of time. Using QMap would be needed if the indexes being associated were not in numerical order.
name[testParser] = "parser";
name[testExpression] = "expression";
name[testTranslator] = "translator";
[commit ad08201092]
Wednesday, October 31, 2012
Minor Build And Test Issues
A build issue was discovered where CMake does not create a release string if the git command is present but the git repository is not (for instance when building from a downloaded archive). The git describe command was returning an error and no release string. CMake now detects this situation and sets the release string the same as if no git command is found.
A test issue was discovered on Windows when building from a downloaded archive, which contains Unix format files (newline only) and not DOS format files (CRLF). When the program is run from the regression test script (the program builds fine), the output files are in DOS format, but the compares fail because the expected output files are in Unix format. The cmp command was changed to the diff command, which has an option to ignore the difference in the line separators (the ‑w ignore white space option).
All these changes have been pushed to GitHub and because of the build and test issues found, new tag v0.2‑2 was added. The changes to the Parser were complete (with the attempt to compile next) before the text stream detour and these other minor issues.
[commit 0e85c83d56] [commit 2be4bd2f91]
A test issue was discovered on Windows when building from a downloaded archive, which contains Unix format files (newline only) and not DOS format files (CRLF). When the program is run from the regression test script (the program builds fine), the output files are in DOS format, but the compares fail because the expected output files are in Unix format. The cmp command was changed to the diff command, which has an option to ignore the difference in the line separators (the ‑w ignore white space option).
All these changes have been pushed to GitHub and because of the build and test issues found, new tag v0.2‑2 was added. The changes to the Parser were complete (with the attempt to compile next) before the text stream detour and these other minor issues.
[commit 0e85c83d56] [commit 2be4bd2f91]
Qt Transition – Strings (File Input)
There was an issue in the way the test files were being read. When the test code was modified to use Qt, the QFile class was used to read the file, where the function used to read a line is actually inherited from the QIODevice class, which QFile is based on. This function returns the line into a QByteArray, which was easily converted to a character array currently used by the Parser.
However, the Parser is being converted to use the QString class. The QString class actually contains QChar characters, which supports 16-bit Unicode. Reading the file as QByteArray would need to be converted to a QString and would not support Unicode text files. After doing some research, it was found that files can be read as Unicode text using the QTextStream class (into a QString).
Therefore, the file reading code was modified to use a QTextStream (where a pointer to the QFile instance is given in the constructor). This will also be used for the standard (console) input for the interactive test modes. The file is opened the same way, but the at end of file check and read line routines from QTextStream are used instead. This read line routine also strips the line separator from the line (ether newline on Linux or CRLF on Windows).
Temporarily, the QString line is converted to a QByteArray, a null character is added to the end and then converted to character array (constant character pointer) to pass to the Parser. So that a type cast to a char * was not needed, the argument and variables in the Parser were changed to const char *, which is fine since the Parser does not modify the input line.
[commit 993aa66765]
However, the Parser is being converted to use the QString class. The QString class actually contains QChar characters, which supports 16-bit Unicode. Reading the file as QByteArray would need to be converted to a QString and would not support Unicode text files. After doing some research, it was found that files can be read as Unicode text using the QTextStream class (into a QString).
Therefore, the file reading code was modified to use a QTextStream (where a pointer to the QFile instance is given in the constructor). This will also be used for the standard (console) input for the interactive test modes. The file is opened the same way, but the at end of file check and read line routines from QTextStream are used instead. This read line routine also strips the line separator from the line (ether newline on Linux or CRLF on Windows).
Temporarily, the QString line is converted to a QByteArray, a null character is added to the end and then converted to character array (constant character pointer) to pass to the Parser. So that a type cast to a char * was not needed, the argument and variables in the Parser were changed to const char *, which is fine since the Parser does not modify the input line.
[commit 993aa66765]
Monday, October 29, 2012
Qt Transition – Strings (Begin)
The last class to replace is the String class. The Qt equivalent class is QString and related QByteArray. As with the List class, the transition will be done in steps. The Token class would be first. The first primary user of the Token class is the Parser class, which is also very dense with string operations. The QString class has many useful functions, and these should help simplify the Parser functions.
Before tackling the Parser, the test code will need to be able to handle the change from String to QString. The are two other major items in the test code that also needed to be transitioned to Qt, namely file handling, console input and console output.
The Qt QFile class handles file handling and is much simpler to use then the c file handling that was being used. The QFile class can also handle reading input from the console. There is also the QFileInfo class that contains many useful functions, but the ones used here were for parsing file names (including the program name). There are single functions for extracting the path, file name, base file name without extension.
The QTextStream class handles output and is very similar to C++ stream output. In the main source file, the standard output is opened as a QFile and is attached to a QTextStream named cout. This text stream is then passed to all the functions that need to do output.
As described in the previous post, the opportunity was taken to rename many of the variables and functions to the Qt naming convention. Now that the callers to the Parser (and Translator) have been transitioned to Qt, it's time to work on the Parser (and Token) classes.
[commit d94cd3c632]
Before tackling the Parser, the test code will need to be able to handle the change from String to QString. The are two other major items in the test code that also needed to be transitioned to Qt, namely file handling, console input and console output.
The Qt QFile class handles file handling and is much simpler to use then the c file handling that was being used. The QFile class can also handle reading input from the console. There is also the QFileInfo class that contains many useful functions, but the ones used here were for parsing file names (including the program name). There are single functions for extracting the path, file name, base file name without extension.
The QTextStream class handles output and is very similar to C++ stream output. In the main source file, the standard output is opened as a QFile and is attached to a QTextStream named cout. This text stream is then passed to all the functions that need to do output.
As described in the previous post, the opportunity was taken to rename many of the variables and functions to the Qt naming convention. Now that the callers to the Parser (and Translator) have been transitioned to Qt, it's time to work on the Parser (and Token) classes.
[commit d94cd3c632]
Qt Transition – Qt Naming Convention
Part of the Qt transition is will be use the Qt naming convention for classes, functions and variables. Specifically, Qt uses camel casing where the first letter of words in a name are upper case, where the first letter of the name is lower case (except for class names where the first letter is upper case. There is also a convention for naming memory variables along with there access functions. This is best shown with this example class:
class MyClass {Note the underline character that is used for separating the words of the variable and function names and the names of the access function. The arguments on the set functions were also prefixed with an underline to make the name unique from the memory variable. Using the Qt naming convention, this class definition will look list this:
int count;
int some_value;
public:
int get_count(void) {
return count;
}
void set_count(int _count) {
count = _count;
}
int get_some_value(void) {
return some_value;
}
int set_some_value(int _some_value) {
some_value = _some_value;
}
bool empty(void);
bool data(void;
void process_some_data(int data, int more_data);
}
class MyClass {Note the "m_" prefix on each member variable, but not on member functions. Also note that getter functions do not begin with get. And other than the "m_" prefix, underlines do not appear on any names. Also note that boolean member functions that return status are prefixed with is or has. As the Qt transition continues, this naming convention will be used (and this has already started with some of the changes made so far).
int m_count;
int m_someValue;
public:
int count(void) {
return m_count;
}
void setCount(int count) {
m_count = count;
}
int someValue(void) {
return m_someValue;
}
int setSomeValue(int someValue) {
m_someValue = someValue;
}
bool isEmpty(void);
bool hasData(void);
void processSomeData(int data, int moreData);
}
Qt Transition – Stacks
The next class to replace is the Stack class. The Qt equivalent is the QStack class. Internally the Stack class is implemented with an array that grows as needed. The QStack class is based on the QVector class, where its members are in contiguous memory (essentially also an array).
The Stack class has two functions not present in QStack (or the underlying QVector). The first is a push function that takes no arguments, which is used to add an element to the stack without copying a value to the new element (which was shortly followed by setting this new element using the top function). The second is a null pop function that removes the top element from the stack without actually returning its value (like the regular pop function does).
The QStack (nor the QVector base) class have similar functions, but can be simulated using the resize and size functions of the QVector class. For example, this line would be used to add an element to the top of the stack:
The four stacks using in the Translator were replaced with QStack using the substitutes for the absent functions. The stack.h header files was removed (along with the program that tested stacks). The next change in the Qt transition will be more radical, so this would be a good place to add the v0.2-1 development tag (and archives will be available).
[commit 5d7c634713]
The Stack class has two functions not present in QStack (or the underlying QVector). The first is a push function that takes no arguments, which is used to add an element to the stack without copying a value to the new element (which was shortly followed by setting this new element using the top function). The second is a null pop function that removes the top element from the stack without actually returning its value (like the regular pop function does).
The QStack (nor the QVector base) class have similar functions, but can be simulated using the resize and size functions of the QVector class. For example, this line would be used to add an element to the top of the stack:
stack.resize(stack.size() + 1);Similarly, the top element of the stack can be removed by using subtraction in the line above.
The four stacks using in the Translator were replaced with QStack using the substitutes for the absent functions. The stack.h header files was removed (along with the program that tested stacks). The next change in the Qt transition will be more radical, so this would be a good place to add the v0.2-1 development tag (and archives will be available).
[commit 5d7c634713]
Sunday, October 28, 2012
Qt Transition – Error Lists
The remaining item that uses the List class are the error lists generated by the Table class constructor (for errors detected in the table entries). This constructor added errors to an error list, and at the end of the constructor, if there were any errors, the constructor threw an exception containing the pointer to the errors list. The token initialization routine also previously threw an exception for any token status errors detected, but this code was removed when the enum.awk script was implemented to generate the token status enumeration and it detects any errors.
Because there were two types of errors and both were handling errors the same way, an Error template was implemented that could be used for both code (table) and token status errors. This template also handled outputting the errors using a print function that was passed to it. This design was also rather complicated. I mentioned previously that Qt did not support exceptions, but this was not correct. Qt can be used with exceptions, but none of the Qt classes and functions throw exceptions themselves.
Since the exception and error template design was complicated, it was removed. For the table initialization to be able to return errors, the code was moved from the constructor to a new initialization function. As for the error list to return, the QStringList class was used. This class is a specialized list class, equivalent to QList<QString>.
Hit Continue... for details of the new table initialization implementation. All lists have now been replaced with Qt equivalents. The list.h header file was removed (along with the program that tested lists). The next replacement will be the various stacks.
[commit a27e456222]
Because there were two types of errors and both were handling errors the same way, an Error template was implemented that could be used for both code (table) and token status errors. This template also handled outputting the errors using a print function that was passed to it. This design was also rather complicated. I mentioned previously that Qt did not support exceptions, but this was not correct. Qt can be used with exceptions, but none of the Qt classes and functions throw exceptions themselves.
Since the exception and error template design was complicated, it was removed. For the table initialization to be able to return errors, the code was moved from the constructor to a new initialization function. As for the error list to return, the QStringList class was used. This class is a specialized list class, equivalent to QList<QString>.
Hit Continue... for details of the new table initialization implementation. All lists have now been replaced with Qt equivalents. The list.h header file was removed (along with the program that tested lists). The next replacement will be the various stacks.
[commit a27e456222]
Qt Transition – Token Lists
There were two lists used to keep a list of allocated tokens and a list of tokens that were deleted multiple times. These lists were used to detect token memory leaks (tokens not released). To support this detection, the new and delete operators of the Token structure were overloaded. In addition to allocating the memory (using regular new), the token was also added to the allocated list. When deleted, the token was removed from the allocated list. After processing a line, any tokens still in the allocated list was considered a leak. Similarly the deleted token list kept track of any tokens that were deleted more than once.
Apparently, the Qt classes (including QList, QLinkedList and QVector) do not interface with overloaded new and delete functions (obscure compiler errors result). Since another method is now being used to detect memory leaks (valgrind), this detection code is not essential. The advantage of this code was that it would output the exact tokens that were not released or were deleted twice. This was nice when the Translator was first implemented and debugged, but now that Translator code is fully working, this level of detail is probably (and hopefully) not necessary. Therefore, these token lists were removed along with the overloaded new and delete operators.
[commit d21d3d38ad]
Apparently, the Qt classes (including QList, QLinkedList and QVector) do not interface with overloaded new and delete functions (obscure compiler errors result). Since another method is now being used to detect memory leaks (valgrind), this detection code is not essential. The advantage of this code was that it would output the exact tokens that were not released or were deleted twice. This was nice when the Translator was first implemented and debugged, but now that Translator code is fully working, this level of detail is probably (and hopefully) not necessary. Therefore, these token lists were removed along with the overloaded new and delete operators.
[commit d21d3d38ad]
Saturday, October 27, 2012
Building On Windows With Qt
In order to build the program (that now requires Qt), CMake needs to be able to find the Qt files. It accomplishes this by looking for the Qmake executable (qmake.exe on Windows) even if it is not actually used, but its location is used to determine where Qt is installed. To find the Qmake executable, CMake searches the directories in the execution path.
On Linux, the qmake executable is already in the standard directories. On Windows, the directory for qmake.exe needs to be added to the execution path. The instructions for adding a directory to the execution path was given in the post on October 20. The directory C:\QtSDK\Desktop\Qt\4.8.1\mingw\bin needs to be added. If using the MSYS command line, the directory /c/QtSDK/Desktop/Qt/4.8.1/mingw/bin needs to be added to the path (see post on October 7).
QtCreator can be used to access the git repository (see post on October 20). To switch to the latest development branch (branch0.2 at the time of this post), the branch needs to retrieved from the repository on GitHub. On the Tools menu, select Git and then Pull. Only local branches can be checked out, so a local branch needs to be created. On the Tools menu, select Git and then Branches.... Select branch0.2 under origin and click the Add... button. Click OK on the next Dialog. This will create local branch0.2, which can now be selected and checked out with the Checkout button. Select Close to dismiss the Branches dialog.
To build, first select Run CMake on the Build menu and make sure the Generator is set to MinGW Generator (MinGW (x86 32bit)). Now click the Run CMake button. As mentioned previously, sometimes a bunch of errors occur the first time. Trying a second time causes no errors. If at any time there are problems running CMake, deleted every file in the qtcreator-build and try again.
There is a problem in running the regtest script in Windows XP when the build directory is under the current user directory under the C:\Documents and Settings\ directory. The problem occurs because there are spaces in the directory name, which causes bash to incorrectly process the line. There is no problem on Windows 7 because the user directories are under the C:\Users\ directory (which has no spaces). There is also no problem if the build directory is under the user home directory within MSYS (again no spaces). As for regtest.bat, this batch file requires the build to be performed in the source directory. Also note that the new memory test script is not available on Windows.
On Linux, the qmake executable is already in the standard directories. On Windows, the directory for qmake.exe needs to be added to the execution path. The instructions for adding a directory to the execution path was given in the post on October 20. The directory C:\QtSDK\Desktop\Qt\4.8.1\mingw\bin needs to be added. If using the MSYS command line, the directory /c/QtSDK/Desktop/Qt/4.8.1/mingw/bin needs to be added to the path (see post on October 7).
QtCreator can be used to access the git repository (see post on October 20). To switch to the latest development branch (branch0.2 at the time of this post), the branch needs to retrieved from the repository on GitHub. On the Tools menu, select Git and then Pull. Only local branches can be checked out, so a local branch needs to be created. On the Tools menu, select Git and then Branches.... Select branch0.2 under origin and click the Add... button. Click OK on the next Dialog. This will create local branch0.2, which can now be selected and checked out with the Checkout button. Select Close to dismiss the Branches dialog.
To build, first select Run CMake on the Build menu and make sure the Generator is set to MinGW Generator (MinGW (x86 32bit)). Now click the Run CMake button. As mentioned previously, sometimes a bunch of errors occur the first time. Trying a second time causes no errors. If at any time there are problems running CMake, deleted every file in the qtcreator-build and try again.
There is a problem in running the regtest script in Windows XP when the build directory is under the current user directory under the C:\Documents and Settings\ directory. The problem occurs because there are spaces in the directory name, which causes bash to incorrectly process the line. There is no problem on Windows 7 because the user directories are under the C:\Users\ directory (which has no spaces). There is also no problem if the build directory is under the user home directory within MSYS (again no spaces). As for regtest.bat, this batch file requires the build to be performed in the source directory. Also note that the new memory test script is not available on Windows.
Qt Transition – Translator RPN List
Before embarking on the memory issues, the Qt Transition was started by changing all List to QList and changing all the access functions accordingly. However, that was going to take while, so those changes were abandoned to start a new approach in only changing one list at a time, and running the new memory test script after each. This started with the Translator's RPN output list.
Wow, what a learning experience. One nice feature of the current List class was being to obtain a pointer to a particular element in the list. Since the underlining code was implemented as a linked list, the element pointer could be used to access elements previous and following the element. Though admittedly, the syntax for these element pointers was rather involved and therefore confusing.
The QList class has several ways to access the elements including by index (something List was not capable of) and with iterators (similar to but not the same as the pointers to list elements). In the Translator, the list element pointer was used in several places, which to modified to use QList.
1. The done stack used to hold pointers to tokens added the output list, which are used when processing operators and functions when checking data types of the operands and arguments. The done stack actually contained pointers to the output list elements (of RPN items containing the token). There was no reason to have a pointer to the list element here, so it was changed to just point to the RPN item.
2. The RPN item structure contained an array of list element pointers for each of the operands of the token. Operands are only kept for identifiers with a parentheses token (until it is determine whether it is an array of function), defined functions with parentheses token (needed to check with the functions arguments), and string operator tokens (until it is determine whether the operands are temporary or not). There was no reason for these pointers to point to list elements, so the array was changed to an array of RPN item pointers.
3. The command stack used to hold the current command contained a pointer to a list element. Currently, the INPUT command handler uses this pointer to insert tokens into the output list. Several unsuccessful attempts were made to use Qt iterators for this pointer, but this only caused weird memory issues to be reported by valgrind (though the code appeared to work). In the end, an index into the output list was used since QList items can be accessed by index. In one location of the code, an iterator was used to access the last element in the list, and if a hidden token, stepped back to the previous element. An iterator worked here because it did not require saving the iterator value and trying to use it later, which appears to cause problems.
One List to QList change is complete; now on to the rest. The CMakeLists.txt file was also modified to support building with Qt, which involved finding the Qt package and added the Qt libraries to the executable (this works on Linux, but Windows requires some additional steps before it can be compiled).
[commit 34cbd66cd9]
Wow, what a learning experience. One nice feature of the current List class was being to obtain a pointer to a particular element in the list. Since the underlining code was implemented as a linked list, the element pointer could be used to access elements previous and following the element. Though admittedly, the syntax for these element pointers was rather involved and therefore confusing.
The QList class has several ways to access the elements including by index (something List was not capable of) and with iterators (similar to but not the same as the pointers to list elements). In the Translator, the list element pointer was used in several places, which to modified to use QList.
1. The done stack used to hold pointers to tokens added the output list, which are used when processing operators and functions when checking data types of the operands and arguments. The done stack actually contained pointers to the output list elements (of RPN items containing the token). There was no reason to have a pointer to the list element here, so it was changed to just point to the RPN item.
2. The RPN item structure contained an array of list element pointers for each of the operands of the token. Operands are only kept for identifiers with a parentheses token (until it is determine whether it is an array of function), defined functions with parentheses token (needed to check with the functions arguments), and string operator tokens (until it is determine whether the operands are temporary or not). There was no reason for these pointers to point to list elements, so the array was changed to an array of RPN item pointers.
3. The command stack used to hold the current command contained a pointer to a list element. Currently, the INPUT command handler uses this pointer to insert tokens into the output list. Several unsuccessful attempts were made to use Qt iterators for this pointer, but this only caused weird memory issues to be reported by valgrind (though the code appeared to work). In the end, an index into the output list was used since QList items can be accessed by index. In one location of the code, an iterator was used to access the last element in the list, and if a hidden token, stepped back to the previous element. An iterator worked here because it did not require saving the iterator value and trying to use it later, which appears to cause problems.
One List to QList change is complete; now on to the rest. The CMakeLists.txt file was also modified to support building with Qt, which involved finding the Qt package and added the Qt libraries to the executable (this works on Linux, but Windows requires some additional steps before it can be compiled).
[commit 34cbd66cd9]
Thursday, October 25, 2012
Third Translator Memory Issue
The memory issue reported for translator test 7 was the same message as test 6 except on a different line, which was another if statement. Again through the process of elimination, the statement causing the error was identified to be MID$(A$ B, which is expected to produce an expected comma error at the B token.
After briefly studying this if statement, the problem was identified and is similar to the previous problem. This if statement, in the process binary operator routine, was checking if the token (within a sub-string assignment) was not a comma. Again, it should not have been checking the token code before checking that the code was valid (since not all token types have a code). The B token is an identifier with no parentheses token type and code is not used.
The if statement was corrected by adding a check if the token has a table entry (and therefore a valid code) and it is not a comma. Now all the memory issues were resolved. I also now understand what the Conditional jump or move depends on unintialised values(s) error is indicating. Apparently, the Analyzer (valgrind) is checking for more than just memory leaks, it is also checking when a variable is being accessed, but it hasn't been initialized, which was the case for these two if statements.
Rechecking all the tests, all the memory issues were resolved. However, upon running the regression tests, translator test 7 was now failing. The problem occurred with the statement above, which was now reporting an expected operator or comma error, which was wrong because with a sub-string assignments, an operator is not allowed after the string variable identifier.
The if statement was modified so that either the token does not have a table entry or the token is not a comma. All tests now pass. To simplify (and automate) this memory testing, a new memtest script was created, but only for Linux as it requires the valgrind program. This new script is based on the regtest script and also checks the regression test results along with checking for memory issues. Now back to replacing the List class with the QList class...
[commit 1d427d7d98] [commit da0014b34d]
After briefly studying this if statement, the problem was identified and is similar to the previous problem. This if statement, in the process binary operator routine, was checking if the token (within a sub-string assignment) was not a comma. Again, it should not have been checking the token code before checking that the code was valid (since not all token types have a code). The B token is an identifier with no parentheses token type and code is not used.
The if statement was corrected by adding a check if the token has a table entry (and therefore a valid code) and it is not a comma. Now all the memory issues were resolved. I also now understand what the Conditional jump or move depends on unintialised values(s) error is indicating. Apparently, the Analyzer (valgrind) is checking for more than just memory leaks, it is also checking when a variable is being accessed, but it hasn't been initialized, which was the case for these two if statements.
Rechecking all the tests, all the memory issues were resolved. However, upon running the regression tests, translator test 7 was now failing. The problem occurred with the statement above, which was now reporting an expected operator or comma error, which was wrong because with a sub-string assignments, an operator is not allowed after the string variable identifier.
The if statement was modified so that either the token does not have a table entry or the token is not a comma. All tests now pass. To simplify (and automate) this memory testing, a new memtest script was created, but only for Linux as it requires the valgrind program. This new script is based on the regtest script and also checks the regression test results along with checking for memory issues. Now back to replacing the List class with the QList class...
[commit 1d427d7d98] [commit da0014b34d]
Second Translator Memory Issue
While the first memory issue was a simple memory leak, the second issue was much more difficult to resolve. The issue on translator test 6 was reported as a Conditional jump or move depends on unintialised values(s). Clicking on this showed the source line and another message reporting Uninitialised value was created by a heap allocation. The line for this message was in the token new function where the memory for the token is allocated. Curious that the token allocation checks were not reporting any token leaks.
Through the process of elimination, the statement (out of 42) causing the error was identified to be PRINT A(TAB(10)). Looking at the line indicated by the first message (an if statement) did not make it clear what the issue was. So the code was stepped through with the debugger to identify the problem, which was caused by an incorrect check for the item on top of the hold stack, which happen to be the line reported with a problem.
This if statement checks if a print-only function is found in an expression, which should be reported as an error since these functions are only valid in a PRINT command. The check is for either the current command is not a PRINT command or the token on top of the hold stack is not the null token (any other token indicates the print-only function is in a parentheses, array or function - an error).
The problem was with the null token check as it was only checking if the code of the token was not the null code. However, not every token type has a valid code, specifically, constants, identifiers (with and without parentheses), and user defined functions (preceded by FN with and without parentheses). This check was replaced with a call to a new token function that checks to see if the token is a null token, which first checks if the token has a table entry (and therefore a code) and then checks the code.
This corrected the problem with translator test 6. All the expression and translator tests were rechecked with the Analyzer. Translator test 7 was still reporting a memory issue.
Through the process of elimination, the statement (out of 42) causing the error was identified to be PRINT A(TAB(10)). Looking at the line indicated by the first message (an if statement) did not make it clear what the issue was. So the code was stepped through with the debugger to identify the problem, which was caused by an incorrect check for the item on top of the hold stack, which happen to be the line reported with a problem.
This if statement checks if a print-only function is found in an expression, which should be reported as an error since these functions are only valid in a PRINT command. The check is for either the current command is not a PRINT command or the token on top of the hold stack is not the null token (any other token indicates the print-only function is in a parentheses, array or function - an error).
The problem was with the null token check as it was only checking if the code of the token was not the null code. However, not every token type has a valid code, specifically, constants, identifiers (with and without parentheses), and user defined functions (preceded by FN with and without parentheses). This check was replaced with a call to a new token function that checks to see if the token is a null token, which first checks if the token has a table entry (and therefore a code) and then checks the code.
This corrected the problem with translator test 6. All the expression and translator tests were rechecked with the Analyzer. Translator test 7 was still reporting a memory issue.
First Translator Memory Leak
The first memory leak, which occurred on every expression and translator test, was easy to identify and correct as the Analyzer pointed directly to the problem. The memory issue was reported in the translator start() function with the RPN (reverse polish notation) list allocated there.
For expression test 1, 13 blocks were reported lost, which exactly coincided with the number of test expressions. This made it obvious that the memory allocated for the RPN list object was not being released. In the translate input routine, after the resulting tokens in the RPN list were output, the memory allocated for each token was released. However, the RPN list object itself was not released.
After the correction, all the expression and translator tests were rechecked with the Analyzer. Translator tests 6 and 7 were still reporting memory issues.
For expression test 1, 13 blocks were reported lost, which exactly coincided with the number of test expressions. This made it obvious that the memory allocated for the RPN list object was not being released. In the translate input routine, after the resulting tokens in the RPN list were output, the memory allocated for each token was released. However, the RPN list object itself was not released.
After the correction, all the expression and translator tests were rechecked with the Analyzer. Translator tests 6 and 7 were still reporting memory issues.
Subscribe to:
Posts (Atom)