mstorsjo edited subscribers, added: libcxx-commits, mstorsjo; removed: llvm-commits, cfe-commits. mstorsjo added a comment.
I see that @smeenai brought up the inconsistency between `_DLL` and `_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS` already back when this was reviewed. I'm running into problems with this inconsistency in various cases. `_DLL` is tied to the choice of CRT (compiling with `-MD` defines it, compiling with `-MT` doesn't define it), while we might have built and be using either a statically linked or dynamically linked libc++. And as @smeenai argued, if `_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS` is not defined, then all declarations are decorated with dllimport, and there's no doubt about it, we have to use the DLL form of the library. This is noticable today when running the libc++ testsuite (which is hooked up in CI these days, so by posting a patch you can get it verified too!); currently the CI configuration builds with both shared and static enabled (which links and uses the shared library only). If disabling building the static library, however, testing fails, as the autolinking tries to pull in the static library. (We could disable autolinking within the test suite altogether, as it explicitly links the right library anyway, but I'd rather fix the autolinking feature instead of having to disable it because it misbehaves.) I guess it's possible to set up cases with `_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS` disabled but still linking against the DLL, that might work at least for some subsets of the library functionality. But if manually enabling `_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS` in a DLL configuration, then one can also define `_LIBCPP_NO_AUTO_LINK` at the same time to opt out of the autolinking. Therefore, I would like to post a patch that fixes the autolinking mechanism to rely on `_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS` which is closely tied to whether dllimport is used, and to whether a shared or static library was built. (And if both were built, it defaults to the shared one.) WDYT @smeenai @compnerd? Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40660/new/ https://reviews.llvm.org/D40660 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
