Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lua 5.3 + 5.4 integer support, with CI and conflicts fixed #84

Merged
merged 24 commits into from
Jun 11, 2024

Conversation

hishamhm
Copy link

@hishamhm hishamhm commented Jul 12, 2022

Hello @leafo, @agentzh and @cloudwu!

I just took #3 which has been open since 2015(!) and got it to a mergeable state, by reconciling the changes made in https://github.com/openresty/lua-cjson and https://github.com/cloudwu/lua-cjson. I also added Lua 5.4 to the test matrix added by @leafo in #50.

I have made an explicit merge commit to keep history of both forks, but I can also rebase the cloudwu branch onto this repo if preferred. I'd just like this to get merged and the lua-cjson Lua version compatibility nightmare to be over after so many years. :)

I also noticed that version 2.1.0.10 was tagged in this repo but it was never uploaded to LuaRocks: https://luarocks.org/modules/openresty/lua-cjson

Ideally, we should merge this, tag 2.1.0.11 and upload it to LuaRocks (I can either add a commit with the version bump, or make a separate PR).

Thanks!


Closes #3.
Fixes #67.
Fixes #36.

@hishamhm
Copy link
Author

Also, friendly ping to @xiaocang since you tagged 2.1.0.10 :)

@leafo
Copy link

leafo commented Oct 17, 2022

Just a heads up: I pushed 2.1.0.10 and 2.1.0.9 to luarocks.org today, which should allow it to install on Lua 5.2+.

mpx and others added 17 commits June 6, 2024 15:34
- Add explicit casts for functions returning void*
- Rename "try" variable to avoid reserved C++ keyword
inline ==> __inline
snprintf ==> _snprintf
strncasecmp ==> _strnicmp
Modern versions of Perl generate warnings, but the unit tests are
currently designed to also test against the invalid UTF-8 characters.
Co-Authored-By: Hisham Muhammad <hisham@gobolinux.org>
This allows using the AR variable to change the linking step.

Co-Authored-By: Hisham Muhammad <hisham@gobolinux.org>
Metamethod __index access only supported for Lua 5.2+.
Lua 5.1 and LuaJIT use raw index access.

Co-Authored-By: Hisham Muhammad <hisham@gobolinux.org>
hishamhm added 4 commits June 6, 2024 16:15
Add inline warning suppressions to silence some reports in dtoa.c:

* nullPointerArithmetic warnings are harmless. In modern systems where
  NULL == 0 these are redundant; that code is probably there for more
  esoteric architectures where assigning an int to a pointer is only
  safe if assigned to the integer + NULL.
* integerOverflowCond warnings are probably intentionally caused integer
  overflows. There's a lot of code in this library to check for
  overflow states, so these are most likely intentional, leading to
  "redundant" checks later, which probably are not redundant in some
  specific architecture/platform.
@hishamhm hishamhm force-pushed the master branch 2 times, most recently from 073735d to ada7880 Compare June 6, 2024 21:58
hishamhm added 2 commits June 6, 2024 19:04
This allows encoding proxy arrays correctly (as in the included
test case), across all Lua versions.
Get a more recent Valgrind which can read the latest Clang's debuginfo.

See: llvm/llvm-project#56550
@hishamhm
Copy link
Author

hishamhm commented Jun 6, 2024

@leafo I rebased the original version of this PR into latest master, and I got the Github Actions workflow to run green (at least on my fork; it does not show up here).

So this branch now contains a bunch of unrelated / semi-related things:

  • Lua 5.3+ integer support, from @cloudwu + tweaks and fixes, including those of mine
  • adjustments to get the Github Actions CI runnable and green (it's not activated in this repo though? it seems to have both Travis and Actions files, but only Travis is running here)
  • vendored dtoa.c version bump from upstream (which I did while fixing cppcheck warnings from CI)
  • various other minor upstream commits, from original lua-cjson author @mpx as well as from @cloudwu's fork
  • rockspec fix and bump, readying it for a new release 2.1.0.14.

I can split these into multiple PRs if necessary for review but I think the Actions CI will only run green with all of them.

@Tieske has pinged me asking whether we can get a new release pushed to luarocks.org because of the security fix in 2.1.0.13 (which has the security fix but a broken rockspec). The latest version in luarocks.org is 2.1.0.10.

Could we get this PR reviewed/merged and a new release out? Or at the very least push 2.1.0.13 to luarocks.org with a fixed rockspec.


if (n > maxthreads) {
L = n*sizeof(ThInfo);
if (TI1) {
TI1 = (ThInfo*)REALLOC(TI1, L);
memset(TI1 + maxthreads, 0, (n-maxthreads)*sizeof(ThInfo));
newTI1 = (ThInfo*)REALLOC(TI1, L);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you please tell me the url of this dtoa.c?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is based on
https://portal.ampl.com/~dmg/netlib/fp/dtoa.c
https://portal.ampl.com/~dmg/netlib/fp/changes

For this particular change you highlighted, I wrote it myself (see commit 026466c), as it was an issue reported in CI by cppcheck.

@zhuizhuhaomeng zhuizhuhaomeng merged commit c92ecda into openresty:master Jun 11, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LuaRocks version fails on Lua 5.4 Won't built against Lua >= 5.2
10 participants