process macro chaining caused by concat correctly

[umegaya]

hi, I love lcpp feature and use it in my project for parsing some practical library's header file (pthread.h in osx). but lcpp did not handle macro chaining as I expected.

below quick example about my expectation.

#define CONCAT_TEST1 foo##bar
#define CONCAT_TEST4(_baz) CONCAT_TEST##_baz
CONCAT_TEST4(1) -- I expect this valid macro definition, but it cause error like "attempt to call global 'CONCAT_TEST4' (a nil value)"

#define CONCAT_TEST5(baz) CONCAT_TEST##baz
CONCAT_TEST5(1) -- its recognize as macro. so I expect its compiled to foobar, but lcpp keep it CONCAT_TEST1

#define LCPP_NOT_FUNCTION (BAR) -- I expect this replacement macro definition, but it also recognized as functional macro

and it seems that this problem caused by 2 reasons. a) if functional macro argument name contains _, entire macro is not recognized as valid one, because regex for argument list definition only match all alphanumeric, space, and comma:

local FUNCMACRO = STARTL.."("..IDENTIFIER..")%s*%(([%s%w,]*)%)%s*(.*)"

b) lcpp missing re-processing token when ## is processed. for above example, after CONCAT_TEST4 processed and it compiled to CONCAT_TEST1, lcpp reprocess and compiled it to foobar.

c) FUNCMACRO allows space between macro name and arguments list. also processLine's macro processing potentially allows 1 macro definitions processed as multiple macro type (I'm not sure its needed).

I add some test case and fixing lcpp itself to pass these new test. but this is not entirely tested for various macro usage. so can you review this fix? if it seems good, I would appreciate if you could merge this to master.

thanks and happy new year!!

[m-schmoock]

Hey,

Thanks for the feedback. i was on vacation and just read this. I will add your patch soon :D

thank you, Michael

On Wed, January 1, 2014 1:06 am, iyatomi takehiro wrote:

hi, I love lcpp feature and use it in my project for parsing some practical library's header file (pthread.h in osx). but lcpp did not handle macro chaining as I expected.

