jyknight added inline comments.
================ Comment at: lld/ELF/InputFiles.cpp:402 + if (Config->DepLibs) + if (fs::exists(Specifier)) + Driver->addFile(Specifier, /*WithLOption=*/false); ---------------- bd1976llvm wrote: > jyknight wrote: > > bd1976llvm wrote: > > > jyknight wrote: > > > > This bit seems unfortunate. "-lfoo" won't search for a file named > > > > "foo", and having this code do so does not seem like a good idea. I'd > > > > want this feature to work just like -l. > > > > > > > > Actually, I think it'd be better to preserve the "-l" prefix in the > > > > deplibs section. (I'd note that even if libs in the search path are > > > > spelled "-lfoo", that doesn't mean we need accept any other options) > > > > > > > > If we want to support loading a filename which not on the search path > > > > (but...do we, even? That seems like a kinda scary feature to me...), > > > > then it'd be obvious how to spell that: "libfoo.a" and "/baz/bar/foo.a" > > > > would open files directly, vs "-lfoo" and "-l:foo.a" would search for > > > > them on the search path. > > > What you have described is the behavior of the INPUT linker script > > > command: > > > https://sourceware.org/binutils/docs/ld/File-Commands.html#File-Commands. > > > > > > We have carefully designed this "dependent libraries" feature to avoid > > > mapping "comment lib" pragmas to --library command line options. My > > > reasons: > > > > > > - I don't want the compiler doing string processing to try to satisfy the > > > command line parsing behaviour of the target linker. > > > > > > - I don't want to couple the source code to a GNU-style linker. After all > > > there are other ELF linkers. Your proposal isn't merely an ELF linking > > > proposal, it's a proposal for ELF toolchains with GNU-like linkers (e.g. > > > the arm linker doesn't support the colon prefix > > > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474c/Cjahbdei.html). > > > > > > - The syntax is #pragma comment(lib, ...) not #pragma > > > linker-option(library, ...) i.e. the only thing this (frankly rather > > > bizarre) syntax definitely implies is that the argument is related to > > > libraries (and comments ¯\_(ツ)_/¯); it is a bit of a stretch to interpret > > > "comment lib" pragmas as mapping directly to "specifying an additional > > > --library command line option". > > > > > > - I want to avoid GNUism's and use a "general" mechanism. MSVC source > > > code compatibility is a usecase. > > > > > > - It is important to support loading a filename which not on the search > > > path. For example I have seen developers use the following: #pragma > > > comment(lib, __FILE__"\\..\\foo.lib") > > > > > > I would like the following to work on for ELF: > > > > > > #pragma comment(lib, "foo") => add libfoo.a to the link. > > > #pragma comment(lib, "foo.lib") => add foo.lib to the link > > > #pragma comment(lib, "c:\\foo.lib") => add c:\foo.lib to the link > > > > > > The way the code is now these will work on both LLD and MSVC (and I think > > > it will work for Apple's linker as well although I haven't checked). In > > > addition, we have been careful to come up with a design that can be > > > implemented by the GNU linkers... with the cost that some complicated > > > links will not be possible to do via autolinking; in these (IMO rare) > > > cases developers will need to use the command line directly instead. > > > > > > What I am trying to accomplish is to try to keep #pragma comment(lib, > > > "foo") compatible across linkers. Otherwise we will be in a situation > > > where you will have to have the equivalent of... > > > > > > #ifdef COFF_LIBRARIES: > > > #pragma comment(lib, "/DEFAULTLIB:foo.lib") > > > #'elif ELF_LIBRARIES_GNU_STYLE_LINKER: > > > #pragma comment(lib, "-lfoo") > > > #'elif DARWIN_FRAMEWORKS: > > > #pragma comment(lib, "-framework foo") > > > #esle > > > #error Please specify linking model > > > #endif > > > > > > ... in your source code (or the moral equivalent somewhere else). We can > > > avoid this. > > > > > > Also note that I am not proposing to remove the .linker-options > > > extension. We can add support for .linker-options to LLD in the future if > > > we find a usecase where developers want pragmas that map directly to the > > > linkers command line options for ELF. If this is a usecase then, in the > > > future, we can implement support for #pragma comment(linker, ....) > > > pragmas that lower to .linker-options; #pragma comment(lib, ...) pragmas > > > can continue to lower to .deplibs. > > It just seems to me a really bad idea to have a situation where specifying > > `#pragma comment(lib, "foo")` can either open a file named "foo" in the > > current directory, or search for libfoo.{a,so} in the library search path. > > > > That is pretty much guaranteed to cause problems due to unintentional > > filename collision in the current-directory. > > > Linkers already have the behaviour of finding the first matching file in the > library search paths, and gnu-ld at least seems to add "." implicitly as the > first library search path. So, developers already need to make sure that they > don't have multiple copies of a library in the wrong place. This behaviour > doesn't seem “too much” of a deviation from that. In other words I suspect > that the chance of having a library named foo in the current directory and a > library named libfoo.a in a library search path is remote, no? > > Would your concerns be alleviated by changing the precedence, like so: > > ``` > static void processDepLib(StringRef Specifier, const InputFile *F) { > if (!Config->DepLibs) > return; > else if (Optional<std::string> S = searchLibrary(Specifier)) > Driver->addFile(*S, /*WithLOption=*/true); > else if (Optional<std::string> S = findFromSearchPaths(Specifier)) > Driver->addFile(*S, /*WithLOption=*/true); > if (fs::exists(Specifier)) > Driver->addFile(Specifier, /*WithLOption=*/false); > else > error(toString(F) + > ": unable to find library from dependent library specifier: " + > Specifier); > } > ``` > > We could also change this function to error if a given specifier would find a > library in more than one location (assuming that there was no early out)? > > gnu-ld at least seems to add "." implicitly as the first library search path I don't see that behavior. Neither GNU ld.bfd, GNU ld.gold, nor llvm's lld use "." as a default library search path as far as my testing can determine. Neither when invoked directly, nor when invoked through the gcc and clang drivers. > In other words I suspect that the chance of having a library named foo in the > current directory and a library named libfoo.a in a library search path is > remote, no? No, I don't think the chance is remote at all. I can easily imagine linking against a library named "clang", ("libclang.so"), while having a file or directory named "clang" in the current directory, for example. > Would your concerns be alleviated by changing the precedence No, not really. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60274/new/ https://reviews.llvm.org/D60274 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
