Are "Number" and "Variable" valid constructs?

[Carotti]

In ast.hpp there are the following functions

inline TreePtr Number(int32_t x)
{ return std::make_shared<Tree>("Number", std::to_string(x)); }

inline TreePtr Variable(std::string id)
{ return std::make_shared<Tree>("Variable", id); }

If you construct ast nodes using these functions, using something like...

int main()
{
    TreePtr var = Variable("x");
    PrettyPrint(std::cerr, var);

    TreePtr num = Number(5);
    PrettyPrint(std::cerr, num);
}

The result is

Variable : x
Number : 5

So, do we need to support the Variable and Number constructs in our interpreter and compiler? Or can we just stick to recognizing their "direct" use when they are in the type?

[maccth]

Furthermore, on line 70 from ast.hpp, IfElse is used as the type but it is an If construct. Is this a mistake? If not, what is the reasoning behind this?

(What is your confidence in your answer? Please note a confidence level of high will result in a loss of 2 SOLE points if the answer is incorrect.)

[m8pple]

Let's pretend this is a teaching moment, as it is exactly the problem I mentioned for for implicit weakly typed ASTs versus explicit strongly typed trees. Because there is no compile-time checking of anything, we have code lingering in the code-base which is valid code at compile-time, but invalid at run-time. It looks like those functions are not actually used from anywhere in the code, as the only way that ASTs are constructed is via the parser.

So what you can see is the vestigial remains of an older (more complex) AST+lab which has managed to hang around because it didn't cause a compile-time error when I simplified+changed the spec.

As a result you don't need to recognise the out-of-spec constructs as valid.

Fixing it presents something of a quandary, as their existence implicitly suggests that they are usable or useful, but at the same time anyone currently relying on Number or Variable will be building an incorrect tree. At the same time, anyone who is doing proper testing would find that the resulting AST is incorrect...

Grepping through everyone's committed master branch:

• No-one is using Variable
• No-one is using Number
• No-one is using IfElse (though one person supports it in their interpreter).

Can't really tell what people are doing on local repos, but it is at least encouraging.

So the fix is to update those three functions to bring them in line with the spec, and I'll have to explicitly notify everyone as it is a potential breaking-change for some people.

"confidence" : "Medium"