Lua 5.1 / 5.3 alignment and document

[TerryE]

This is a big PR encompassing #2865, #3152, #3165 (part), #3175, #3176 and #3180.

Make sure all boxes are checked (add x inside the brackets) when you submit your contribution, remove this sentence before doing so.

• This PR is for the dev branch rather than for release.
• This PR is compliant with the other contributing guidelines as well (if not, please describe why).
• I have thoroughly tested my contribution.
• The code changes are reflected in the documentation at docs/*.

Overview of PR

This aims to sweep up most of the loose ends following the Alpha release of Lua 5.3 and as such is a big PR. Sorry but all of the interdependencies make this a difficult one to untangle. So this encompasses:

• Incompatible internal luaL_buffer probe in sjson.c #2865 Use of luaL_buffer internals in sjson.c
• Emergency GC in Lua 5.3 #3152 Emergency GC in Lua 5.3.
• Additional documentation for Lua 5.3 #3165 Additional documentation for Lua 5.3. See new docs/ files for the NodeMCU LRM and the LFS Whitepaper. This second is probably far too detailed for mode readers and needs culling, IMO.
  The NodeMCU Lua Reference Manual documents for the first time how our firmware and C API differs from the standard Lua 5.3 Reference Manual. Can I ask others to print this off and read it? Please give feedback.
• Stripdebug feature #3170 Implement Stripdebug feature for Lua 5.3
• RFC: Tidy up node.flashindex interface #3176 RFC: Tidy up node.flashindex interface
• RFC: Tidy up the Lua C API / library code split and interface #3180 RFC: Tidy up the Lua C API / library code split and interface. To see how this works see for example node.c:node_lfs_func()

Still TBD

• I am still tuning the GC stuff. I will add this as a separate commit.
• Review / implementation of luac.cross strip debug levels
• @HHHartmann nodemcu-firmware/msvc/luac-cross/ files need updating to reflect the linux make, plus similar changes for app/lua53

Caveat

Because of the size of this, I am expecting extra rework so lease don't merge until we've had this review cycle.

[nwf]

I'm afraid I have time only for a cursory inspection at the moment, but I've pulled my dev-active up atop this PR and have successfully run several iterations of my test harness. It seems as though this PR also corrects some LFS bug I was half-heartedly hunting, as I am now able to load sizable LFSes (40576 bytes at the moment) that previously would result in the module being unable to find _init.

I am occasionally seeing #3096, I think, as well as the occasional invalid instruction panic that I haven't yet debugged, but neither of these are regressions that should hold up this PR.

[TerryE]

I've got at least one more commit to add:

• updates to nodemcu-lrm.md following review comments
• updates to node.c and node.md following resolution of RFC: Tidy up node.flashindex interface #3176 and RFC: what should we do with the lua_examples/lfs ? #3200
• updates to lua_examples/lfs following RFC: what should we do with the lua_examples/lfs ? #3200
• possible updates to documentation into page depending on @marcelstoer feedback.
• updates to MinGW and MSVS make files for lua/luac_cross and lua53/host
• EGC tuning.

We are really blocked on this until I get some of this feedback / input.

[TerryE]

I had hoped to get some feedback on the new documentation that I've added, but alas not to be. This PR is still blocked pending other committer input as per my previous comment. I can't update the node.md documentation until we have agreement on the exact details of the node.LFS implementation.

[marcelstoer]

I had hoped to get some feedback on the new documentation that I've added

I had planned to make some time for that (late) tonight.

[TerryE]

I had planned to make some time for that (late) tonight.

Thanks Marcel. Much appreciated. Unlike the rest of you guys, I am now a gentleman of leisure / independent means, so I don't have to worry about the day job. 😄

[TerryE]

I'm going to try to sit down with this this evening.

@jmattsson, thanks for this. Even if you only have limited time available and can only give general feedback, I still value your perspective and depth of experience.

It terms of the overall structure of the code, you will see that all of the eLua botches are gone and we now have a proper layering of node and other libraries over lauxlib.h with the only API into Lua internals offered by lua.h. I've backported enough Lua 5.3 extension into the Lua 5.1 API that all of the app/modules compile and work against both runtimes without material #ifdef variants. Lua 5.3 seems to be stable as well; more RAM efficient and slightly faster than Lua 5.1.

[TerryE]

Woah Terry, for this alone I will from now on call you Sir Terry ...

Thanks for the complement. Much appreciated.

What I would appreciate if you included it in this PR are changes to the Lua Developer FAQ and the Extension Developer FAQ. I am quite convinced that a number of paragraphs in both documents could be replaced by links to the new reference documents.

To understand the reason you need to have a scan through the companion document the Lua 5.3 Reference Manual and start with its contents. This is a very dense but formal specification: the basic concepts and language is covered in §1-3; the standard Lua libraries are in §6; and §4-5 cover the C API that is used to code C library modules.

This NLR is really a supplement specifying how our NodeMCU implementation differs from the standard Lua language and implementation as described in the LRM. You will see that I follow the same order, prose style and layout, but only discuss the differences. The reason for the seeming focus on the C API is that there very few differences for §1,2,3,6 as the Lua language and standard libraries are almost unchanged in NodeMCU.

The bit that we have added is really the support for ROTables and the ability to move constant data structures from RAM in code space in ROM, and the extra API calls defined in lua.h and lauxlib.h that support, and so perhaps 80-90% of its content relates to additions to and some tweaks of §4 and §5.

So essentially the LRM + NLR + PiL + its supplement (I haven't got a neat name for this) + the ReadTheDocs library documentation now for the first time form a complete specification of NodeMCU Lua, its libraries and how to implement new ones.

I am not wedded to NLR. If any of you guys can come up with a more appropriate name and acronym then we can do the swap.

As to Lua Developer FAQ, the Extension Developer FAQ, and the _LFS Whitepaper, these are getting a little dated and it's probably time for new editions removing content that overlaps with these new and more formal references. However, I would rather do this editing cycle as a separate PR. This PR is already big enough IMO.

[TerryE]

To be honest documenting the NodeMCU delta acted as a trigger to tidy up the API a little more, because as I documented some of the calls I realised that they hadn't been done in the "Lua implementation style", and by reworking the API to conform to this style, I actually improved the implementation. A good example of this is the node.LFS.__index implementation. The original 5.1 version had lots of nasty internal Lua runtime stuff in it and this would have to be very different for Lua 5.3. By abstracting this and moving the appropriate internal Lua runtime functionality into the lua.h interface and the luaxlib.h one, each runtime looks after its own version specific internals and the node.c code is now common to both.

[HHHartmann]

More small bits as PM.

[jmattsson]

Overall, absolutely fabulous. Loving all the documentation!

Only other comment is that Travis looks unhappy about Windows builds, and all I can say about that is that I too would be unhappy if I had to build on Windows 😆

[nwf]

Just some very minor nits. This is delightful. 🎉 🥇

[KT819GM]

Not sure, but maybe someone should note about esp32-s2. It will be ideal replacement for esp8266 (in my opinion), but at the moment it’s not supported and possibly will not be till 4.x LTS will roll out.

[TerryE]

First of all, my thanks to everyone for a super job in doing this review. I suspect that 90% of the comments will result in an "OK will do". In this case instead of explicitly posting this and generating a storm of these email notifications, I will just mark the comment as resolved. In such cases, please take the "thanks, will do" implicitly.

I had emailed an ODT version of the docs to some viewers. (Gregor replied with a marked version so his proofing comments aren't inline here.) What I suggest I do is that as well updating the document here I save the before and after versions and do a markup compare in Google Docs. I will send out this link by email to reviewers.

So now begins my task of reviewing and aggregating all comments. I suspect that this will need a chunk of work. Where appropriate I will do this inline, but will post separate comments for specific that merit wider discussion To be continued 😄

[TerryE]

After comments from @marcelstoer, I suggest that we adopt the abbreviations:

• LRM for the Lua 5.3 Reference Manual
• NRM for the supplemental NodeMCU Reference Manual has has been created in this PR
• PiL for the 4th edition of Roberto Ierusalimschy's book "Programming in Lua"
• PiN for the supplemental NodeMCU reference "Programming in NodeMCU" also created in this PR.

I have tweaked the lede sections of these documents to adopt this convention consistently. I was toying with "NodeMCU" vs "NodeMCU Lua" in our documents, but as Marcel raised in one review comment: we have left the Lua language and libraries largely unchanged, so nearly all of the content in our supplemental documents is about the C API for writing C modules; therefore having "Lua" in the titles could be a little confusing.

[TerryE]

I've been away from my main dev setup for a few days, so I have been coding up #3206 in my mind. However, I will resist the scope creep of adding it here. I will update this with the review comments and let's merge this first. #3206 can quickly follow.

I have been going though and updating the documentation, but I propose to hold off on lfs.md updates, since will be significantly impacted by the new functionality. (It's quite difficult writing this when you have a 3 year-old grandson wanting a cuddle 🤣)

[TerryE]

Hopefully this latest set of updates has swept up all of the review comments. Note that I've missed hooking the lua53.md paper into mkdocs.yml, but it is in now. I'd like to merge this in the next da or so, so I can move onto the 2 LFS + ESP building changes.

[galjonsfigur]

When I was testing Lua 5.3 LFS I found that when node.flashreload function gets called on non-existing file it gives misleading error message: No LFS partition allocated and on Lua 5.1 it gives expected error message: LFS image file not found. I think the fix could be added to this PR rather than opening separate issue and PR.

[galjonsfigur]

Another little bug was found by updating to GCC 10 - in lua53/linit.c, line 78:

LROT_TABLE(base_func);

is missing extern attribute and that causes build to fail with message:

/bin/ld: .output/host/release/obj/linit.o:(.rodata+0x260): multiple definition of `base_func_ROTable'; .output/host/release/obj/lbaselib.o:(.rodata+0x2e0): first defined here

[TerryE]

BTW @pjsg you mentioned somewhere that the new code returns from reload. This was because the old version did a forever loop to trigger a restart and I replaced this by a system restart call which is is executed in a separate task. What I've done is to follow the restart call by throwing a "system restarting" error which will normally terminate the current task, whilst allowing the SDK to shutdown and restart gracefully.

[TerryE]

@galjonsfigur you mentioned above that node.flashreload function gives misleading error when called with a non-existing file. I am about to issue a new PR for on-ESP LFS creation which reworks this area anyway so let me sweep the point up in this PR.