Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parser portability issues #30

Closed
ljanyst opened this issue Jun 29, 2022 · 7 comments
Closed

Parser portability issues #30

ljanyst opened this issue Jun 29, 2022 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@ljanyst
Copy link

ljanyst commented Jun 29, 2022

Hello everyone,

Thanks for developing this new IMAP library and improving the bridge!

I have noticed that you generate ANTLR C++ parser to process the IMAP commands and this results in a need to pull some static libraries and do some heavy lifting to communicate between go and C++. I currently run the bridge on a bunch of architectures that you don't normally support. It's normally pretty easy with pure go, but quite a pain if you need to interface with C++.

ANTLR seems to be capable of generating go directly without having to go through C++: https://github.com/antlr/antlr4/blob/master/doc/go-target.md. Please consider using that instead. It should significantly simplify your integration and testing process too.

@jameshoulahan
Copy link
Contributor

jameshoulahan commented Jul 5, 2022

Hi @ljanyst, thanks for your interest in the library and in our products!

I definitely understand your concerns regarding portability and unnecessary complexity. Indeed, we used ANTLR's go target earlier in this project's history, and we already use it in another library used by bridge, see go-rfc5322, but we were forced to switch to C++ because the generated go code was horribly slow.

Longer term, we are interested in investigating better solutions to parsing the IMAP commands, but for now, we will stick with the current approach, as it is the simplest way to get a high quality parser for all of our officially supported architectures.

@ljanyst
Copy link
Author

ljanyst commented Jul 5, 2022

Thanks for taking a look!

@kortschak
Copy link

@jameshoulahan Did you take a look at using ragel for this?

@jameshoulahan
Copy link
Contributor

@kortschak no I didn't, I wasn't aware of it at the time. What would be the benefits of using it?

@kortschak
Copy link

It's significantly simpler than antlr.

@LBeernaertProton LBeernaertProton self-assigned this Feb 10, 2023
@LBeernaertProton LBeernaertProton added the enhancement New feature or request label Feb 10, 2023
@LBeernaertProton
Copy link
Contributor

To all those who are interested, work has begun on rewriting the C++ parser dependencies in pure Go.

@LBeernaertProton
Copy link
Contributor

As of c8f22fc, Gluon no longer depends on C++ libraries. The issue is now considered resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants