Fix references when rules are unused, and fix missing cloneExpr #83
Conversation
ast/ast.go
Outdated
| @@ -166,7 +166,7 @@ func (a *ActionExpr) Pos() Pos { return a.p } | |||
|
|
|||
| // String returns the textual representation of a node. | |||
| func (a *ActionExpr) String() string { | |||
| return fmt.Sprintf("%s: %T{Expr: %v, Code: %v}", a.p, a, a.Expr, a.Code) | |||
| return fmt.Sprintf("%s: %T{Expr: %v, Code: %v, FuncIx: %d, ID: %p}", a.p, a, a.Expr, a.Code, a.FuncIx, a) | |||
There was a problem hiding this comment.
I had added this for easier debugging (and noticing the missing clone), let me know if you'd rather have it removed (this could be considered a breaking change, but I think that's a stretch :) )
Makefile
Outdated
| @@ -154,6 +154,12 @@ $(TEST_DIR)/issue_65/optimized/issue_65.go: $(TEST_DIR)/issue_65/issue_65.peg $( | |||
| $(TEST_DIR)/issue_65/optimized-grammar/issue_65.go: $(TEST_DIR)/issue_65/issue_65.peg $(BINDIR)/pigeon | |||
| $(BINDIR)/pigeon -nolint -optimize-grammar $< > $@ | |||
|
|
|||
| $(TEST_DIR)/issue_70/issue_70.go: $(TEST_DIR)/issue_70/issue_70.peg $(BINDIR)/pigeon | |||
There was a problem hiding this comment.
Why have you removed the additional lines for the optimized parsers? (see here)?
There was a problem hiding this comment.
Probably a mistake when fixing the makefile conflict. Added them back.
ast/ast.go
Outdated
| @@ -166,7 +166,7 @@ func (a *ActionExpr) Pos() Pos { return a.p } | |||
|
|
|||
| // String returns the textual representation of a node. | |||
| func (a *ActionExpr) String() string { | |||
| return fmt.Sprintf("%s: %T{Expr: %v, Code: %v}", a.p, a, a.Expr, a.Code) | |||
| return fmt.Sprintf("%s: %T{Expr: %v, Code: %v, FuncIx: %d, ID: %p}", a.p, a, a.Expr, a.Code, a.FuncIx, a) | |||
There was a problem hiding this comment.
I checked my local branches and found a commit, where I started to work on the bigger problem, that I mentioned in #70. I updated the issue #70. Would you like to look into this as well?
I noticed that cycle too, as mentioned in https://github.com/mna/pigeon/pull/83/files#diff-e9f48c3beb9b8048f6d2247c99564198R47 . I don't have the bandwidth to look into this, but with the clone fix, this shouldn't cause any build issues, it is just additional code minification optimization that could be improved.
Makefile
Outdated
| @@ -154,6 +154,12 @@ $(TEST_DIR)/issue_65/optimized/issue_65.go: $(TEST_DIR)/issue_65/issue_65.peg $( | |||
| $(TEST_DIR)/issue_65/optimized-grammar/issue_65.go: $(TEST_DIR)/issue_65/issue_65.peg $(BINDIR)/pigeon | |||
| $(BINDIR)/pigeon -nolint -optimize-grammar $< > $@ | |||
|
|
|||
| $(TEST_DIR)/issue_70/issue_70.go: $(TEST_DIR)/issue_70/issue_70.peg $(BINDIR)/pigeon | |||
There was a problem hiding this comment.
Probably a mistake when fixing the makefile conflict. Added them back.
ast/ast.go
Outdated
| @@ -166,7 +166,7 @@ func (a *ActionExpr) Pos() Pos { return a.p } | |||
|
|
|||
| // String returns the textual representation of a node. | |||
| func (a *ActionExpr) String() string { | |||
| return fmt.Sprintf("%s: %T{Expr: %v, Code: %v}", a.p, a, a.Expr, a.Code) | |||
| return fmt.Sprintf("%s: %T{Expr: %v, Code: %v, FuncIx: %d, ID: %p}", a.p, a, a.Expr, a.Code, a.FuncIx, a) | |||
|
@mna Thank Martin, LGTM. |
This fixes #70 and an additional issue with optimization where a clone was missing, resulting in build errors on otherwise valid grammars.
This includes the changes in #71 .