Nasal Maintenance

From FlightGear wiki
Jump to navigation Jump to search

Background

There's a backlog of more or less long-standing Nasal-related issues, some of which are also explicitly mentioned by various FIXME and TODO annotations to be found in the source code. This is a list of known issues, to help getting rid of them eventually. This article is strictly about C/C++ code changes, and about maintaining/cleaning up existing code, rather than implementing new features, for the latter, see Improving Nasal (which is currently in the process of being cleaned up).

Misc

  • Get rid of C++ keywords in the C code, so that the Nasal code base can be optionally compiled by a C++ compiler, for better error-checking and overall better tool support
  • The naContext supports a user data pointer that can be used to store data specific to an naCall invocation without exposing it to Nasal as a ghost. FIXME: this API is semi-dangerous, there is no provision for sharing it, nor for validating the source or type of the pointer returned. [1]
  • improved typeof() behavior: "having typeof return 'number' or 'string' – which would better represent what is happening in the source code. I find it hard to think of reasons against this change other than backwards compatibility and 'cause Andy made it that way – the former of which is easily fixed with a regex and some human intervention, the latter with a stare." [2]

Lexer

JSON compliance

  • FIXME: need to handle \b (8), \f (12), and \uXXXX for JSON compliance [3]Not done Not done

Parser

  • ticket #737: fix default argument handling for functions Done Done by Philosopher [4]
  • ticket #585: fix multi-assignment expressions Done Done by Philosopher [5]
  • fix token precedence rule Done Done by Philosopher [6]

Codegen

  • ticket #587: fix break/continue segfaults outside loops Done Done by Philosopher [7]
  • FIXME: this API is fundamentally a resource leak, because symbols can't be deregistered. The "proper" way to do this would be to keep a reference count for each symbol, and decrement it when a code object referencing it is deleted. [8] Not done Not done

Virtual Machine

  • FIXME: unify with almost identical checkVec() [9] Not done Not done

Standard Library

  • FIXME: need a place to save the current IP when we get an error so that it can be reset if we get a die()/naRethrowError() situation later. Right now, the IP on the stack trace is the line of the die() call, when it should be this one... [10] Not done Not done
  • FIXME: don't use naCall at all here, we don't need it. Fix up the context stack to tail call the function directly. There's no need for f_call() to live on the C stack at all. [11] Not done Not done
    • We actually do need it for error handling, as the workaround would need to introduce a jmp_buf into each stack frame. However, it is simple enough to use naCall for error-handling and tail-call otherwise. The code for setting up the frames should also be unified into a single function (naCall and setupFuncall currently duplicate code, and so would f_call). —P

Garbage Collector

Cquote1.png allocating objects into generations and only mark/reaping from the most recent one on most iterations is a straightforward optimization and will definitely make things faster.
— (Andy Ross) FlightGear Devel mailing list[1]
Cquote2.png
  • We need to investigate abstracting the GC interface, so that it will be easier to experiment with alternate schemes, without them touching tons of places in Nasal Not done Not done
  1. Andy Ross (22 May 2012). Nasal performance.

FlightGear Integration Layer

Line numbers in embedded Nasal code don't show absolute line numbers

Most XML files that support embedded Nasal code show relative line numbers once errors/warnings are shown - but they should ideally show the line number using the offset in the XML file, and not just the line number of the embedded Nasal section - i.e. add some userData field that computes the proper XML file offset.

Cleaning up FGNasalSys::init()

If you're looking for a structured environment here, what about a cppbind port of SGSubsystem for Nasal? I.e. with .init, .postinit, .reinit, .update, etc.? That would be structured It might also help with Nasal loading as an alternative to require() --

We once discussed the idea a long time ago, and it would actually be great if all Nasal code were using such an established and structured interface. It would be much easier to support simulator resets/re-initialization that way (see Reset & re-init), if all scripts (core sim, GUI, aircraft) would follow a standard interface.

But first of all, one would need to provide such a wrapper, and then document it extensively and illustrate its benefits - i.e. in some of the default aircraft (c172p), so that developers are motivated to actually do all the porting work.

Overall, it would definitely align well with the Reset & re-init work done by ThorstenB & James, because they're trying to solve the problem in a brute-force fashion by simply shutting down everything and starting over. Personally, I have come to the conclusion that NasalSys::init() is a huge troublemaker actually - it would be so much more simpler/elegant if that stuff was done in Nasal directly.

It would be also useful to add support for additional Nasal dirs - and a well-defined search order.

NasalSys::init() should really only handle the necessary C++/internal side (i.e. stuff like cppbind/NasalPositioned initialization) and just load a single script with will in turn load the others through Andy's original import() magic. It might also help by keeping everything under a single naContext, but I dunno if that is good or bad (I would suspect good).

And I am actually thinking of having more than just $FG_ROOT/Nasal - i.e. different folders for different modules, not just sub modules - but hierarchies that handle dependencies. For example, props.nas would be one of the first modules to be loaded, while gui.nas would only be available in a "GUI" run-level, i.e. after the sim is up and running. The current init code is incredibly messy and causing trouble. For example, there used to be the FlightGear Run Levels discussion back on the devel list, that is also about the same issue - Nasal dependencies.

In the FGRadar/Nasal code, Icecode_GL actually introduced another folder for unit testing purposes, that would be loaded/executed prior to anything else, where you would then simply add unit tests - which would all need to succeed first.

These things would be all good to have, but they don't need to be implemented in C++ space. That doesn't mean that mapping SGSubsystem to Nasal space would not be useful - I actually like the idea a lot, because we definitely have a bunch of REAL subsystems implemented in Nasal, such as bombable or advanced weather - these would ideally use a real SGSubsystem interface, possibly even SGSubsystemGroups (basically vectors of related subsystems) to ensure that the design supports reset/reinit and other important stuff without shutting down the whole beast.

The current way of developing Nasal code and just dropping it into $FG_ROOT/Nasal or $FG_AIRCRAFT simply isn't overly scalable, while also being really fragile and difficult to provide opportunities for code reuse/sharing.

Also see: http://forum.flightgear.org/viewtopic.php?f=30&t=19487