edubart / nelua-lang

Minimal, efficient, statically-typed and meta-programmable systems programming language heavily inspired by Lua, which compiles to C and native code.
https://nelua.io
MIT License
1.99k stars 64 forks source link

Defer fails to modify return values when a function has multiple return values #233

Closed jrfondren closed 10 months ago

jrfondren commented 10 months ago

Bug description

I knew that defer in Odin was defined to not be able to do this: deferred code sees variables only after they've already been copied for the return, so I tested this in Nelua and saw that it seemed to modify returned variables just fine. This resulted in a strange infinite loop that went away when I manually copied the deferred lines to where they should run.

Code example

local function f(n: integer)
  defer n = n + 1 end
  return n
end

local function g(n: integer)
  defer n = n + 1 end
  return n, true
end

print(f(5))  -- 6
print(g(5))  -- 5 true

Multiple-return case:

  _mulret_1.r1 = n;
  _mulret_1.r2 = true;
  { /* defer */
    n = (n + 1);
  }
  return _mulret_1;

Expected behavior

For the deferred lines to be invoked as if they'd been copied and pasted prior to all exits from the scope.

Environment

x86_64 linux Nelua 0.2.0-dev Build number: 1588 Git date: 2023-09-16 16:20:44 -0300 Git hash: 596fcca5c77932da8a07c249de59a9dff3099495 Semantic version: 0.2.0-dev.1588+596fcca5 Copyright (C) 2019-2022 Eduardo Bart (https://nelua.io/)

stefanos82 commented 10 months ago

I don't know whether you have seen it or not, but I have just found it and it's kind of baffling:

int nelua_main(int argc, char** argv) {
  {
    nelua_print_1(tmp_f(5));
    {
      nlmulret_nlint64_nlboolean _tmp1 = tmp_g(5);
      nelua_print_2(_tmp1.r1, _tmp1.r2);
    }
  }
  return 0;
}

Should not this code be

int nelua_main(int argc, char** argv) {
  {
    nelua_print_1(tmp_f(5));
  }
    {
      nlmulret_nlint64_nlboolean _tmp1 = tmp_g(5);
      nelua_print_2(_tmp1.r1, _tmp1.r2);
    }
  return 0;
}

because print(f(5)) and print(g(5)) are executed as two separate defers behind the scenes, correct?

jrfondren commented 10 months ago

The braces around nelua_print_2() are probably there to quickly free _tmp1 from occupying stack space - it's about multiple value returns from g() rather than anything getting deferred.

stefanos82 commented 10 months ago

OK, I have found the bug; the generated C code for defer takes place in wrong line(s):

/* Buggy version */
nlmulret_nlint64_nlboolean tmp_g(int64_t n) {
  nlmulret_nlint64_nlboolean _mulret_1;
  _mulret_1.r1 = n;
  _mulret_1.r2 = true;
  { /* defer */
    n = (n + 1);
  }
  return _mulret_1;
}

It should be

/* Fixed version*/
nlmulret_nlint64_nlboolean tmp_g(int64_t n) {
  nlmulret_nlint64_nlboolean _mulret_1;
  { /* defer */
    n = (n + 1);
  }
  _mulret_1.r1 = n;
  _mulret_1.r2 = true;
  return _mulret_1;
}

Update: After thinking about it, my fix violates the logic of defer mechanism that is supposed to take place as last. I wonder how @edubart could solve this peculiar case...maybe with the use of GOTO labels? :thinking:

Update 2: On second thought, the code below should work too:

nlmulret_nlint64_nlboolean tmp_g(int64_t n) {
  nlmulret_nlint64_nlboolean _mulret_1;
  { /* defer */
    n = (n + 1);
  }
  return _mulret_1  = (nlmulret_nlint64_nlboolean){.r1=n, .r2=true};
}

Since we are deferring n only and then return n, true, a compound literal should work as shown above.

edubart commented 10 months ago

This is a serious bug, I try to fix it next weekend. Thanks for reporting!

stefanos82 commented 10 months ago

@edubart keep up the good work brother :+1:

@jrfondren Good job my friend with your thorough investigation; that makes us two now! :smiley:

edubart commented 10 months ago

I fixed the misbehavior in https://github.com/edubart/nelua-lang/commit/dc479bf4b1ad1335d9caa6cdbf282cc5979f06c0, by definition defer should only execute after evaluating all function statements, including return expressions.