6

Background

I am writing a compiler for a custom language for a school project and it is going really well for me.

If I where to start all over from scratch I would have done many software architecture things different, but right now I do not have time or motivation for a complete rewrite so I will focus on just finishing this project instead.

There is one thing in my code that I want to refactor right now and it is how I organize my assertions.

I am writing this compiler in the C using flex and bison to generate my lexer and parser. The code in this project is not very clean and quite messy but I am not ashamed of it (compilers are quite complex and this is my first try). But I have added assertions in the code to make bugs get spotted in time and easier to track down.

The problem

The problem I am having with assert in this project is that there is simply too much to check for.

Here is an example from a function that adds the function parameters to the symbol table when compiling a function definition:

static void add_function_params_to_symbols(struct context *con, 
        const struct ast_op *function_def) {
    struct ast *child = function_def->childs[0];
    assert(child);
    assert(child->type == SYN_DECLARATOR);
    assert(child->val_type == AST_OP);
    child = child->val.op.childs[1];
    assert(child);
    assert(child->type == SYN_FUNCTION);
    assert(child->val_type == AST_OP);
    child = child->val.op.childs[1];
    assert(child);
    assert(child->type == SYN_PARAMETER_LIST);
    assert(child->val_type == AST_OP);
    parameter_list_to_symbols(con, &child->val.op);
}

This function has three times as many assertions as code statements!

This is quite an extreme example because this function has to go down three levels to the bottom of the tree in order to find the PARAMETER_LIST which it then passes to parameter_list_to_symbols which adds the parameters to the symbol table.

The parse tree for a function definition in my compiler looks somewhat like this:

FUNCTION_DEF
 DECLARATOR
  IDENTIFIER [function_name]
  FUNCTION
   [return type]
   PARAMETER_LIST
    [declarators containing type and name for each parameter]
    [...]
 FUNCTION_BODY
  [statements in function body]
  [...]

There are four things for me to test for each node that I use throughout the program.

  1. assert(node) to check that the node is not NULL

  2. assert(node->type == SYN_FOOBAR) to check that the node is of the expected type (like SYN_EXPRESSION or SYN_VARIABLE).

  3. assert(child->val_type == AST_OP) to check that the node has the right kind of union value. Nodes can also contain other values. For example in nodes that store a string value child->val_type would be equal to AST_STRING. AST_OP means that the node is an operator node which is a node that has children.

  4. I should probably also assert that each node has the right number of children with assert(child->val.op.n_childs == XXX) but I have not done that in this code example because it is already so full of assertions and asserting this too does not seem to add much value.

I have probably written hundreds of assertions like this in my code and it feels like this is getting quite hard to maintain but I am not sure how I should reorganize it.

The question

Am I "over asserting" stuff in my code? What things should I assert and what things can I assume are OK?

How should I structure my assertions to make them easier to maintain?

If you have more tips on using assertions and similar tests in code please mention them in your answer.

closed as primarily opinion-based by Scant Roger, user22815, Michael Kohne Dec 2 '15 at 14:03

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    Assertions should only be used to find programming errors. The question to ask yourself is: If any of those assertions fails, does that indicate I made an error in my compiler or does it fail because I am trying to compile some strange source file that my compiler should be able to handle gracefully. Assertions are only suitable for the first of those. – Bart van Ingen Schenau Nov 29 '15 at 9:22
  • 1
    @BartvanIngenSchenau I am using assertions to find bugs not to check input – wefwefa3 Nov 29 '15 at 16:46
12

Am I "over asserting" stuff in my code?

No. You have even admitted that you know of at least one little thing that you could have asserted but you haven't, so you are not asserting enough.

What things should I assert and what things can I assume are OK?

Assert everything. Assume nothing.

How should I structure my assertions to make them easier to maintain?

One technique that I use is that I have special assertion functions which are meant to be invoked from within assertions, to perform many assertions together. They always begin with the prefix 'assert', and they always return true, so that if none of the contained assertions fails, then the calling assertion will succeed. So, for example:

boolean assertNodeOfTypeXYZIsOkay( Node* pNode )
{
    assert( pNode != NULL );
    assert( pNode->something_irrelevant_to_node_type );
    assert( pNode->type == NODE_TYPE_XYZ );
    assert( pNode->one_thing_relevant_to_node_type );
    assert( pNode->another_thing_relevant_to_node_type );
    ...
    return true;
}

Which is invoked as follows:

assert( assertNodeOfTypeXYZIsOkay( pNode ) );

This allows me to get many assertions out of the way, while at the same time enjoying the benefit of conditional compilation: if assertions are disabled, assertNodeIsOkay() will not only be empty, but it will not even be invoked.

If you have more tips on using assertions and similar tests in code please mention it in your answer.

Having more assertions than actual code is kind of rare, but it is not unheard of. It really depends on what you are doing, and it appears that your particular kind of project calls for this situation to happen. That's fine.

Once your compiler matures and becomes stable, it will stop failing every once in a while due to oddly formulated input text. When that happens, you will be able to have a release build of your compiler on which assertions are disabled, for maximum performance. With that in mind, you should be applying assertions as liberally as you can, because they practically cost nothing.

Assertions are an excellent documentation tool, since a) comments are not enforceable by the compiler, and b) no comment ever was, or will ever be, as unambiguous as a piece of code stipulating the same thing.

Assertions reduce the complexity of your code, in contrast to virtually any other statement that you can use, which in fact increases the complexity of your code.

Assertions help build systems that, in the event of an error, can tell you what is wrong with themselves. That, right there, is priceless.

Assertions are the best thing since sliced bread.

The question you should always be asking is not "should I assert this?" but "is there anything I forgot to assert?"

  • 1
    For a lengthier discussion of some of the points that I made above, see this blog post of mine: michael.gr - Assertions and Testing – Mike Nakis Nov 29 '15 at 0:08
  • 1
    Very good tip refactoring the assertion code into seperate functions. I did that and now everything is so much cleaner. I have no idea why I did not think of that before. The intention of assert(is_valid_declaration(node)) is also so much more clearer than the gibberish I had before! – wefwefa3 Nov 29 '15 at 18:16
  • The idea behind calling these functions assert_... instead of is_valid_... is to make it evident that they are not meant to be invoked as part of regular user input validation. Because if you mistakenly invoke one of these functions for validation, then on the debug build you will get an assertion failure, (that's not what you want to happen as part of validation,) while on the release build everything will pass, no matter how invalid. – Mike Nakis Nov 29 '15 at 20:12
  • In some cases you may have both assert_xyz() and is_valid_xyz() side by side, and assert_xyz() may be implemented as assert( is_valid_xyz() ); but you need to be careful not to confuse the two. Thus, the distinct naming convention. – Mike Nakis Nov 29 '15 at 20:14

Not the answer you're looking for? Browse other questions tagged or ask your own question.