Use from library

[daurnimator]

It'd be nice to use compat53 without having global side effects.
e.g. local xpcall = require "compat53.base".xpcall

The top level "compat53" could then import into the global scope.

[siffiejoe]

I have implemented what you ask for in the "isolated" branch. However, I'm not convinced that this is necessary (given that the newer functions are pretty much backwards compatible) or that the result is particularly robust (e.g. the compatibilty functions for debug.traceback, xpcall, and coroutine.running are interdependent -- this could already bite us now if someone stores the old functions in upvalues). Also some features only work when applied to the global environment (e.g. string methods, file methods, and the package.searchers alias).

I have also added another shortcut module ("compat53.module") which replaces the environment of the calling chunk, so you don't modify the environment of other files but you can use the new functions without storing them in locals one by one. The same gotchas as above apply though (no global features, unexpected results when mixing functions from different versions).

I want to hear some more opinions about this (e.g. @hishamhm), and some more testing wouldn't hurt either, before I consider merging this into master ...

[daurnimator]

The idea behind 'base' was it would be the things from the base library. Then compat53.math would be the math things, etc.

Regarding shared metatables (e.g. string), it's easy enough to write code in a non-method fashion if you know you need 5.3 only features.

The xpcall+coroutine.status is a tricky one though...

[siffiejoe]

I guess we have to untangle the xpcall situation first -- if we pursue this approach (since using a single file module also has its charm). Other than that I see no big problems (and it probably helps maintainability).

[daurnimator]

we have to untangle the xpcall situation

Is it a one way dependency?
If so we can at least get it half done....

[siffiejoe]

I took a closer look. There is shared state (pcall_previous, pcall_callOf, xpcall_running, pcall_mainOf) between the following public functions:

• debug.traceback (r)
• coroutine.running (r)
• pcall (rw)
• xpcall (rw)

We could probably move the shared state to some helper module -- I don't want to redo the LuaJIT detection in every submodule anyway ...

[hishamhm]

I want to hear some more opinions about this (e.g. @hishamhm), and some more testing wouldn't hurt either, before I consider merging this into master ...

I don't like the concept of compat53.base. The purpose of compat-5.x is to encourage developers to write code targeting the latest Lua version even if they're tied to an older Lua VM for practical reasons, not to help them stick to writing code to old Lua versions by letting them cherry-pick features from the current version.

OTOH, if I follow the idea of compat53.module correctly, it is to help a developer to distribute modules targeting the 5.3 API, advertise them as Lua 5.1+5.2+5.3 compatible and not affect their users' global environment... is that right? That sounds good to me in principle, but the gotchas are worrying and I'm not sure if it's worth the added complexity.

In short, I prefer if the compat modules adopt an all-or-nothing approach (at the very least at module level) instead of promoting even more API-dialect fragmentation.

[daurnimator]

OTOH, if I follow the idea of compat53.module correctly, it is to help a developer to distribute modules targeting the 5.3 API, advertise them as Lua 5.1+5.2+5.3 compatible and not affect their users' global environment... is that right?

Yeah that's what I wanted: to develop modules against 5.3 and have them work with 5.1 and 5.2. As a good library writer, you want no global side effects of loading the library.

[siffiejoe]

Ok, this is my analysis of the xpcall situation:

• It concerns only PUC-Rio Lua 5.1. (LuaJIT and Lua 5.2+ don't need/use the code in question.)
• The compatibility versions for pcall/xpcall use coroutines under the hood to allow yielding. The shared variables are either used to hide the internal coroutines (pcall_mainOf) or to collect accurate stack traces in case of errors with debug.traceback set as message handler (pcall_callOf, pcall_previous, and xpcall_running).
• Therefore, if you don't use the compat52 pcall or xpcall, you can't yield (which is to be expected), but all other compatibility functions should be able to handle the missing info.
• If you mix old/new pcalls/xpcalls, you could get incorrect/incomplete stack traces by debug.traceback. (I don't think that's very serious, and I probably would have ignored that issue altogether if I had implemented the yieldable pcall/xpcall.)
• If you use the compat52 pcall/xpcall (from within a running coroutine), but the old coroutine.running, you will get the internal coroutine used by the compatibility functions, not the one you'd ordinarily get. I don't know how serious that is, but I would imagine that this could result in subtle bugs in some very rare circumstances.

@renatomaia: Please correct me if I'm wrong.

Anyhow, if I read that correctly we are down to two options:
1.) Leave it as it is (compat53.lua modifies the global environment)
2.) Provide compat53.lua which modifies the global environment and an isolated, module-friendly alternative.
(I.e. the fine-grained option of compat53.string, compat53.table, etc. is off the table.)

