Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<vector>: std::vector<std::shared_ptr<mytype<T>>> fails to compile when T is incomplete #3842

Closed
iboB opened this issue Jun 30, 2023 · 2 comments · Fixed by #4373
Closed
Labels
bug Something isn't working fixed Something works now, yay!

Comments

@iboB
Copy link
Contributor

iboB commented Jun 30, 2023

Describe the bug

Creating a vector of shared_ptr of a template instantiated incomplete type fails to compile with type uses undefined class

Test case

Source:

#include <vector>
#include <memory>

template <typename T>
struct holder {
    T t;
};

class fwd;
using hptr = std::shared_ptr<holder<fwd>>;

std::vector<hptr> vec(5);

Output:

example.cpp
<source>(6): error C2079: 'holder<fwd>::t' uses undefined class 'fwd'
C:/data/msvc/14.36.32530-Pre/include\vector(2043): note: see reference to class template instantiation 'holder<fwd>' being compiled
C:/data/msvc/14.36.32530-Pre/include\vector(2033): note: while compiling class template member function 'void std::vector<hptr,std::allocator<hptr>>::_Tidy(void) noexcept'
C:/data/msvc/14.36.32530-Pre/include\vector(765): note: see the first reference to 'std::vector<hptr,std::allocator<hptr>>::_Tidy' in 'std::vector<hptr,std::allocator<hptr>>::~vector'
<source>(12): note: see the first reference to 'std::vector<hptr,std::allocator<hptr>>::~vector' in 'void __cdecl `dynamic atexit destructor for 'vec''(void)'
<source>(12): note: see reference to class template instantiation 'std::vector<hptr,std::allocator<hptr>>' being compiled

Godbolt: https://godbolt.org/z/738sMK337

STL version

  • visual studio 17.6.0
  • reproducible with godbolt msvc latest

Additional info

It seems the problem is in <vector> and not in memory. Instantiating hptr from the demo when no std::vector is involved compiles fine,

This code, for example, compiles:

#include <memory>

template <typename T>
struct holder {
    T t;
};

class fwd;
using hptr = std::shared_ptr<holder<fwd>>;

template <typename T>
struct my_vec {
    T* data;
    my_vec(int n) { data = new T[n]; }
    ~my_vec() { delete[] data; }
};

my_vec<hptr> vec(5);

Godbolt link: https://godbolt.org/z/zWd6E13Y3

This is also DevCom-10535345 / VSO-1926908 / AB#1926908.

@iboB iboB changed the title <vector>: std::vector<std::shared_ptr<mytype<T>>> fails to compile when T is incomplete <vector>: std::vector<std::shared_ptr<mytype<T>>> fails to compile when T is incomplete Jun 30, 2023
@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Jun 30, 2023

This is probably because of the unqualified call to _Destroy_range which triggers ADL (argument-dependent lookup) and thus requires the associate class to be complete.

_Destroy_range(_Myfirst, _Mylast, _Al);

Probably duplicate, see #140, #1596, and #3100.

@CaseyCarter CaseyCarter added the bug Something isn't working label Jul 1, 2023
@CaseyCarter
Copy link
Contributor

It isn't only _Destroy_range, but you're right about the problem being _Ugly ADL:

diff --git a/stl/inc/vector b/stl/inc/vector
index c6552845..40f34e00 100644
--- a/stl/inc/vector
+++ b/stl/inc/vector
@@ -2052,7 +2052,7 @@ private:
         _My_data._Orphan_all();
 
         if (_Myfirst) { // destroy and deallocate old array
-            _Destroy_range(_Myfirst, _Mylast, _Al);
+            _STD _Destroy_range(_Myfirst, _Mylast, _Al);^M
             _ASAN_VECTOR_REMOVE;
             _Al.deallocate(_Myfirst, static_cast<size_type>(_Myend - _Myfirst));
 
diff --git a/stl/inc/xmemory b/stl/inc/xmemory
index c2c73ef5..df8a9eba 100644
--- a/stl/inc/xmemory
+++ b/stl/inc/xmemory
@@ -966,7 +966,7 @@ public:
     _CONSTEXPR20 void deallocate(_Ty* const _Ptr, const size_t _Count) {
         _STL_ASSERT(_Ptr != nullptr || _Count == 0, "null pointer cannot point to a block of non-zero size");
         // no overflow check on the following multiply; we assume _Allocate did that check
-        _Deallocate<_New_alignof<_Ty>>(_Ptr, sizeof(_Ty) * _Count);
+        _STD _Deallocate<_New_alignof<_Ty>>(_Ptr, sizeof(_Ty) * _Count);^M
     }
 
     _NODISCARD_RAW_PTR_ALLOC _CONSTEXPR20 __declspec(allocator) _Ty* allocate(_CRT_GUARDOVERFLOW const size_t _Count) {
@@ -1084,7 +1084,7 @@ _CONSTEXPR20 void _Destroy_range(_Alloc_ptr_t<_Alloc> _First, const _Alloc_ptr_t
     using _Ty = typename _Alloc::value_type;
     if constexpr (!conjunction_v<is_trivially_destructible<_Ty>, _Uses_default_destroy<_Alloc, _Ty*>>) {
         for (; _First != _Last; ++_First) {
-            allocator_traits<_Alloc>::destroy(_Al, _Unfancy(_First));
+            allocator_traits<_Alloc>::destroy(_Al, _STD _Unfancy(_First));^M
         }
     }
 }

CaseyCarter added a commit to CaseyCarter/STL that referenced this issue Feb 7, 2024
By qualifying `_Ugly` calls in `vector::_Tidy`, `allocator`'s member functions, and `_Destroy_range`.

Fixes microsoft#3842
CaseyCarter added a commit to CaseyCarter/STL that referenced this issue Feb 7, 2024
By qualifying `_Ugly` calls in `vector::_Tidy`, `allocator`'s member functions, and `_Destroy_range` to avoid inadvertent ADL.

Fixes microsoft#3842
@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Something works now, yay!
Projects
None yet
4 participants