search

goccmack / gocc

Parser / Scanner Generator
Other
622 stars 48 forks source link

gen: fix imports in source template #86

Closed kortschak closed 5 years ago

kortschak commented 5 years ago

See https://github.com/goccmack/gocc/pull/83#issuecomment-470014589.

Please take a look. Apologies for not noticing this earlier.

awalterschulze commented 5 years ago

Can we also fix travis to catch this in future

On Wed, 6 Mar 2019 at 08:32, Dan Kortschak notifications@github.com wrote:

See #83 (comment) https://github.com/goccmack/gocc/pull/83#issuecomment-470014589.

Please take a look. Apologies for not noticing this earlier.

You can view, comment on, or merge this pull request online at:

https://github.com/goccmack/gocc/pull/86 Commit Summary

  • gen: fix imports in source template

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/goccmack/gocc/pull/86, or mute the thread https://github.com/notifications/unsubscribe-auth/ABvsLY1li7TbwUiEELytaXyefsQDO2E0ks5vT30QgaJpZM4bgUqL .

kortschak commented 5 years ago

I found it by running our gonum dot parser tests after applying the new code gen. Are there complete runnable code gen examples in the repo that can be leveraged for testing?

kortschak commented 5 years ago

Actually, they don't need to be runnable, just buildable.

kortschak commented 5 years ago

I have looked through the examples and none of those are affected by the breakage that this fixes. Finding the breakage appears to depend on including parse stack and error reporting. I'm not entirely clear how this in included.

/cc @mewmew

mewmew commented 5 years ago

I'm not entirely clear how this in included.

/cc @mewmew

I am not too familiar with those parts of the Gocc code. Perhaps @sangisos remembers? Alex, if I remember correctly, you looked at error reporting last time we played with this? Though that was some time ago :)

awalterschulze commented 5 years ago

Maybe I am missing something, but currently we run

make install
make regenerate
make govet
make goimports
make errcheck
make test
git diff --exit-code .

as part of make travis Is is possible to add something as simple as go build ./... to catch this error?

kortschak commented 5 years ago

The issue is that the current examples do not incorporate the parser error code from those template. The Gonum dot parser does. This means that running that check does not fail here while it does in the Gonum generated code.

A possibility is to include the Gonum code which is CC0 and written by @mewmew, then run the test above.

awalterschulze commented 5 years ago

Or to include a very small example, that uses the error code.

awalterschulze commented 5 years ago

For now, I think we can merge this.