below quick example about my expectation. ``` lua

define CONCAT_TEST1 foo##bar

define CONCAT_TEST4(baz) CONCAT_TEST##baz

CONCAT_TEST4(1) -- I expect its compiled to foobar, but lcpp keep it CONCAT_TEST1

and it seems that lcpp missing re-processing token when ## is processed.
for above example, after CONCAT_TEST4 processed and it compiled to
CONCAT_TEST1, lcpp reprocess it again
and compiled it to foobar.

I add some test case and fixing code which passes these new test.
but this is not entirely tested for various macro usage. so can you review
this fix? if it seems good, please merge this commit. thanks and happy new
year!! You can merge this Pull Request by running:

git pull https://github.com/umegaya/lcpp
umegaya/feature/concat_macro_chaining

Or you can view, comment on it, or merge it online at:

https://github.com/willsteel/lcpp/pull/11

-- Commit Summary --

* process macro chaining caused by concat correctly

-- File Changes --

M lcpp.lua (30)

-- Patch Links --

https://github.com/willsteel/lcpp/pull/11.patch
https://github.com/willsteel/lcpp/pull/11.diff

[umegaya]

hi, after tried to handle many complex case, I realized that not only concatenate but also macro replacement triggers new macro replacement. so I change state:apply so that it repeatedly try to apply macro set until no macro can apply. also change state:prepareMacro repeatedly try to concatenate and state:apply until no concatenation happen.

also I fix following issue

1. C string literal concatenate "A" "B" => "AB"
2. stringify operator "#" #define STRING(x) #x STRING(bar) => "bar"
3. processing macro argument string which contains ","

with these fix, now my version of lcpp seems to be able to parse pthread.h, lualib.h, lauxlib.h, stdlib.h, string.h in macosx correctly. can you also review these fix? slightly big change compared with first 2 commits. I worry about both correctness and parsing performance. (though I think there is no significantly slow down with my fix)

if necessary, I will squash these commits together. thanks!

[m-schmoock]

Hey,

many thanks. I will review and add it to the main repository. I think those two features were on the top of the list, so lcpp is now more or less complete.

Thanks, Michael

On Fri, January 10, 2014 5:28 pm, iyatomi takehiro wrote:

hi, after tried to handle many complex case, I realized that not only concatenate but also macro replacement triggers new macro replacement. so I change state:apply so that it repeatedly try to apply macro set until no macro can apply. also change state:prepareMacro repeatedly try to concatenate and state:apply until no concatenation happen.

also I fix following issue 1. C string literal concatenate "A" "B" => "AB"

1. stringify operator "#" #define STRING(x) #x STRING(bar) => "bar"
   1. processing macro argument string which contains ","

with these fix, now my version of lcpp seems to be able to parse pthread.h, lualib.h, lauxlib.h, stdlib.h, string.h in macosx correctly. can you also review these fix? slightly big change compared with first 2 commits. I worry about both correctness and parsing performance. (though I think there is no significantly slow down with my fix)

if necessary, I will squash these commits together. thanks!

---

Reply to this email directly or view it on GitHub: https://github.com/willsteel/lcpp/pull/11#issuecomment-32047462

[m-schmoock]

Hey,

sorry im a bit busy and also lazy. im reviewing your branches right now: umegaya/feature/remove_comment umegaya/feature/concat_macro_chaining

the latter one seems to produce an test error: will@will-mac ~/lcpp.umegaya.git $ make test lua -e 'local lcpp = require("lcpp"); lcpp.test();' lua: ./lcpp.lua:1308: ./lcpp.lua:961: attempt to index global 'bit' (a nil value) stack traceback: C: in function '(for generator)' ./lcpp.lua:1308: in function 'compile' ./lcpp.lua:1808: in function 'test' (command line):1: in main chunk

make: *\ [test] Error 1

[umegaya]

oh, sorry, I only test my code with luajit (because I only uses lcpp for luajit FFI purpose) and actually test works for luajit in my environment as below. but you aim to use it also for lua code, I should support lua as well. it seems to be cause by vanilla lua does not have bit module. plz wait for fixing it :D

$ git branch -v
  master                                81ac3e6 Merge branch 'master' of https://github.com/willsteel/lcpp
* umegaya/feature/concat_macro_chaining 5e3f651 fix numerical literal parsing small fix for parsing rare case in C system headers
$ luajit -e 'local lcpp = require("lcpp"); lcpp.test();'
lcpp INF [0000] Test run suscessully

[umegaya]

with e0aaa66, now it should work with lua 5.1.5

[m-schmoock]

I merged stuff and added you to contributos so you can chaing main repo now :D code looks good. thanks for you work!

[m-schmoock]

nice. thanks. your now contributor in main repo. should we repackage the rockpec and announce to luarocks mailing list?

On Tue, March 18, 2014 1:39 am, iyatomi takehiro wrote:

with e0aaa66, now it can work with lua 5.1.5

---

Reply to this email directly or view it on GitHub: https://github.com/willsteel/lcpp/pull/11#issuecomment-37891642

[umegaya]

thank you for adding me as contributer :D

I think it is good time to announce new release. I don't know about the process for releasing new version of module to luarocks very well, this 2 steps are only things to do?

• change rockspec's name according to new version (eg. lcpp-scm-2.rockspec) and also version value inside of rockspec file
• post new rockspec's URL to luarocks ML

[m-schmoock]

the scm has not tp be update because scm means something like "git package version". so its always up2date. however I didnt announced it yet.

so what we can do is build a lcpp 1.1 packaged luarocks. and then announce both, the scm and the 1.1. for the URL inside the luarock we maybe can use github: tag/branch 1.1 version. and use a raw link or something like that.

I also have none experience in contributing luarocks :D

Ah, one thing to the lcpp interface functions (conpile and compileFile) where you added some parameters: macro_sources, next, _local

_local and next dont seem to be used. the are all not luadoc inline documented. I assume _local and next are useless and only added by mistake. or not? The "macro_sources" seems okay, but do we need it in the signature of the interface functions? Is there any usecase or special need?

cheers, Michael

On Tue, March 18, 2014 3:07 pm, iyatomi takehiro wrote:

thank you for adding me as contributer :D

I think it is good time to announce new release. I don't know about the process for releasing new version of module to luarocks very well, this 2 steps are only things to do? - change rockspec's name according to new version (eg. lcpp-scm-2.rockspec) and also version value inside of rockspec file - post new rockspec's URL to luarocks ML

---

Reply to this email directly or view it on GitHub: https://github.com/willsteel/lcpp/pull/11#issuecomment-37943785

[umegaya]

hi, _local and next is for resolving header file path. that is hint for user of lcpp. example is here.

if lcpp.compileFile called with "_local = true" means file is included by #include "...", instead of #include <...> and "next = true" means file is included by #include_next ".." or #include_next <...>

my usage estimation is inherit lcpp.compileFile like my example and change the way of resolving actual header file path according to the value of next and _local.

at lcpp level, current default behavior is treated file path provided from directive as real file path.

but when user try to parse practical header files, it probably includes some header file path like

#include <stdio.h> 

which full path needs to be resolved. in such case, _local and next is very useful.

ofcourse I can add resolving path feature to lcpp itself, but some rule of resolving header file path is implementation defined (for example, see here), so I think this feature beyond the problem domain of lcpp and decided to just keep it customizable for user of lcpp.

but at least I should document about _local and next in function signature. I will fix it.

and please tell me your opinion. for example, should we prepare for some interface, instead of telling user to replace compileFile?

regards

Takehiro Iyatomi

[aktau]

@umegaya hey! Sorry to butt in on the conversation but how does this relate to ffiex now? Are you planning on discontinuing it now that your fixes have been merged or do you feel like you want to build out some more functionality?

[umegaya]

@aktau hi, I will keep on maintaining and developing ffiex. this pull request just feedback what I have done in the development of ffiex.

now I become collaborator of lcpp, so I will take care of maintaining lcpp also, but maybe my main development for lcpp will be still done in ffiex first, then feeding it back to here (if it is suitable for the problem domain of lcpp).

for new functionality of ffiex, please check https://github.com/umegaya/ffiex/issues

thanks!

[aktau]

@umegaya great, thanks!

@willsteel sorry for the hijack, I'll shut up now ;)