[libcxx-commits] [PATCH] D62259: shared_ptr changes from library fundamentals (P0414R2)

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/memory:3912-3913
+    {
+        static_assert(!_VSTD::is_array<_Tp>::value,
+                      "std::shared_ptr<T>::operator-> is only valid when T is not an array type.");
+        return __ptr_;
----------------
@cedral wrote:
> using static_assert breaks the ability to include <memory> in a precompiled header and then explicitly define a shared pointer type for export. You should have used enable_if on the return type instead. [...] test.cpp will fail to compile because it attempts to instantiate the [] lookup operator for export. I don't think this is a bug. It needs to do this because it doesn't know which member functions will be called when the exported type is used by clients of the library.

On the one hand, I confirm your understanding of how `template class std::shared_ptr<int[]>` is treated by the compiler. https://godbolt.org/z/7bbasY83G
On the other hand, I cannot reproduce your issue with libc++ itself; the reason is that `_LIBCPP_INLINE_VISIBILITY` expands to a series of attributes including `__attribute__((__exclude_from_explicit_instantiation__))`. If you're seeing the problem only with PCHes, I wonder if there's a bad interaction between PCHes and that attribute.
Alternatively, are you using GCC (or anything else non-Clang)? It seems like GCC doesn't support that attribute, so I think on GCC you're just out of luck by design.

On the third hand, the language-lawyer answer is that your whole program is "ill-formed, no diagnostic required," and so libc++ isn't required to make it work anyway. Users aren't allowed to explicitly instantiate library types like that. One reason is that you don't know the name of the type — is it really `class std::shared_ptr`, or is it `struct std::shared_ptr`, or is `shared_ptr` just an alias for `__shared_ptr`, etc. etc. The more important reason is exactly what you already found out: When you explicitly instantiate a class template, you get //all// its members, including the private ones that you couldn't otherwise mess with, and which might well not be instantiable for your particular type. Example: https://godbolt.org/z/3hb7Wb3Te
Here's a real-world example from libstdc++: `template class std::atomic<void*>;` is a hard error. https://godbolt.org/z/eTq9TWqnK


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62259/new/

https://reviews.llvm.org/D62259