- There's a semicolon missing at the end of the assignment this.toString
Fortunately, it makes no difference in this case, but I suppose it would be more consistent to have it after function assignments as well.
- It would be more space efficient to do, after the function definition,
StrConst.prototype.toString = ... (I didn't have that habit when I wrote the parser, but in new code I am now using that style as it can save a lot of space for objects that are frequent).
Valid point.
- It might be very OOish to have separate classes for each type of
constant (null, undefined, boolean, integer, float, string, object) as subclasses to the ConstInstr, but I don't see the point. The code becomes very verbose for no apparent benefit. In the AST there is just one node type for literals with the field "value", and JS introspection can be used on the value when it is important to know the type of value (i.e. typeof x.value == "number"). For example, to test for a constant that is a number equal to 0, it would be possible to do if (x.value === 0) ... instead of if (x instanceof IntConst && x.value == 0) ... Moreover, to test for a number n it would be possible to do if (x.value === n) ... instead of if ((x instanceof IntConst || x instanceof FPConst) && x.value == n) ... If a class hierarchy is used for constants, why stop at those specific types? Why not also have subclasses of "boolean" for "true" and "false", and a subclass of integer for nonnegative integers, and one for 32 bit integers (or fixnums)? Also, to be consistent, the integer and float subclasses should have a "number" superclass. For other types of instructions I agree that a class hierarchy can be beneficial for code clarity and modularity.
The point is simply that different constants *could* someday have their own methods, specific to their type, and we can avoid having to access the value field to know what kind of constant it is. "x instanceof IntConst" vs "typeof x.value == 'number' && NumberIsInt(x.value)" or "x instanceof ConstInstr && x.isIntConst()". The value field can still be accessed directly, since all constants use the same field name. I will consider this refactoring eventually, but for now, I'm just trying things out to see how things should go together.
- What is an "Object" constant? I fail to see when it would be useful.
In JS the notations [ ... ] and { ... } construct *new* objects when they are evaluated, even when all their fields are constant, including when there are no fields or elements.
It's not an object constant, it's an object reference constant, i.e.: a way for us to include references to pre-existing objects in the IR, such as a global object for example, or even a function object we want to make a call to. I did this in the mindset that we could eventually have multiple "global objects" (one for Tachyon, one or more for the code Tachyon compiles/executes). I will probably also include a global object constant, which will eventually get translated to one of these.
- What is a "ConstInstr"? Is it a constant instruction? What does a
constant *instruction* do? Perhaps "ConstExpr", for constant expression, or "ConstOperand" would be a better name?
It's a constant value that other SSA instructions can use as input. These are not meant to be included in the CFG, however. I simply thought it made sense to have all values used in the CFG derive from a base instruction class. It could make sense to have instruction derive from a Value class instead.
- The toString function for strings (which generates escapes) is needed
in several places, including the AST pretty-printer, so this is definitely a function that should be in a global module (such as a "miscellaneous" module misc.js).
If you write such a function, I suggest placing it under utility/strings.js.
I'm happy to see that the code base is growing quickly!
Me too.
I have a feedback regarding your code as well... Please comment more!
- Maxime