[siffiejoe]

Btw., I think most of those shared tables should be weak ...

[hishamhm]

Anyhow, if I read that correctly we are down to two options:
1.) Leave it as it is (compat53.lua modifies the global environment)
2.) Provide compat53.lua which modifies the global environment and an isolated, module-friendly alternative.
(I.e. the fine-grained option of compat53.string, compat53.table, etc. is off the table.)

This is how I read it too.

Since the module-friendly version will have more limitations than the global version anyway (such as no extended string methods), maybe we could avoid the extra gotchas by simply not supporting enhanced pcall/xpcall for PUC-Rio Lua 5.1 in the module-friendly version. This is just an idea I'm throwing, I'm not sure if it's a good one.

[renatomaia]

On 13 Apr 2015, at 14:22, Philipp Janda wrote:

� Therefore, if you don't use the compat52 pcall or xpcall, you can't yield (which is to be expected), but all other compatibility functions should be able to handle the missing info.

There is one problem in my original implementation of this "yieldable pcall" support for Lua 5.1+Compat52, which are the debug functions. I didn't do any wrapping debug API to hide this corotouine-based infrastructure, mainly because it would be very complicated. I have a utility module in pure Lua to transverse all values in a lua_State that relies on the 'debug' module that have different implementations for 5.1 and 5.2 because of the lack of this support. :-(

� If you mix old/new pcalls/xpcalls, you could get incorrect/incomplete stack traces by debug.traceback. (I don't think that's very serious, and I probably would have ignored that issue altogether if I had implemented the yieldable pcall/xpcall.)

My main motivation for this is that I use a lot of modules written in Lua that interact with each other (e.g. through 'require'). Therefore if my applications don't guarantee that the global 'pcall' and other standard libraries are consistent (e.g. by requiring 'compat52' in Lua 5.1) then some modules might mix them and get wrong results because the semantics they rely on are not respected.

� If you use the compat52 pcall/xpcall (from within a running coroutine), but the old coroutine.running, you will get the internal coroutine used by the compatibility functions, not the one you'd ordinarily get. I don't know how serious that is, but I would imagine that this could result in subtle bugs in some very rare circumstances.

Again the motivation was to guarantee the standard semantics of these functions for all modules written in Lua that are written expecting such semantics.

@renatomaia: Please correct me if I'm wrong.

I far as I can recall from what I wrote, you are absolutely right in your analysis.

Anyhow, if I read that correctly we are down to two options:

1.) Leave it as it is (compat53.lua modifies the global environment)

For the implementation of compat53 for Lua 5.1, I think this is useful for the reasons I mentioned earlier. In case of a compat53 for Lua 5.2, this whole "yieldable pcall" implementation over coroutines should just be dropped for good as you suggested.

2.) Provide compat53.lua which modifies the global environment and an isolated, module-friendly alternative.
(I.e. the fine-grained option of compat53.string, compat53.table, etc. is off the table.)

I personally agree with Hisham's concerns about the support with older versions of Lua and the complexity of providing all these features.

Renato Maia

[siffiejoe]

After thorough consideration and the feedback so far I have modified the second option (the first is still to leave everything as it is now (@renatomaia: the coroutine based pcall is only active if running PUC-Rio Lua 5.1, already)):

The compat53.module would not install pcall, xpcall, coroutine.running, and debug.traceback into the custom environment. Instead, those functions are transfered to a separate module compat53.pcall (which is loaded by the main compat53 module automatically of course). If the module author can make sure that all functions pcalled by him/her don't reference the old coroutine.running, he/she can load the yieldable pcall support explicitly. I would also rename compat53.base to something that indicates that it's supposed to be a private module (like private, impl, internal, or core, although I figure that one's for C modules -- other suggestions?). What I still don't like about compat53.module is that it uses debug functions, and I consider it "impolite" for normal modules to rely on the debug module (not much better than monkey patching in fact, but you could at least choose the lesser of the two evils) ...

Further opinions/feedback welcome. If nothing new comes up, I'll decide in a few days.

Anyhow, I shall add a wiki page for coroutine.running to describe the problem (which could occur if someone saves the old version in an upvalue before compat53 is required).

[siffiejoe]

Ok, I chose option 2c ;-)

I think that providing a way to use Compat-5.3 in a Lua module without affecting other modules would greatly increase the usefulness of the Lua part of this project. Therefore, I want to split the current single file module in two modules (one with global effects, one for module authors). However, using debug functions to achieve isolation would prevent many uses (I also considered using a C module for that, but that could exclude other developers interested in this module). There is a way that does not need debug functions but instead shifts some of the burden to the module developer:

local _ENV = require("compat53.module")
if setfenv then setfenv(1, _ENV) end  -- if you need to support Lua 5.1

This approach also makes it possible to merge compat53.module and compat53.impl, so we end up with only two files (both of which can be required by other people).
compat53.module will not include yieldable pcall support (like @hishamhm suggested). I decided against the extra compat53.pcall module because the implementation became too messy: When compat53.pcall is required you would need to identify the environment in which to import the functions. One can not simply add those to the module table because then a user of compat53.module would end up with the modified functions if some other module developer decided to require both compat53.module and compat53.pcall. This requires the use of debug functions, in which case the main module (compat53) would have a dependency on debug functions as well (or we would end up with two different compat53.pcall modules). Also some coroutine functions depend on a shared upvalue (a fake main coroutine), so mixing modules written with/without compat53.pcall could lead to similar problems as the current coroutine.running issue. If you really need yieldable pcall in a module for PUC-Rio Lua 5.1, there's always coxpcall (which should be fit for Lua 5.3 as soon as my pull request is merged).

For users of the normal compat53 module nothing changes (other than that you need two files now): yieldable pcall, string/file methods, etc. are all available.

Regarding cherry picking of individual functions/mixed APIs: This is definitely possible in this approach, and frankly I don't care that much. I've made it clear in the readme how compat53.module is supposed to be used, and that we don't recommed/support cherry picking. But with pcall etc. gone from compat53.module I can see no technical reason why it shouldn't work (other than bad style).

I've implemented the outlined approach in the isolated branch for testing. I plan to merge it into master in a few days, so this is the last chance for objections/feedback.

[siffiejoe]

Done.

[daurnimator]

I was reading through the xpcall code today, is it intentional that both compat.init and compat.module have their own main_coroutine? it seems we're doing that wrapping twice...

[siffiejoe]

Yes, that was intentional. main_coroutine is needed for three of the functions from the coroutine library (one of them coroutine.running). coroutine.running has to change when (x)pcall become yieldable. I found it easier/cleaner to redefine the other two coroutine functions as well and keep main_coroutine as an upvalue than to share main_coroutine between compat.module and compat.init somehow.

[daurnimator]

Yes, that was intentional. main_coroutine is needed for three of the functions from the coroutine library (one of them coroutine.running). coroutine.running has to change when (x)pcall become yieldable. I found it easier/cleaner to redefine the other two coroutine functions as well and keep main_coroutine as an upvalue than to share main_coroutine between compat.module and compat.init somehow.

I thought you could write all the stuff in init.lua without recreating the main coroutine.
Couldn't you have:

local coroutine_running = M.coroutine.running -- from compat53.module
      function M.coroutine.running()
         local co, main = coroutine_running()
         if not main then
            return pcall_mainOf[co] or co, false
         else
            return co, true
         end
      end

Once you have this, resume and status don't need to be overridden a second time.

[siffiejoe]

Right, that should work ...