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

Hey again, got another clang/libc++ issue but I think this one should be a lot simpler than my other one. I notice here there is some logic for determining which filesystem library needs to be linked.

From the clang docs :

Prior to LLVM 9.0, libc++ provides the implementation of the filesystem library in a separate static library. Users of and <experimental/filesystem> are required to link -lc++fs. Prior to libc++ 7.0, users of <experimental/filesystem> were required to link libc++experimental.

So incidentally, these lines suggest that for llfio_dl we may be doing the right thing for the wrong reason in some cases.

This is a bit tedious to be sure, but not too terrible. I've had to patch a CML.txt to handle this in the past. Once you've established that clang/libcxx is in use you should be able to use COMPILER_VERSION_LESS to pick if you need c++experimental , c++fs , or nothing at all.

I don't see it linking a libc++ edition of LLFIO with stdc++fs on Linux?

Sure, so I just pulled in e90847a which appears to have the filesystem linkage changes. And I got a fresh build going where CMAKE_CXX_COMPILER points to a clang++-9 install and CMAKE_CXX_FLAGS has stdlib=libc++ just to verify that stdc++fs is still appearing in the build.ninja for the llfio targets and in the llfio_<...>_LIB_DEPENDS cache entries.

My analysis of the flow is as follows.

Starting from if(CMAKE_SYSTEM_NAME MATCHES "Linux") , we look for stdc++fs . The path to this library is stored in libstdcxx_stdcxxfs if found, else libstdcxx_stdcxxfs is set to -lstdc++fs . So unconditionally, the variable points to stdc++fs in some form or another.

Next there is get_target_property(val llfio_dl USING_LIBCXX_ON_LINUX) . Skimming some quickcpplib code , this appears to get set if we've applied coroutines to llfio_dl . If this property is NOTFOUND , then we do

all_link_libraries(PUBLIC ${libstdcxx_stdcxxfs} rt)
The else of the NOTFOUND check above then leads to
target_link_libraries(${PROJECT_NAME}_hl INTERFACE ${libstdcxx_stdcxxfs} rt)
target_link_libraries(${PROJECT_NAME}_sl PUBLIC ${libstdcxx_stdcxxfs} rt)
(and the same for the special builds), and then we link libc++ and libc++abi. But it's too late, as we've also already linked GNU's fs lib.
Thus:
libstdcxx_stdcxxfs either points to the path to stdc++fs, or is set to -lstdc++fs; and
we use libstdcxx_stdcxxfs in a call to either all_link_libraries or target_link_libraries for the sl and dl variant and their specials.
In other words, all paths lead to linking stdc++fs in some form or another, and this logic is guarded by nothing but checking if we're on Linux. Any checking for clang or clang version never takes place. And also before all this happens we have set(USING_LIBCXX_ON_LINUX 0). That variable is never branched on to evaluate FS linkage, but it sort of makes it seem like we were off to a bad start
          
@ned14 LGTM, thank you! did a fresh build, no mention whatsoever of c++fs, c++experimental, or stdc++fs in my cache or build.ninja :)
At this point my use case is perfectly handled, so the following is just an FYI--I notice c++fsor c++experimental are linked iff found. I think if a user has an older clang install kicking around on their path but has configured this build for clang/libc++ >= 9 it's possible c++fs and/or c++experimental will still be linked, although I suspect this is probably harmless. But if you do want to handle that scenario (or preempt the library search entirely in some cases) I think it would suffice to use the version check logic you wrote for the previous commit.