[PATCH] [libcxx] Visibility fixes for Windows

[Howard Hinnant]

On Aug 7, 2013, at 8:26 AM, Nico Rieck <nico.rieck at gmail.com> wrote:

> Hi,
> 
> this patch series fixes visibility issues on Windows as explained in <http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-August/031214.html>.
> 
> The first patch ensures that templates are not imported/exported by using the new macro _LIBCPP_TYPE_VIS_ONLY.
> The other two patches decorate public and internal (with "__" prefix) types and functions.
> 
> I will deal with the exception types later.

Thanks for your work on this.  If I understand correctly, types that exist only in the headers are decorated with _LIBCPP_TYPE_VIS_ONLY, and types that are compiled into the dylib are decorated with _LIBCPP_TYPE_VIS.

I like this direction.

I'm seeing a few places where I'm questioning the choice:

-------------
<__hash_table> lines 1163-1164:

    template <class, class, class, class, class> friend class _LIBCPP_TYPE_VIS unordered_map;
    template <class, class, class, class, class> friend class _LIBCPP_TYPE_VIS unordered_multimap;

should be _LIBCPP_TYPE_VIS_ONLY because these are templates.

-------------
<__tree> lines 1111-1112:

    template <class, class, class, class> friend class _LIBCPP_TYPE_VIS map;
    template <class, class, class, class> friend class _LIBCPP_TYPE_VIS multimap;

should be _LIBCPP_TYPE_VIS_ONLY because these are templates.

-------------
<cstddef>

struct _LIBCPP_TYPE_VIS nullptr_t

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

-------------

<ext/__hash>

template <typename T> struct _LIBCPP_TYPE_VIS hash : public std::hash<T>
template <> struct _LIBCPP_TYPE_VIS hash<const char*>
template <> struct _LIBCPP_TYPE_VIS hash<char *>

should be _LIBCPP_TYPE_VIS_ONLY because these are templates, or are always-inlined.

-------------

<future>

line 117

class _LIBCPP_TYPE_VIS promise<void>

the above line is a documentation comment, and should have no visibility tag.

-------------

<iterator>

struct _LIBCPP_TYPE_VIS input_iterator_tag {};
struct _LIBCPP_TYPE_VIS output_iterator_tag {};
struct _LIBCPP_TYPE_VIS forward_iterator_tag       : public input_iterator_tag {};
struct _LIBCPP_TYPE_VIS bidirectional_iterator_tag : public forward_iterator_tag {};
struct _LIBCPP_TYPE_VIS random_access_iterator_tag : public bidirectional_iterator_tag {};

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

-------------

<map>

    class _LIBCPP_TYPE_VIS value_compare

should be _LIBCPP_TYPE_VIS_ONLY because this is a templates (2 places, map and multimap).

-------------

<memory>

class _LIBCPP_TYPE_VIS allocator<void>
class _LIBCPP_TYPE_VIS allocator<const void>
class _LIBCPP_TYPE_VIS auto_ptr<void>

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

-------------

<mutex>

struct _LIBCPP_TYPE_VIS once_flag

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

--------------

<random>

class _LIBCPP_TYPE_VIS seed_seq
class _LIBCPP_TYPE_VIS bernoulli_distribution

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

--------------

<regex>

class _LIBCPP_TYPE_VIS __match_any_but_newline

Should have no visibility tag because it is an inlined implementation detail.

--------------

<string>

struct _LIBCPP_TYPE_VIS char_traits<char>
struct _LIBCPP_TYPE_VIS char_traits<wchar_t>
struct _LIBCPP_TYPE_VIS char_traits<char16_t>
struct _LIBCPP_TYPE_VIS char_traits<char32_t>

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

----------------

<system_error>

struct _LIBCPP_TYPE_VIS is_error_condition_enum<errc>
struct _LIBCPP_TYPE_VIS is_error_condition_enum<errc::__lx>
struct _LIBCPP_TYPE_VIS hash<error_code>

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

---------------

<thread>

class _LIBCPP_TYPE_VIS __thread_id;
template<> struct _LIBCPP_TYPE_VIS hash<__thread_id>;
class _LIBCPP_TYPE_VIS __thread_id
    friend struct _LIBCPP_TYPE_VIS hash<__thread_id>;
struct _LIBCPP_TYPE_VIS hash<__thread_id>

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

class _LIBCPP_TYPE_VIS __thread_id;  // harmless 2nd declaration should just be erased

---------------

<tuple>

struct _LIBCPP_TYPE_VIS allocator_arg_t { };
class _LIBCPP_TYPE_VIS tuple<>

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

---------------

<typeindex>

class _LIBCPP_TYPE_VIS type_index
struct _LIBCPP_TYPE_VIS hash<type_index>

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

---------------

<utility>

struct _LIBCPP_TYPE_VIS piecewise_construct_t { };

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

---------------

<valarray>

class _LIBCPP_TYPE_VIS slice

should be _LIBCPP_TYPE_VIS_ONLY because this is never compiled into the dylib.  Everything is always inlined.

----------------

If you agree with these changes, I have them already made and can go ahead and commit them.  Otherwise we can iterate further until we are all satisfied.  I'm attaching a patch of what I have so that there is no ambiguity.

Also could you please prepare a patch for CREDITS.TXT?

Thanks!

Howard

-------------- next part --------------
A non-text attachment was scrubbed...
Name: temp.patch
Type: application/octet-stream
Size: 212917 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130808/7bb780a4/attachment.obj>