lua-users home
lua-l archive

[Date Prev][Date Next][Thread Prev][Thread Next] [Date Index] [Thread Index]


Hi,

Luiz Henrique de Figueiredo wrote:
> Please take a look and send us your comments or post them here.

Ok, I've tested it on a dozen OS, compiler and CPU flavours and
so far everything looks good. It compiles without warnings on all
systems. The TKey related changes have silenced GCC 4.1 beta, too.

I've noticed some configuration issues though:

- Solaris works fine with the "posix" target. But if you want
  to enable dynamic loading: it needs "-ldl", but not "-Wl,-E".
  In case you want to support it officially:

solaris:
	$(MAKE) all MYCFLAGS="-DLUA_USE_POSIX -DLUA_USE_DLOPEN" MYLIBS="-ldl"

- Much to my surprise some SLES9 systems (Novell/SuSE Linux)
  didn't have readline installed (uh?). But many (but not all)
  BSD systems I tested did have the readline compatibility
  installed. See the discussion about Mac OS X 10.4, too.
  So, do we need (want?) two targets for each OS?

[I've reported some more Makefile issues to lhf by mail (make
install complains, problem with non-"all" targets and mingw).]

Some other noteworthy things:

- lvm.c:luaV_execute() case OP_FORLOOP:
  'step > 0' should be 'luai_numlt(0, step)'.

- Maybe we need to be able to redefine the casts to (and from!)
  lua_Number in luaconf.h, too? It could be a struct ...

- luaconf.h should have a
    #define LUA_NUMBER_DOUBLE
  near the LUA_NUMBER define to allow checking for the number
  type with CPP. Some modules really need to know this. Patches
  to change the number type should change this one, too.

- I guess the call to buffreplace() in read_numeral should get an
    if (ls->decpoint != '.')
  to avoid copying every number twice in the standard case.
  I'm not sure how happy everyone will be with the new dependency
  on localeconv() (from <locale.h>) in llex.c. Maybe add an
  option in luaconf.h?

- doc/lua.html and luac.html start with a comment line. Better
  move this line inside (e.g. /usr/bin/file detects it as SGML).

Digging in my archives, I've been looking for previously reported
but unfixed issues. There are a few:

- IMHO the problematic syntax "a=1b=2" should be explicitly
  disallowed. This avoids problems with foreign parsers and
  allows for more extensibility (e.g. adding hex numbers in a
  future release would make "y=0x=1" unparseable).

  Add this at the end of llex.c:read_numeral():
    if (isalpha(ls->current) || ls->current == '_')
      luaX_syntaxerror(ls, "ambiguous syntax");

- The new calling convention for luaopen_* functions should be
  listed under "Incompatibilities", too. Every embedder needs to
  know this (it's only mentioned briefly in chapter 5).

- A warning about the side-effects of DirectX/Direct3D is needed
  in the manual (and maybe in INSTALL, too):

    A warning for users of DirectX/Direct3D: You MUST set the
    D3DCREATE_FPU_PRESERVE flag upon initialization when you
    use Lua in the same thread. Otherwise you'll encounter
    strange behaviour -- complain to Microsoft, not to us.

- table.maxn(t) should be documented as 'expensive' and only to
  be used if #t does not work (for arrays with holes or tables
  with non-integer keys).

- The docs still don't pass the HTML validation suites.

- 'make install' still does not create the module directories.
  It's the duty of the core package to create the module
  directories and the duty of the add-on (module) packages to put
  modules into them:

INSTALL_LMOD= $(INSTALL_TOP)/share/lua/5.1
INSTALL_CMOD= $(INSTALL_TOP)/lib/lua/5.1

install: all
	... mkdir -p ... $(INSTALL_LMOD) $(INSTALL_CMOD)

- etc/strict.lua should be installed, too.

- etc/lua.pc should include variables for the standard/preferred
  module directories to install modules.

Bye,
     Mike