[PATCH] D27430: [libc++] Annotate template methods with visibility
smeenai abandoned this revision. smeenai added a comment. Folded into https://reviews.llvm.org/D25208 https://reviews.llvm.org/D27430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27430: [libc++] Annotate template methods with visibility
smeenai added a comment. In https://reviews.llvm.org/D27430#634882, @smeenai wrote: > In https://reviews.llvm.org/D27430#634880, @smeenai wrote: > > > Sounds good to me for `_LIBCPP_HIDDEN`, but as far as I understand, for the > > `inline _LIBCPP_INLINE_VISIBILITY` cases, at least the `inline` needs to be > > at the definition itself, otherwise it won't have any effect. > > > Err this might actually have just been me being stupid. Lemme confirm that > first. Yup, I'm dumb; ignore me. I'll move everything to the declarations. https://reviews.llvm.org/D27430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27430: [libc++] Annotate template methods with visibility
smeenai added a comment. In https://reviews.llvm.org/D27430#634880, @smeenai wrote: > Sounds good to me for `_LIBCPP_HIDDEN`, but as far as I understand, for the > `inline _LIBCPP_INLINE_VISIBILITY` cases, at least the `inline` needs to be > at the definition itself, otherwise it won't have any effect. Err this might actually have just been me being stupid. Lemme confirm that first. https://reviews.llvm.org/D27430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27430: [libc++] Annotate template methods with visibility
smeenai added a comment. In https://reviews.llvm.org/D27430#632652, @EricWF wrote: > Please put these attributes to the first declaration instead of the > definition. Sounds good to me for `_LIBCPP_HIDDEN`, but as far as I understand, for the `inline _LIBCPP_INLINE_VISIBILITY` cases, at least the `inline` needs to be at the definition itself, otherwise it won't have any effect. For those, do you want to keep the `_LIBCPP_INLINE_VISIBILITY` at the declaration and the `inline` at the definition? Comment at: include/thread:392 template +_LIBCPP_HIDDEN thread::thread(_Fp __f) EricWF wrote: > We really should hide this using `inline _LIBCPP_INLINE_VISIBILITY` because > it's a special C++03 symbol, so we don't even want a hidden definition > omitted ideally. Will do. https://reviews.llvm.org/D27430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27430: [libc++] Annotate template methods with visibility
EricWF added a comment. Please put these attributes to the first declaration instead of the definition. Comment at: include/thread:392 template +_LIBCPP_HIDDEN thread::thread(_Fp __f) We really should hide this using `inline _LIBCPP_INLINE_VISIBILITY` because it's a special C++03 symbol, so we don't even want a hidden definition omitted ideally. https://reviews.llvm.org/D27430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27430: [libc++] Annotate template methods with visibility
smeenai requested a review of this revision. smeenai added a comment. I think this actually stands well on its own. I'll do the extern template annotations in a different diff. Also, ping :) https://reviews.llvm.org/D27430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27430: [libc++] Annotate template methods with visibility
smeenai planned changes to this revision. smeenai added a comment. Hidden visibility also requires extern template declarations to expand to default visibility (as opposed to just default type visibility) to be feasible. Will fix annotations there and add those to this diff. https://reviews.llvm.org/D27430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27430: [libc++] Annotate template methods with visibility
smeenai created this revision.
smeenai added reviewers: EricWF, mclow.lists.
smeenai added subscribers: cfe-commits, thakis.
https://reviews.llvm.org/D25208 switches `_LIBCPP_TYPE_VIS` to expand to
default visibility
instead of default type visibility, in order to ease building libc++
with hidden visibility. With that change made, any template methods of a
class marked `_LIBCPP_TYPE_VIS` will also get default visibility when
implicitly instantiated, which is not desirable for clients of libc++
headers who wish to control their visibility; a similar issue led to
PR30642. Annotate all problematic methods with an explicit visibility
specifier in order to avoid this issue.
The problematic methods were found by applying https://reviews.llvm.org/D25208
and then running
https://github.com/smeenai/bad-visibility-finder. The small methods were
marked for inlining; the larger ones marked hidden.
https://reviews.llvm.org/D27430
Files:
include/__locale
include/__mutex_base
include/condition_variable
include/future
include/mutex
include/thread
Index: include/thread
===
--- include/thread
+++ include/thread
@@ -389,6 +389,7 @@
}
template
+_LIBCPP_HIDDEN
thread::thread(_Fp __f)
{
Index: include/mutex
===
--- include/mutex
+++ include/mutex
@@ -252,6 +252,7 @@
};
template
+_LIBCPP_HIDDEN
bool
timed_mutex::try_lock_until(const chrono::time_point<_Clock, _Duration>& __t)
{
@@ -295,6 +296,7 @@
};
template
+_LIBCPP_HIDDEN
bool
recursive_timed_mutex::try_lock_until(const chrono::time_point<_Clock, _Duration>& __t)
{
Index: include/future
===
--- include/future
+++ include/future
@@ -588,6 +588,7 @@
};
template
+_LIBCPP_HIDDEN
future_status
__assoc_sub_state::wait_until(const chrono::time_point<_Clock, _Duration>& __abs_time) const
{
@@ -1716,6 +1717,7 @@
};
template
+_LIBCPP_HIDDEN
promise::promise(allocator_arg_t, const _Alloc& __a0)
{
typedef __assoc_sub_state_alloc<_Alloc> _State;
Index: include/condition_variable
===
--- include/condition_variable
+++ include/condition_variable
@@ -191,6 +191,7 @@
};
template
+_LIBCPP_HIDDEN
void
condition_variable_any::wait(_Lock& __lock)
{
@@ -212,6 +213,7 @@
}
template
+_LIBCPP_HIDDEN
cv_status
condition_variable_any::wait_until(_Lock& __lock,
const chrono::time_point<_Clock, _Duration>& __t)
Index: include/__mutex_base
===
--- include/__mutex_base
+++ include/__mutex_base
@@ -368,14 +368,16 @@
#ifndef _LIBCPP_HAS_NO_THREADS
template
+inline _LIBCPP_INLINE_VISIBILITY
void
condition_variable::wait(unique_lock& __lk, _Predicate __pred)
{
while (!__pred())
wait(__lk);
}
template
+_LIBCPP_HIDDEN
cv_status
condition_variable::wait_until(unique_lock& __lk,
const chrono::time_point<_Clock, _Duration>& __t)
@@ -386,6 +388,7 @@
}
template
+_LIBCPP_HIDDEN
bool
condition_variable::wait_until(unique_lock& __lk,
const chrono::time_point<_Clock, _Duration>& __t,
@@ -400,6 +403,7 @@
}
template
+_LIBCPP_HIDDEN
cv_status
condition_variable::wait_for(unique_lock& __lk,
const chrono::duration<_Rep, _Period>& __d)
Index: include/__locale
===
--- include/__locale
+++ include/__locale
@@ -162,6 +162,7 @@
}
template
+inline _LIBCPP_INLINE_VISIBILITY
locale
locale::combine(const locale& __other) const
{
@@ -316,6 +317,7 @@
};
template
+inline _LIBCPP_INLINE_VISIBILITY
bool
locale::operator()(const basic_string<_CharT, _Traits, _Allocator>& __x,
const basic_string<_CharT, _Traits, _Allocator>& __y) const
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
