Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #1801 and Fixes #1784. #1811

Merged

Commits on Sep 9, 2016

  1. Fixes KSP-KOS#1801 and Fixes KSP-KOS#1784.

    The problem was that the logic in the Compiler's IterateuserFunctions() that does a tree walk through everywhere a ``function`` statement might exist to preform function preprocessing on it, was failing to descend into the ``instruction_block`` of an anon function.
    
    This was because the ``instruction_block`` of an anon function is a kind of ``expr`` instead of being a kind of ``instruction``, and the case statement wasn't set up to descend into ``expr``'s.
    Dunbaratu committed Sep 9, 2016
    Configuration menu
    Copy the full SHA
    7b89977 View commit details
    Browse the repository at this point in the history

Commits on Sep 21, 2016

  1. Fix in response to @hvacengi's review comment.

    Why it wasn't noticed before:
    -----------------------------
    
    The problem only happened when you use a SET command to
    assign an anonymous function to a variable, as in
    
    SET anon TO {lock foo to 1.}.
    
    But if you happened to use a DECLARE command of some sort to do so,
    like a LOCAL... or GLOBAL, like so:
    
    LOCAL anon IS {lock foo to 1.}.
    
    then it didn't exhibit the problem.  This is why I didn't
    notice it before.  It turns out all my tests were doing it
    that way.
    
    The Cause:
    ----------
    
    The problem had been caused by the fact that when
    Compiler.IterateUserFunctions() walked the parse tree to try to
    find all the user functions and lock statements, it failed
    to recurse into the anon functions in some cases and thus didn't
    find the locks that were inside them.  (It only pre-processes the
    lock statements when this walk finds them).
    
    It failed to recurse into some of the kinds of parse tree nodes
    which could contain anon functions because it only recursed into
    node types that were explicitly mentioned in a case statement list,
    and it assumed anything not on the list cannot contain further
    statements nested inside it.  This list was not entirely up to
    date and correct, missing a few types of statement that were now
    capable of containing further nested statements.  Now that anon
    functions exist, almost every kind of expression could be an anon
    function with statements nested inside it.
    
    The Fix:
    --------
    
    Given that two different recent issues both were caused by failures
    of this walk to recurse into some kinds of parse node, and some
    other issues in the past were also caused by the very same
    section of code failing to mention a new part of speech after
    the grammar file changed,  I decided it was high time I flipped
    the logic around. Now it operates on a blacklist instead of a
    whitelist.  Now it automatically descends into every kind of
    node *except* the ones explicitly mentioned that we know can't
    contain any further open-ended expressions inside them.  This
    way if we forget to mention a node type on that list, the failure
    results in merely a bit of wasted execution time for failing
    to prune the tree walk early enough, rather than a fatal error
    for forgetting to mention a node like we had before.
    Dunbaratu committed Sep 21, 2016
    Configuration menu
    Copy the full SHA
    81de368 View commit details
    Browse the repository at this point in the history