search

vnmakarov / yaep

Yet Another Earley Parser
Other
135 stars 13 forks source link

Travis: Also install YAEP #15

Closed sanssecours closed 5 years ago

sanssecours commented 5 years ago

While the changes in commit 7d1bb5c might be controversial, it makes sense in my opinion to also offer support for an allocation facility “out of the box”. At least as user of this library, I am currently not really interested on how it allocates memory for the parse tree. This might change over time, but I assume most people just want an nice default allocation function, which objstack.h seems to offer.

This pull request also reverts commit 1a5f6cb. While I also noticed that the library seems to leak memory, I still think the unit tests should pass, after someone applies a fix for the problematic code.

If I should change anything in this PR, please just comment below. Thank you.

TheCount commented 5 years ago

@sanssecours just a heads-up, I plan to open a pull request that replaces the current allocator with a reentrant version some time this weekend. While this does not change any external interfaces of YAEP (if you remove allocate.h from the install), this means some internal changes, including to the OS_* macros.

TheCount commented 5 years ago

@sanssecours See #16.