What's wrong with this Lua code?

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

What's wrong with this Lua code?

Soni "They/Them" L.
I have this Lua code:

local string = "" print(string.format("%X", 16777216))

And it works perfectly, but I was told it's wrong? What's wrong with it?
Why shouldn't I do this?

--
Disclaimer: these emails may be made public at any given time, with or without reason. If you don't agree with this, DO NOT REPLY.


Reply | Threaded
Open this post in threaded view
|

Re: What's wrong with this Lua code?

Jonathan Goble
On Fri, Jun 16, 2017 at 3:45 PM Soni L. <[hidden email]> wrote:
I have this Lua code:

local string = "" print(string.format("%X", 16777216))

And it works perfectly, but I was told it's wrong? What's wrong with it?
Why shouldn't I do this?

Let's break it down:

> local string = ""

The variable `string` is now an empty string and, in this scope, no longer references the `string` standard library.

> print(string.format("%X", 16777216))

Here, Lua first looks up `string.format`. While `string` no longer refers to the string standard library, it is an actual string, and strings (since Lua 5.1) have a metatable with an __index metamethod pointing to the actual string library (which isn't changed by your local `string` assignment, nor would an assignment to the global `string` affect it). So the lookup of `string.format` produces, via metamethod, the string standard library function `format`. From there, you can call it with whatever arguments you want.

So why is it a bad idea? One, because indexing through a metamethod is slower than directly indexing the actual `string` table, and two, because shadowing a built-in global (whether with a global or local assignment) is usually a code smell (in any language) and generally shouldn't be done without a very good reason.
Reply | Threaded
Open this post in threaded view
|

Re: What's wrong with this Lua code?

William Ahern
On Fri, Jun 16, 2017 at 08:18:10PM +0000, Jonathan Goble wrote:
<snip>

> Let's break it down:
>
> > local string = ""
>
> The variable `string` is now an empty string and, in this scope, no longer
> references the `string` standard library.
>
> > print(string.format("%X", 16777216))
>
> Here, Lua first looks up `string.format`. While `string` no longer refers
> to the string standard library, it is an actual string, and strings (since
> Lua 5.1) have a metatable with an __index metamethod pointing to the actual
> string library (which isn't changed by your local `string` assignment, nor
> would an assignment to the global `string` affect it). So the lookup of
> `string.format` produces, via metamethod, the string standard library
> function `format`. From there, you can call it with whatever arguments you
> want.
>
> So why is it a bad idea? One, because indexing through a metamethod is
> slower than directly indexing the actual `string` table, and two, because
> shadowing a built-in global (whether with a global or local assignment) is
> usually a code smell (in any language) and generally shouldn't be done
> without a very good reason.

OTOH, using the string global is actually more like _G.string.find,
requiring two VM index operations. Whereas an index through a local (or
immediate) value is a single VM index operation; the loading and indexing of
the metatable is direcly implemented as compiled C code that doesn't require
stepping through the VM interpreter and so theoretically should be much
faster.

... and my hypothesis checks out:

  $ /tmp/bench.lua
  256           subject:find    0.0001s
  256           string.find     0.0001s
  4096          subject:find    0.0010s
  4096          string.find     0.0013s
  65536         subject:find    0.0109s
  65536         string.find     0.0114s
  1048576       subject:find    0.1455s
  1048576       string.find     0.1827s
  16777216      subject:find    2.2815s
  16777216      string.find     2.8463s
  268435456     subject:find    36.6191s
  268435456     string.find     45.4249s

Code:

  local unix = require"unix"
  local CLOCK_MONOTONIC = unix.CLOCK_MONOTONIC
  local clock_gettime = unix.clock_gettime
 
  for e=8,28,4 do
  local n = math.pow(2, e)
  do
  local start = clock_gettime(CLOCK_MONOTONIC)
  local subject = "foo"
  for i=1,n do
  subject:find("foo")
  end
  local elapsed = clock_gettime(CLOCK_MONOTONIC) - start
  print(string.format("%-8d", n), "subject:find", string.format("%.4fs", elapsed))
  end
 
  do
  local start = clock_gettime(CLOCK_MONOTONIC)
  local subject = "foo"
  for i=1,n do
  string.find(subject, "foo")
  end
  local elapsed = clock_gettime(CLOCK_MONOTONIC) - start
  print(string.format("%-8d", n), "string.find", string.format("%.4fs", elapsed))
  end
  end

Reply | Threaded
Open this post in threaded view
|

Re: What's wrong with this Lua code?

Jonathan Goble
On Fri, Jun 16, 2017 at 5:07 PM William Ahern <[hidden email]> wrote:
On Fri, Jun 16, 2017 at 08:18:10PM +0000, Jonathan Goble wrote:
<snip>
> Let's break it down:
>
> > local string = ""
>
> The variable `string` is now an empty string and, in this scope, no longer
> references the `string` standard library.
>
> > print(string.format("%X", 16777216))
>
> Here, Lua first looks up `string.format`. While `string` no longer refers
> to the string standard library, it is an actual string, and strings (since
> Lua 5.1) have a metatable with an __index metamethod pointing to the actual
> string library (which isn't changed by your local `string` assignment, nor
> would an assignment to the global `string` affect it). So the lookup of
> `string.format` produces, via metamethod, the string standard library
> function `format`. From there, you can call it with whatever arguments you
> want.
>
> So why is it a bad idea? One, because indexing through a metamethod is
> slower than directly indexing the actual `string` table, and two, because
> shadowing a built-in global (whether with a global or local assignment) is
> usually a code smell (in any language) and generally shouldn't be done
> without a very good reason.

OTOH, using the string global is actually more like _G.string.find,
requiring two VM index operations. Whereas an index through a local (or
immediate) value is a single VM index operation; the loading and indexing of
the metatable is direcly implemented as compiled C code that doesn't require
stepping through the VM interpreter and so theoretically should be much
faster.

... and my hypothesis checks out:

Interesting! So my first point is invalid, but the second point (about shadowing built-ins being a generally bad idea) still stands.
Reply | Threaded
Open this post in threaded view
|

Re: What's wrong with this Lua code?

Sean Conner
In reply to this post by Jonathan Goble
It was thus said that the Great Jonathan Goble once stated:

> On Fri, Jun 16, 2017 at 3:45 PM Soni L. <[hidden email]> wrote:
>
> > I have this Lua code:
> >
> > local string = "" print(string.format("%X", 16777216))
> >
> > And it works perfectly, but I was told it's wrong? What's wrong with it?
> > Why shouldn't I do this?
> >
>
> Let's break it down:
>
> > local string = ""
>
> The variable `string` is now an empty string and, in this scope, no longer
> references the `string` standard library.
>
> > print(string.format("%X", 16777216))
>
> Here, Lua first looks up `string.format`. While `string` no longer refers
> to the string standard library, it is an actual string, and strings (since
> Lua 5.1) have a metatable with an __index metamethod pointing to the actual
> string library (which isn't changed by your local `string` assignment, nor
> would an assignment to the global `string` affect it). So the lookup of
> `string.format` produces, via metamethod, the string standard library
> function `format`. From there, you can call it with whatever arguments you
> want.
>
> So why is it a bad idea? One, because indexing through a metamethod is
> slower than directly indexing the actual `string` table, and two, because
> shadowing a built-in global (whether with a global or local assignment) is
> usually a code smell (in any language) and generally shouldn't be done
> without a very good reason.

  You have to shadow the built-in globals for modules written in Lua 5.1
because the global environment goes away when you call module().  I still
do that in Lua 5.3 because I attempt to maintain compatability with Lua 5.1:

        local _VERSION     = _VERSION
        local math         = require "math"
        local string       = require "string"
        local io           = require "io"
        local pairs        = pairs
        local tostring     = tostring
        local type         = type
        local print        = print
        local pcall        = pcall
        local getmetatable = getmetatable
       
        if _VERSION == "Lua 5.1" then
          module("org.conman.table")
        else
          _ENV = {} -- luacheck: ignore
        end

        function dump(name,value)
         -- ...
        end

        if _VERSION >= "Lua 5.2" then
          return _ENV -- luacheck: ignore
        end

  Yes, I require() the base modules because that signals intent.  If I could
require() global functions like type() or pcall(), I would do that as well
(if I could do that, a side effect is that the only global in scope would be
require()).

  -spc

Reply | Threaded
Open this post in threaded view
|

Re: What's wrong with this Lua code?

Jonathan Goble
On Fri, Jun 16, 2017 at 6:21 PM Sean Conner <[hidden email]> wrote:
It was thus said that the Great Jonathan Goble once stated:
> So why is it a bad idea? One, because indexing through a metamethod is
> slower than directly indexing the actual `string` table, and two, because
> shadowing a built-in global (whether with a global or local assignment) is
> usually a code smell (in any language) and generally shouldn't be done
> without a very good reason.

  You have to shadow the built-in globals for modules written in Lua 5.1
because the global environment goes away when you call module().  I still
do that in Lua 5.3 because I attempt to maintain compatability with Lua 5.1:

        local _VERSION     = _VERSION
        local math         = require "math"
        local string       = require "string"
        local io           = require "io"
        local pairs        = pairs
        local tostring     = tostring
        local type         = type
        local print        = print
        local pcall        = pcall
        local getmetatable = getmetatable

        if _VERSION == "Lua 5.1" then
          module("org.conman.table")
        else
          _ENV = {} -- luacheck: ignore
        end

        function dump(name,value)
         -- ...
        end

        if _VERSION >= "Lua 5.2" then
          return _ENV -- luacheck: ignore
        end

  Yes, I require() the base modules because that signals intent.  If I could
require() global functions like type() or pcall(), I would do that as well
(if I could do that, a side effect is that the only global in scope would be
require()).

  -spc

As I said, "generally shouldn't be done ***without a very good reason***." This is one example of such a very good reason.
Reply | Threaded
Open this post in threaded view
|

Re: What's wrong with this Lua code?

Dirk Laurie-2
In reply to this post by Soni "They/Them" L.
2017-06-16 21:45 GMT+02:00 Soni L. <[hidden email]>:
> I have this Lua code:
>
> local string = "" print(string.format("%X", 16777216))
>
> And it works perfectly, but I was told it's wrong? What's wrong with it? Why
> shouldn't I do this?

You have had two thorough replies from people I respect a lot,
but an overriding problem with cute code like that is that it redefines
a global to something that is not backward compatible.

Before you did that, one could add useful PERLy methods to 'string'.
Several articles on the Wiki, e.g. StringTrim. After you did that, you
must add them to 'getmetatable"".__index'.

I.e. your code is not wrong, but the trick should only be used on
purpose to discourage custom extensions to the string library.

Reply | Threaded
Open this post in threaded view
|

Re: What's wrong with this Lua code?

Luiz Henrique de Figueiredo
In reply to this post by Sean Conner
> If I could require() global functions like type() or pcall(), I would
> do that as well

linit.c contains:
static const luaL_Reg loadedlibs[] = {
  {"_G", luaopen_base},
  ...

So, you can require"_G", but that is hardly clear.