Crystal: Request on improving DL module

[hyronx]

Hi,

I'm playing around with this module and it currently has really not many features.
So I want to show you my ideas on how to improve it.

1. A new name
_DL_ is not really meaningful if you don't know the dl library and even then it could be something different. Renaming it to _DynamicLibrary_ or _DynLib_ would make it more clear what this module is about.

2. Renaming the functions
With the first point it wouldn't be a problem to rename _dlopen_ to just _open_ and later (when implemented) also using _symbol_ or _sym_ instead of _dlsym_.

3. Changing the module to a class
Instead of passing the Pointer(Void) around to a symbol method or the like, the class could store it in an instance variable making it easier to load multiple symbols and, in the finalize method, could automatically close the library:

(In my opinion _DynamicLibrary_ is a more suitable name)
my_lib = DynamicLibrary.new "my_lib", flags
my_func = my_lib.symbol "my_func"
my_func2 = my_lib.symbol "my_func2

instead of

handle = DynLib.open "my_lib", flags
my_func = DynLib.symbol handle, "my_func"
my_func2 = DynLib.symbol handle, "my_func2"
DynLib.close handle

I could create a new feature branch and implement this stuff together with the additional methods. :)

Sorry for any English mistakes...

[jhass]

Your suggestions look good to me, the bigger question is whether it's actually useful enough to stay in the standard library, or whether we should move it into its own shard.

There is one usage of it in the compiler, https://github.com/crystal-lang/crystal/blob/master/src/compiler/crystal/codegen/codegen.cr#L25 -> https://github.com/crystal-lang/crystal/blob/master/src/compiler/crystal/program.cr#L437 but I don't understand its purpose at all, it feels like it tries to verify existence of libraries mentioned in @[Link()] attributes, but there's no evaluation of the return value _at all_, neither in the linked code nor in DL itself. https://github.com/crystal-lang/crystal/commit/02f721bbeaed introduced it, I really don't understand what effect this is supposed to have. @asterite do you remember why you did this?

[hyronx]

Well, I would use it for a plug-in system but otherwise there aren't many more use cases I can think of... You could be right to kick it out.

[asterite]

DL is in the standard library only because we needed that for the compiler's specs. That's why it's very poor and undocumented. We should either improve it or remove it from the std (maybe keeping it is good).

When doing some compiler specs we use LLVM's JIT. If the test program needs an extra library, because the compiler wasn't linked with that library it won't find it in the current binary. That's why we use dlopen to load it dynamically in the current executable, so that the specs can find it.

That said, maybe we needed that before some libraries like pcre were included in the distributed binary, because I tried to remove that load_libs call and it worked just fine.

[jhass]

So there's no usage left now, right? I would vote to drop it from stdlib then.

[ysbaddaden]

Note that we do use the dladdr syscall.

[jhass]

Yes, but not in a way that would benefit from a good DL API, which would be centered on opening a .so/.dylib, not reading a symbol of the current binary, right?

[ozra]

It has crossed my mind a couple of times that DL should be renamed, so agree with @hyronx. And with the intell @jhass and @asterite brought up, that it's dropped from stdlib. But since it's "core close stuff", maybe it _should_ be available, and then the suggested improvements would be nice.

[david50407]

+1 for moving to its own shard.

BTW, dynamic loading library is depends on the platform OS, so is there any way to require target-specific codes like Crystal compiler auto adds lib_c/<target>/ into the require path?

[hyronx]

I've just created a little implementation as a possible reference or whatever. Contains specs and is documented.

@david50407 Target-specific code would be great, I agree. But for this module it isn't really necessary as long as there is no Windows implementation.

[ysbaddaden]

@david50407 the libc target is injected into CRYSTAL_PATH because there are many files to be required for many targets from many different places. This is a special case, and shall eventually disappear when crystallib will be capable to generate the bindings on the fly.

In the case here, a couple ifdef would be enough.

[david50407]

@hyronx I think Windows' dynamic library API cannot be mapped to UNIX's syscall easily, but yes we don't have windows right now. 😄

[hyronx]

@david50407 Windows has a LoadLibrary function which has some other flags but otherwise does the same like dlopen and GetProcAddress is comparable to dlsym so it wouldn't be so hard to implement. But no Windows so unimportant.. :D

[bew]

I'd suggest to move DL to the internals of the compiler, similar to the Markdown module, since it's not documented nor has a good api