KEMBAR78
`<vector>`: Fix allocator types in reallocation guards by StephanTLavavej · Pull Request #5729 · microsoft/STL · GitHub
Skip to content

Conversation

@StephanTLavavej
Copy link
Member

Reported by Weipeng Liu internally. Consider the following code, which is activating our escape hatch to disable enforcement of WG21-N5014 [container.alloc.reqmts]/5 "Mandates: allocator_type::value_type is the same as X::value_type."

D:\GitHub\STL\out\x64>type meow.cpp
#include <memory_resource>
#include <vector>
using namespace std;

int main() {
    vector<int, pmr::polymorphic_allocator<double>> my_vector;

    my_vector.emplace_back(3);
}
D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /std:c++latest /D_ENFORCE_MATCHING_ALLOCATORS=0 meow.cpp
meow.cpp
D:\GitHub\STL\out\x64\out\inc\vector(897): error C2440: 'initializing': cannot convert from 'std::pmr::polymorphic_allocator<_Newfirst>' to '_Alloc &'
        with
        [
            _Newfirst=int
        ]
        and
        [
            _Alloc=std::pmr::polymorphic_allocator<double>
        ]
[...]

Although I grumbled at the use of our escape hatch allowing non-conformantly mismatched allocators, Weipeng identified the problem - vector's reallocation guards mention the wrong allocator type. In STL containers, we have a convention that _Alloc is the user's original allocator, while _Alty is the properly rebound allocator. The reallocation guards refer to original _Alloc, contrary to our convention.

This was introduced by #4977 on 2024-10-11 in VS 2022 17.13, so it's a recent regression.

Because the reallocation guard types are used locally within member functions, we can fix this in an ABI-compatible way by renaming the guard types while fixing the allocator reference types.

I am not adding test coverage because we have none for the escape hatch, and this fix is unlikely to accidentally regress again.

@StephanTLavavej StephanTLavavej requested a review from a team as a code owner September 18, 2025 11:11
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Sep 18, 2025
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews Sep 18, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in STL Code Reviews Sep 18, 2025
@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Sep 19, 2025

Thanks for the fix!

I strongly thought that renaming was meaningless because we previously almost always reject uses where _Alloc and _Alty were different. However, there might be inheritance between these two types, which is highly pathological but still possible.

@StephanTLavavej StephanTLavavej moved this from Final Review to Ready To Merge in STL Code Reviews Sep 19, 2025
@StephanTLavavej StephanTLavavej moved this from Ready To Merge to Merging in STL Code Reviews Sep 19, 2025
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 9e2d9ec into microsoft:main Sep 22, 2025
39 checks passed
@github-project-automation github-project-automation bot moved this from Merging to Done in STL Code Reviews Sep 22, 2025
@StephanTLavavej StephanTLavavej deleted the match-three branch September 22, 2025 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants