-
Notifications
You must be signed in to change notification settings - Fork 12
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
Refactor/libs #265
Refactor/libs #265
Conversation
Replace with extensions array as at least with macos we need to check for .so and .dylib
@fodinabor Can you have a look. The |
@fodinabor bump |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay...
I am somewhat struggling to see the point in having the plugin as a (shared) library. For the gtest suite, there are the "whitebox" automaton tests, which could just as well link the automaton lib statically (I just was to lazy to make it a static lib) as could the regex plugin. When the regex plugins' passes are needed in the gtest suite, it loads the module (.so) anyways via the usual mechanics.
That is not to say that some of the changes might make sense nevertheless, but I don't really see a benefit of having the option for plugins to be compiled as either or..
Well, one part of my changes is to have the automaton lib as an ordinary lib (which is shared by default but you could also make it static - wouldn't change much). The bigger change is about allowing plugins as a |
I would've proposed to link both |
* gtest/automaton.cpp: regex2nfa is now dynamically loaded * one test case needs still porting
As far as I can tell, lib automaton is not the issue here. It doesn't matter whether we link statically or dynamically, no? I tried my latest commit to dynamically load |
Yeah, that's why I'd propose to have that statically linked, so we don't care about c linkage 🤔 |
This returns a raw pointer that you have to pack again into a unique_ptr
dfa2matcher and regex2nfa are not passes
Moves automaton into its dedicated lib and avoids double compilation.
thorin/plug/regex/automaton
->automaton
SHARED
to also link normally against a pluginthorin-regex-gtest
makes use of this feature