._add() meta-method not called for strings

[jayeheffernan]

I've run into an oddity in the way the .add() meta-method works. I have specified it on a class and I was expecting it to be called whenever an instance of that that class is followed by + some_other_value. The weird thing is it is not called when the value being added is a string. It is called for all other types that I could think of. The ._sub() meta-method for subtraction is called for all of the same types as .add() except that it is still called when the right operand is a string.

I've run into this issue in the Electric Imp version of Squirrel, but I've modified the code (slightly) to run in Squirrel 3.1 64 on Linux and I get the same results, so it seems to be a "feature" of the language rather than compiler or implementation specific. I've included code and logs below.

Is this a bug? Is there some rationale behind this exception?

// "polyfill" my logging method
server <- { log=@(str) print(str + "\n") };

class A {
    function _add(thing) {
        server.log("adding " + (typeof thing) + ": " + thing);
        return "added";
    }

    function _sub(thing) {
        server.log("subtracting " + (typeof thing) + ": " + thing);
        return "subtracted";
    }
}

results <- [
    A() + 123,
    A() + 123.4,
    A() + "abc", // this is the odd one out: no meta-method is called
    A() + true,
    A() + null,
    A() + {},
    A() + A,
    A() + [],
    A() + (@() null),

    A() - 123,
    A() - 123.4,
    A() - "abc",
    A() - true,
    A() - null,
    A() - {},
    A() - A,
    A() - [],
    A() - (@() null),
];

server.log("   --- results ---   ");
foreach (r in results) {
    server.log(r);
}

adding integer: 123
adding float: 123.4
adding bool: true
adding null: (null : 0x(nil))
adding table: (table : 0x0x564b1effc3f0)
adding class: (class : 0x0x564b1effafb0)
adding array: (array : 0x0x564b1effbed0)
adding function: (function : 0x0x564b1effc110)
subtracting integer: 123
subtracting float: 123.4
subtracting string: abc
subtracting bool: true
subtracting null: (null : 0x(nil))
subtracting table: (table : 0x0x564b1effc3f0)
subtracting class: (class : 0x0x564b1effafb0)
subtracting array: (array : 0x0x564b1effbed0)
subtracting function: (function : 0x0x564b1effba20)
   --- results ---   
added
added
(instance : 0x0x564b1effb260)abc
added
added
added
added
added
added
subtracted
subtracted
subtracted
subtracted
subtracted
subtracted
subtracted
subtracted
subtracted

[albertodemichelis]

Hi, this is expected behavior, however I agree that this might not be the best one. A better choice would have been to have a dedicated string concatenation operator. So, yeah we have to live with this. Sorry. :P

[mingodad]

Hello Jaye ! Thanks for reporting it ! We do not need to live with this, it's fixed in SquiLu https://github.com/mingodad/squilu/commit/fce59250eb2502821caf58acdb8ea5661a710d72

Cheers !

[jayeheffernan]

Thanks @albertodemichelis I thought this might be the case since it would technically be a break of backwards compatibility to change (although it's very much an edge case).

@mingodad, SquiLu looks to have some helpful features. I wonder if I'm understanding that code snippet right: if you try to + with an instance, but you have not implemented ._add() on the class will this throw an error? The behaviour I was hoping for was that the metamethod is called if it is defined, otherwise string concatenation can go ahead (as if the default implementation of ._add() is to string concatenate). I think string concatenation makes sense as a default, but we should be able to override this ._add().

[mingodad]

Hello Jaye ! You are right with this change https://github.com/mingodad/squilu/commit/fce59250eb2502821caf58acdb8ea5661a710d72 if there is no method _add implemented an error will be raised.

[mingodad]

if we want to add a string to an instance with the operator '+' without defining it in my point of view it's an error. Otherwise we should be using :

instance.tostring() + "another string";

[jayeheffernan]

Thanks for the clarification. I think that behaviour makes a lot of sense as well (there doesn't seem to be any clear answer for what should happen when an instance is added to any given type, so force the user to specify), but as a change for Squirrel it would increase the chances of breaking old code. I'll raise this with Electric Imp in case they're interested in making any change to their version, and point to SquiLu as an example.

[rversteegen]

But concatenating a string to an arbitrary object (which might be an instance) is not at all an edge case, it's a common thing to do, eg in a debugging print statement. I think the desired fix is to swap the order of the two checks in ARITH_OP: first check for a metamethod, then try string concatenation if there isn't one.

[jayeheffernan]

@rversteegen absolutely I agree that changing Squirrel to SquiLu's behaviour in this case would break too much. The change (if any) should be as you say, which could conceivably still break some old code, but only in actual edge cases.

[mingodad]

Hello Ralph, Jaye ! I still do not think that it's a good idea, otherwise doing what you think is a good solution, what do you expect by this:

class A {
  str = null;
}
local a = A();
a += "boom !";
print(a);

[jayeheffernan]

@mingodad I think your behaviour would be just as good or better if starting from scratch. Your solution is "stricter" about throwing exceptions to alert the user of potential errors and force them to address them, like what you get with strongly typed languages, whereas the other solution is more permissive and I think closer to existing behaviour and what you get in Javascript, for example. I think the other solution is a more reasonable/targeted change to make with backwards compatibility in mind. With your example, although it's a little confusing, the behaviour of that snippet would be identical after implementing the change @rversteegen is suggesting to what it is now.

[rversteegen]

I forgot that this only affects the case where the first arg is an instance, and because + is left-associative, if you wrote something like print("Arg 1: " + arg1 + " Arg 2: " + arg2) you would end up with a string regardless of the type of arg1/arg2 or their metamethods. So less common than I thought.

(I see that the Squirrel manual doesn't mention associativity of operators, unfortunately. Are they all left-associative?)

[todace]

I've found that use of + for string concatenation is dangerous

• when used with numerals and strings mixed is not associative operation (a+b)+c != a + (b+c) That means that result depends sometimes on small changes in data like [1,2,"a"].reduce(@(a.b) a+b) == "3a"

I prefer now to avoid it. In Quirrel fork static analyzer https://github.com/GaijinEntertainment/quirrel_static_analyzer (it can check most of old Squirrel files too) we have even warning for string concatenation via +