lingua-franca
lingua-franca copied to clipboard
Add support for the `link-libs` target property
Fixes #1120.
I'm not sure how to update the reference to the benchmarks to point here: lf-lang/benchmarks-lingua-franca#11
@lhstrh do you happen to know how this can be done?
Incidentally, the I actually do not understand why the LSP tests are failing in this branch. I tried to reproduce the Python validation test failure on my machine but could not, and instead, I had a test failure in Rust validation tests, which I should think would be unaffected by these changes. I thought that the random seed was set up so that this would not happen. I wonder if the LSP tests should be disabled temporarily until I find a way to make them less complex.
EDIT: The LSP test failures are no longer mysterious. I know not why the error went away.
I would like to point out that there is already a field compileLibraries
in TargetConfig
. There appears to be a proliferation of competing mechanisms...
One odd thing about this design is that m
isn't, in fact, the library. The library is math
. It's just that the common compiler flag to link it is m
. I think that if we were to generalize, we should use the library names...
Also, it appears that math is actually the only standard library that requires an extra flag (at least with gcc), so I think we've already created excess generality with this target property. It seems that a property link-math
could possibly be sufficient. I'm now running tests to see what flags are really needed for standard libraries across the platforms we support.
I think the only reasonable thing to do is to not make this a feature of 0.2.0
and first make sure this solution is solid. We can always publish a patch version that includes this feature.
I think that handling special cases in this way is appropriate and could lead to an overall better user experience. Use of the math library is very common in C programs, and it really unfortunate that the it has to handled differently on different platforms. But once we understand those needs, why leave it users to have rediscover these quirks themselves?
The problem that I see is that this knowledge base will never be complete, and it needs to be maintained (as libraries and their usage change over time). If we promise users that they can just specify the library they want to link to and it works on any platform, then we need to fulfill this promise somehow. The effort required to do this seems extensive to me. If we really want to go down that path, we should at least be transparent about precisely which libraries are supported. We should have tests for every library we claim to support and issue validation errors (or at least warnings) if users try to use the target property for linking other libraries.
I see that there has been a lively discussion here in my absence. I apologize for my late response.
In terms of a special handling for libm
, the unfortunate situation is that some systems require the -lm
flag and some don't. This goes back to older days where some implementations of the C standard library (libc
) separated out the math functions' implementation into a separate library. Newer implementations of libc
seem to already incorporate the math functions, so there is no need to link against libm
. Most newer compilers, including GCC and Clang, just ignore the -lm
if you pass it, but, unfortunately, the MSVC compiler (which is a compiler that we support) patently refuses to compile if libm
is linked.
If you want to use the math functions in your code and ensure maximum compatibility, you need to add the following to your cmake-includes:
IF (NOT WIN32)
find_package(m REQUIRED)
target_link_libraries(${LF_MAIN_TARGET} m)
ENDIF()
The math library is really an oddity in the C standard library. As far as I know, there is no other standard library for C that needs to be specifically linked. Hence, I think it was justified to provide an easy shortcut for this in the form of link-libraries: ["m"]
, as suggested originally in #1120. An alternative would be to provide a built-in mlib.cmake
to allow for cmake-include: ["mlib.cmake"]
.
In terms of the usefulness of link-libraries
in general, it is really a shorthand for the following:
- If
cmake: false
, pass the specified libraries via the-l
flag. - If
cmake: true
, pass the specified libraries viafind_library( LF_FOO_LIB foo ) target_link_libraries( ${LF_MAIN_TARGET} "${LF_FOO_LIB}")
This is providing a shorthand for a scenario that can arise many times. Note that this mechanism supports cmake: false
as well. Of course, cmake-include will always be there if a more advanced situation arises (although, you would have to leave cmake: false
behind). Think of link-libraries
as the easy mode for a common pattern.
Finally, I envisioned link-libraries
as a method that also allows for inclusion of our own utility libraries (e.g., link-libraries: ["deque"]
), but this could wait until a future PR.
I see that the the link-libraries
is renamed to link-std-libs
. This sounds wrong to me. I think if we want to keep it, the link-libraries
name sounded more appropriate.
My suggestion would be to wait with finalizing this until @cmnrd returns from his vacation so we can reach agreement on what the final solution should look like.
Tagging @cmnrd now that he's back to see if we can reach a consensus. @cmnrd, please see my comment above.
With regard to the math library, why not add -lm
by default? Is there a case where we really don't want to add the math library?
Ok, so if we see link-libraries
as a shortcut for a specific piece of cmake code, then I think it is reasonable to include it. We should then specifically describe in the documentation which cmake code it produces, in which scenarios it works, and how the user can add manual cmake code when it doesn't work. From the initial proposal, I understood link-libraries
as a general solution to the problem of finding and linking dependencies, which I don't think it is possible. If we label this as a limited solution that works in some (or many?) cases, I am fine with including it.
The c-benchmark tests will fail because lf-lang/benchmarks-lingua-franca#11 is not merged yet. I have no idea how to proceed. Could you help me @lhstrh, please?