Open rurban opened 11 years ago
Thanks for the (huge) PR — I'll have to check how well nagaqueen fares on your version.
When you mean 'proper' error blocks, do you mean that they behave the same, but that instead of being attached to every kind of node, they're just a node of their own now? (hence smaller memory footprint, etc.) I haven't had time to understand all the changes yet.
yes, 'proper' error blocks mean 1. only one special Error node and action instead of being attached to every node, and 2. using the upstream version, not our own :)
re Makefile 3rd line: hmm, since it's only commented I thought of leaving it there.
yes, 'proper' error blocks mean 1. only one special Error node and action instead of being attached to every node, and 2. using the upstream version, not our own :)
Yup, I noticed a few things were borrowed from upstream. What I'm really wondering is how functionally equivalent they are. An incompatible change would be painful for both nagaqueen & other greg users.
The only incompatibility is:
upstream leg always used to differ from our YY_ERROR and YY_INPUT. He recently added an optional context variable, which resembles our struct _GREG being passed around, and also renamed the internal struct members.
See that's why I think more details are needed in your changelog, along with migration instructions for users. And any rename fo internal struct members might need to be documented if they could have been used.
For example, the YY_INPUT in NagaQueen does need the _GREG struct because it has this 'read routine' thing that allows reading from file or from a memory block in a cross-platform fashion: https://github.com/nddrylliog/nagaqueen/blob/master/grammar/nagaqueen.leg#L136 - it that can't be done with your 0.4.5 update, that's a step back.
I'm all for getting closer to (or at least keeping track of) Ian's changes, but I want to see the implications.
The only action for greg users is to remove the 1st G arg from YY_ERROR and YY_INPUT.
Rationale: Whenever YY_ERROR and YYINPUT is used there already is G available, there's no need to pass it around as the macro arg. This is also consistent with all other YY* macros. Most of them do not pass G around. E.g.
#define YY_ERROR(message) yyerror(G, message)
define YY_BEGIN ( G->begin= G->pos, 1)
The required patch for nagaqueen.leg is as follows:
diff --git a/grammar/nagaqueen.leg b/grammar/nagaqueen.leg
index 480a15d..f43ef63 100644
--- a/grammar/nagaqueen.leg
+++ b/grammar/nagaqueen.leg
@@ -36,7 +36,7 @@ void GC_free(void *);
#define rewindWhiteSpace { \
/* only rewind if at end of file */ \
- if (core->eof == 1) { \
+ if (G->data->eof == 1) { \
int originalPos = G->pos; \
char *c = G->buf + G->pos; \
/* rewind until we reach something non-whitespace */ \
@@ -50,12 +50,12 @@ void GC_free(void *);
// Throw error at current parsing pos. Used when nothing valid matches.
#define throwError(val, message) \
- nq_error(core->this, (val), (message), G->pos + G->offset)
+ nq_error(G->data->this, (val), (message), G->pos + G->offset)
// Throw error at last matched token pos. Used with invalid tokens being
// parsed for more helpful messages (e.g. misplaced suffixes).
#define throwTokenError(val, message) \
- nq_error(core->this, (val), (message), core->token[0])
+ nq_error(G->data->this, (val), (message), core->token[0])
#define missingOp(c) { \
rewindWhiteSpace; \
@@ -133,7 +133,7 @@ static int _nq_fread(void *ptr, size_t size, NagaQueenCore *core) {
#define YY_XTYPE NagaQueenCore *
#define YY_XVAR core
-#define YY_INPUT(buf, result, max_size, core) yyInput(buf, &result, max_size, core)
+#define YY_INPUT(buf, result, max_size) yyInput(buf, &result, max_size, (G->data))
void yyInput(char *buf, int *result, int max_size, NagaQueenCore *core) {
(*result) = core->io.read(buf, max_size, core);
I'll revert the YY_INPUT and YY_ERROR api changes, so that the impact will be minimal. I'm persuaded.
no it's okay, the diff you provided for nagaqueen looks good, I just haven't had time to merge it yet. I will soon.
@rurban Your version seems to work fine with the nagaqueen patch you've provided, which is cool with me.
However, I've tried (unsuccessfully) to merge your changes with the changes we've had on this repo - our forks have diverged, please rebase your changes on the latest master here so that it applies cleanly.
(For clarification: I spent about an hour resolving merge conflicts and when I was done, I got a greg that segfaults - I'm just not that well-versed in greg anymore to know what's going on exactly, nor do I have the time. Since you're proposing a lot of changes/fixes I think it's only fair if you do the merging yourself.)
e@rurban what's the current status about this?
Please hold. I tried to make greg safe for GC usage (re-entrant eval), but failed, and have no time right now. In a few weeks.
On Mon, Nov 18, 2013 at 5:38 PM, Tokuhiro Matsuno notifications@github.comwrote:
e@rurban what's the current status about this?
— Reply to this email directly or view it on GitHubhttps://github.com/nddrylliog/greg/pull/15#issuecomment-28751346 .
Reini Urban http://cpanel.net/ http://www.perl-compiler.org/
ping?
Highlights: less memory leaks, default parse_new default yyerror filename "" => "-"
remove G from YY_ERROR and YY_INPUT 1st arg
initialize input, filename, lineno in parse_new
skip -vv verbose printing for typical lexer rules
add main in default trailer with -DYY_MAIN, build samples
add improvements from upstream peg-0.1.13: add predicates have access to yytext use __inline on Win32 proper ~{ error } block use C89 escape for ESC enclose user actions in braces to allow local variables to be declared in C89