Extending a class doesn't seem to inherit members

[mingodad]

Class inheritance doesn't seem to work properly see example and output bellow:

class Obj {
    name = null;           // name of this object
    constructor(s) {
        name = s;
        print("Obj\t" + s + "\t" + this.name + "\n");
    }
}

class Proc extends Obj {

    constructor(name) {
        base(name);
        print("Proc\t" + name + "\t" + this.name + "\n");
    }
}

local v = Proc("dad");
print(v.name + "\n");

Output:

sq test.nut 
Obj dad dad
Proc    dad null  //!!!<< expect 'dad'
null                 //!!!<< expect 'dad'

[mingodad]

This variation shows the same but can give more insights on the problem:

class Obj {
    xname = "notyet";           // name of this object
    constructor(s) {
        this.xname = s;
        //print("Obj\t" + s + "\t" + this.name + "\n");
    }
}

class Proc extends Obj {

    constructor(s) {
        base(s);
        //print("Proc\t" + name + "\t" + this.name + "\n");
    }
}
local s = "dad";
local v = Proc(s);
print(v.xname + "\n");
print(s + "\n");
foreach( k,v in Proc) print(k + ": " + v + "\n");
foreach( k,v in Proc.getbase()) print(k + ": " + v + "\n");

Output:

./sq test.nut 
notyet
dad
xname: notyet
constructor: (function : 0x0x55f7678c02c0)
xname: notyet
constructor: (function : 0x0x55f7678c3980)

[mingodad]

Looking at SQClass constructor we can see that it's cloning the inherited members and doesn't seem to have any link to remember/update changes made in the base class.

SQClass::SQClass(SQSharedState *ss,SQClass *base)
{
    _base = base;
    _typetag = 0;
    _hook = NULL;
    _udsize = 0;
    _locked = false;
    _constructoridx = -1;
    if(_base) {
        _constructoridx = _base->_constructoridx;
        _udsize = _base->_udsize;
        _defaultvalues.copy(base->_defaultvalues);
        _methods.copy(base->_methods);
        _COPY_VECTOR(_metamethods,base->_metamethods,MT_LAST);
        __ObjAddRef(_base);
    }
    _members = base?base->_members->Clone() : SQTable::Create(ss,0); ///!!!<< Cloning here
    __ObjAddRef(_members);

    INIT_CHAIN();
    ADD_TO_CHAIN(&_sharedstate->_gc_chain, this);
}

[mingodad]

After reading again the Squirrel documentation I found that I should be calling base.constructor() instead of base(), somehow would be nice if a warning/error could be issued by the compiler to prevent wasting time with mistakes like mine here.

[mingodad]

This is what I'll implement in https://github.com/mingodad/squilu to prevent this issue again:

--------------------------- squirrel/sqcompiler.cpp ---------------------------
index 095edd7..cc2f0ba 100644
@@ -81,6 +81,7 @@ public:
         _scope.outers = 0;
         _scope.stacksize = 0;
         _compilererror[0] = _SC('\0');
+        _inside_constructor = false;
     }
     static void ThrowError(void *ud, const SQChar *s) {
         SQCompiler *c = (SQCompiler *)ud;
@@ -722,6 +723,8 @@ public:
             break;
         case TK_BASE:
             Lex();
+            if(_token == '(' && _inside_constructor)
+                Error(_SC("calling 'base' as a function inside a constructor"));
             _fs->AddInstruction(_OP_GETBASE, _fs->PushTarget());
             _es.etype  = BASE;
             _es.epos   = _fs->TopTarget();
@@ -986,6 +989,7 @@ public:
             switch(_token) {
             case TK_FUNCTION:
             case TK_CONSTRUCTOR:{
+                if(_token == TK_CONSTRUCTOR) _inside_constructor = true;
                 SQInteger tk = _token;
                 Lex();
                 SQObject id = tk == TK_FUNCTION ? Expect(TK_IDENTIFIER) : _fs->CreateString(_SC("constructor"));
@@ -993,6 +997,7 @@ public:
                 _fs->AddInstruction(_OP_LOAD, _fs->PushTarget(), _fs->GetConstant(id));
                 CreateFunction(id);
                 _fs->AddInstruction(_OP_CLOSURE, _fs->PushTarget(), _fs->_functions.size() - 1, 0);
+                _inside_constructor = false;
                                 }
                                 break;
             case _SC('['):
@@ -1593,6 +1598,7 @@ private:
     SQLexer _lex;
     bool _lineinfo;
     bool _raiseerror;
+    bool _inside_constructor;
     SQInteger _debugline;
     SQInteger _debugop;
     SQExpState   _es;

Now if we try run then we'll get this error message:

sq test2.nut 
test2.nut line = (11) column = (8) : error calling 'base' as a function inside a constructor
Error [calling 'base' as a function inside a constructor]

[mingodad]

Because I've implemented warnings in SquiLu compiler I'll emit a warning instead of an error because can be real need to call base() as a constructor in some special cases.

See here the commit https://github.com/mingodad/squilu/commit/85892308ef027b2796bf3100597ddb4e5f280f87

[rversteegen]

I would suggest changing the warning message to "calling 'base' as a function; did you mean base.constructor()?" to make it even clearer. Ideally the warning wouldn't be printed if you assign the result of the base() call to something, and in general you want a way to avoid causing warning messages.

I hope that Squirrel and Quirrel would be willing to add compiler warnings too if someone were to submit a PR to backport them from SquiLu.

[mingodad]

Thank you for the suggestion of a better warning message I changed it here https://github.com/mingodad/squilu/commit/01ca8d89f3c707d8fa7f5b4ef1ccbb61dee4cae1 , the compiler 'warning' implementation in SquiLu are not bigger or complicated and anyone with familiarity with C++ can port it.

[mingodad]

Or even instead of calling base.constructor() as of now probably change it by super() and leave base for the special cases.

[RizzoRat]

i think its perfectly fine as is. As documented, the keyword base is the base CLASS (=class definition), not an INSTANCE. Calling base() hence perfectly does what it is expected to - creating a new instance first and then calling its constructor. I wouldn't recommend throwing an error, because I may WANT to create another instance of the base class.

[mingodad]

Thank you for your feedback ! Yes I noticed this case and that's I also suggested add the keyword super that will only call the base constructor.