[PATCH] D48680: Add missing visibility annotation for __base

manojgupta added a comment. Thanks a lot! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48680/new/ https://reviews.llvm.org/D48680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D48680: Add missing visibility annotation for __base

This revision was automatically updated to reflect the committed changes. Closed by commit rG30ccc2e8d24b: [libc++] Add missing visibility annotation for __base (authored by yunlian, committed by ldionne). Herald added a project: libc++. Herald added a reviewer: libc++. Changed prior to commit:

[PATCH] D48680: Add missing visibility annotation for __base

manojgupta added a comment. Friendly ping @pcc and @ldionne . We have been carrying this patch I think for too long now. Also, we have not discovered any LTO issues elsewhere so can't tell from our side if there are other places in libc++ that need visibility annotations. Repository: rCXX

[PATCH] D48680: Add missing visibility annotation for __base

pcc added a comment. In D48680#1587967 , @ldionne wrote: > @pcc In your reproducer, what is `~/l3/ra-cmake/bin/clang++`? That's just clang built from trunk at the time that I posted my comment. > Are you able to reproduce without `-fuse-ld=lld`? I'm

[PATCH] D48680: Add missing visibility annotation for __base

ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. Herald added a subscriber: dexonsmith. @pcc In your reproducer, what is `~/l3/ra-cmake/bin/clang++`? Are you able to reproduce without `-fuse-ld=lld`? I'm trying to reproduce

[PATCH] D48680: Add missing visibility annotation for __base

pcc added a comment. I think it would be up to the libc++ maintainers to approve the patch. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48680/new/ https://reviews.llvm.org/D48680 ___ cfe-commits mailing list

[PATCH] D48680: Add missing visibility annotation for __base

manojgupta added a comment. @pcc can you please submit this patch if there are no objections? Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48680/new/ https://reviews.llvm.org/D48680 ___ cfe-commits mailing list

[PATCH] D48680: Add missing visibility annotation for __base

pcc added a comment. The same problem occurs whether or not `exe.cpp` is compiled with`-fno-exceptions -fno-rtti`. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48680/new/ https://reviews.llvm.org/D48680 ___

[PATCH] D48680: Add missing visibility annotation for __base

EricWF added a comment. You have an ODR violation. You can't compile one TU with `-fno-rtti -fno-exceptions` and another without. You give the definitions of `__base` different vtables. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48680/new/

[PATCH] D48680: Add missing visibility annotation for __base

manojgupta added a comment. @ldionne Does Peter's example answer your questions? Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48680/new/ https://reviews.llvm.org/D48680 ___ cfe-commits mailing list

[PATCH] D48680: Add missing visibility annotation for __base

pcc added a comment. Here's a standalone reproducer for the problem: $ cat exe.cpp #include std::function f(); int main() { f()(); } $ cat dso.cpp #include __attribute__((visibility("default"))) std::function f() { return [](){}; } $

[PATCH] D48680: Add missing visibility annotation for __base

ldionne added a comment. More specifically, the following program always causes the vtable for `__base` to be internal: cat < int go(float) { return 0; } std::function foo() { return go; } int main() { foo()(3.3f); } EOF That's true with or without this patch, and with or

[PATCH] D48680: Add missing visibility annotation for __base

ldionne added a comment. What I don't understand is under which circumstances this changes anything, since we don't export `__base` from the dylib, and implicit instantiations of `__base` don't cause it to be exported. Can you please provide a sample program where this changes what's exported,

[PATCH] D48680: Add missing visibility annotation for __base

manojgupta added a comment. Herald added a subscriber: libcxx-commits. Hi Peter and Marshall, Yunlian has moved to a different project. Can you let me know what is missing in this patch so that it can be submitted. Repository: rCXX libc++ CHANGES SINCE LAST ACTION

[PATCH] D48680: Add missing visibility annotation for __base

pcc added a comment. This change ensures that __func receives public LTO visibilty: https://clang.llvm.org/docs/LTOVisibility.html if a translation unit is compiled with `-fvisibility=hidden` and without `_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS` defined (i.e. dynamically linking against libc++).

[PATCH] D48680: Add missing visibility annotation for __base

mclow.lists added a comment. Sorry - what problem is this solving? Repository: rCXX libc++ https://reviews.llvm.org/D48680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48680: Add missing visibility annotation for __base

yunlian created this revision. Herald added subscribers: cfe-commits, ldionne. This adds missing visibility annotation for __base. Repository: rCXX libc++ https://reviews.llvm.org/D48680 Files: include/functional Index: include/functional