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

few syntax improvements #63

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

few syntax improvements #63

wants to merge 13 commits into from

Conversation

McIkye
Copy link

@McIkye McIkye commented Apr 23, 2015

improve some of the compiler compatibility and get rid of the dead code

@bogdanm
Copy link
Member

bogdanm commented Apr 28, 2015

Hi,

Thanks for your PR. I understand what ed74533 and b62616b are fixing, but I'd rather not modify the original Lua sources unless really needed (becuase that would make applying (and extracting) patches much harder). About 7aa2e45, I'm not sure about its scope, is it fixing an actual problem?

Thanks,
Bogdan

@McIkye
Copy link
Author

McIkye commented Apr 28, 2015

it removes dependency on locale.h which is not necessarily available on embedded platforms and is not required for the absanse of float numbers

@bogdanm
Copy link
Member

bogdanm commented Apr 29, 2015

That makes sense, thank you. I'll take the commit(s) that remove the dependency on locale.h, but as mentioned before, I'd rather not merge the ones related to toolchain compatibility improvement, or other such commits that change the base Lua code without a very good reason to do so.
I saw that you also added some commits around porting back bitops from Lua 5.3.I believe those are important in an embedded system and I'm willing to allow changing the Lua code because of that, but please issue a separate PR with those changes, this one is getting too big.

@jsnyder
Copy link
Member

jsnyder commented Apr 29, 2015

Two added notes:

  1. To make separate pull requests you can put these changes in branches rather than your master branch.

  2. For ed74533 and b62616b, I think at least some of these were introduced in some of the patches we applied to the Lua core and the original sources are C89 compliant.

For example: https://github.com/jsnyder/lua/blob/master/src/lapi.c#L926 vs ed74533

That said, I believe what we have is still valid C99. Also the Lua Tiny RAM patches that allow for read-only tables that conserve SRAM are built using designated initializers which bump our compiler requirement up to C99 anyways.

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.

None yet

3 participants