Skip to content

Commit

Permalink
Don't deduce the return type of emplace{,_back,_front}
Browse files Browse the repository at this point in the history
The instantiation used to deduce the return types of template functions with placeholders in their declared return type happens outside the immediate context of any other template instantiation. Notably, that means the body of such an instantiation isn't an unevaluated context.

Fixes DevCom-10745303 / VSO-2252142
  • Loading branch information
CaseyCarter committed Sep 18, 2024
1 parent a26f4ad commit 6209e0b
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 9 deletions.
4 changes: 2 additions & 2 deletions stl/inc/deque
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,7 @@ private:

public:
template <class... _Valty>
decltype(auto) emplace_front(_Valty&&... _Val) {
_CONTAINER_EMPLACE_RETURN emplace_front(_Valty&&... _Val) {
_Orphan_all();
_Emplace_front_internal(_STD forward<_Valty>(_Val)...);
#if _HAS_CXX17
Expand All @@ -1169,7 +1169,7 @@ public:
}

template <class... _Valty>
decltype(auto) emplace_back(_Valty&&... _Val) {
_CONTAINER_EMPLACE_RETURN emplace_back(_Valty&&... _Val) {
_Orphan_all();
_Emplace_back_internal(_STD forward<_Valty>(_Val)...);

Expand Down
2 changes: 1 addition & 1 deletion stl/inc/forward_list
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ public:
}

template <class... _Valty>
decltype(auto) emplace_front(_Valty&&... _Val) { // insert element at beginning
_CONTAINER_EMPLACE_RETURN emplace_front(_Valty&&... _Val) { // insert element at beginning
_Insert_after(_Mypair._Myval2._Before_head(), _STD forward<_Valty>(_Val)...);

#if _HAS_CXX17
Expand Down
4 changes: 2 additions & 2 deletions stl/inc/list
Original file line number Diff line number Diff line change
Expand Up @@ -981,7 +981,7 @@ public:
}

template <class... _Valty>
decltype(auto) emplace_front(_Valty&&... _Val) { // insert element at beginning
_CONTAINER_EMPLACE_RETURN emplace_front(_Valty&&... _Val) { // insert element at beginning
reference _Result = _Emplace(_Mypair._Myval2._Myhead->_Next, _STD forward<_Valty>(_Val)...)->_Myval;

#if _HAS_CXX17
Expand All @@ -992,7 +992,7 @@ public:
}

template <class... _Valty>
decltype(auto) emplace_back(_Valty&&... _Val) { // insert element at end
_CONTAINER_EMPLACE_RETURN emplace_back(_Valty&&... _Val) { // insert element at end
reference _Result = _Emplace(_Mypair._Myval2._Myhead, _STD forward<_Valty>(_Val)...)->_Myval;

#if _HAS_CXX17
Expand Down
2 changes: 1 addition & 1 deletion stl/inc/queue
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public:
#endif // _HAS_CXX23

template <class... _Valty>
decltype(auto) emplace(_Valty&&... _Val) {
_CONTAINER_EMPLACE_RETURN emplace(_Valty&&... _Val) {
#if _HAS_CXX17
return c.emplace_back(_STD forward<_Valty>(_Val)...);
#else // ^^^ _HAS_CXX17 / !_HAS_CXX17 vvv
Expand Down
2 changes: 1 addition & 1 deletion stl/inc/stack
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public:
#endif // _HAS_CXX23

template <class... _Valty>
decltype(auto) emplace(_Valty&&... _Val) {
_CONTAINER_EMPLACE_RETURN emplace(_Valty&&... _Val) {
#if _HAS_CXX17
return c.emplace_back(_STD forward<_Valty>(_Val)...);
#else // ^^^ _HAS_CXX17 / !_HAS_CXX17 vvv
Expand Down
4 changes: 2 additions & 2 deletions stl/inc/vector
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ private:

public:
template <class... _Valty>
_CONSTEXPR20 decltype(auto) emplace_back(_Valty&&... _Val) {
_CONSTEXPR20 _CONTAINER_EMPLACE_RETURN emplace_back(_Valty&&... _Val) {
// insert by perfectly forwarding into element at end, provide strong guarantee
_Ty& _Result = _Emplace_one_at_back(_STD forward<_Valty>(_Val)...);
#if _HAS_CXX17
Expand Down Expand Up @@ -2979,7 +2979,7 @@ public:
}

template <class... _Valty>
_CONSTEXPR20 decltype(auto) emplace_back(_Valty&&... _Val) {
_CONSTEXPR20 _CONTAINER_EMPLACE_RETURN emplace_back(_Valty&&... _Val) {
bool _Tmp(_STD forward<_Valty>(_Val)...);
push_back(_Tmp);

Expand Down
6 changes: 6 additions & 0 deletions stl/inc/xmemory
Original file line number Diff line number Diff line change
Expand Up @@ -2621,6 +2621,12 @@ _NODISCARD _Elem* _UIntegral_to_buff(_Elem* _RNext, _UTy _UVal) { // used by bot
}
_STD_END

#if _HAS_CXX17
#define _CONTAINER_EMPLACE_RETURN reference
#else // ^^^ _HAS_CXX17 ^^^ / vvv !_HAS_CXX17 vvv
#define _CONTAINER_EMPLACE_RETURN void
#endif // ^^^ _HAS_CXX17 ^^^

#pragma pop_macro("new")
_STL_RESTORE_CLANG_WARNINGS
#pragma warning(pop)
Expand Down
1 change: 1 addition & 0 deletions tests/std/test.lst
Original file line number Diff line number Diff line change
Expand Up @@ -766,3 +766,4 @@ tests\VSO_0971246_legacy_await_headers
tests\VSO_1775715_user_defined_modules
tests\VSO_1804139_static_analysis_warning_with_single_element_array
tests\VSO_1925201_iter_traits
tests\VSO_2252142_wrong_C5046
4 changes: 4 additions & 0 deletions tests/std/tests/VSO_2252142_wrong_C5046/env.lst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

RUNALL_INCLUDE ..\usual_matrix.lst
56 changes: 56 additions & 0 deletions tests/std/tests/VSO_2252142_wrong_C5046/test.compile.pass.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright (c) Microsoft Corporation.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include <deque>
#include <forward_list>
#include <list>
#include <queue>
#include <stack>
#include <type_traits>
#include <utility>
#include <vector>

// Test for DevCom-10745303 / VSO-2252142 "C5046 is wrongly triggered in unevaluated context"

#define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__)

using namespace std;

template <class T>
struct convertible_to_any {
operator T() &&; // not defined, only used in unevaluated context
};

template <class Cont, class = void>
constexpr bool has_emplace = false;
template <class Cont>
constexpr bool has_emplace<Cont,
void_t<decltype(declval<Cont&>().emplace(declval<convertible_to_any<typename Cont::value_type>>()))>> = true;

template <class Cont, class = void>
constexpr bool has_emplace_back = false;
template <class Cont>
constexpr bool has_emplace_back<Cont,
void_t<decltype(declval<Cont&>().emplace_back(declval<convertible_to_any<typename Cont::value_type>>()))>> = true;

template <class Cont, class = void>
constexpr bool has_emplace_front = false;
template <class Cont>
constexpr bool has_emplace_front<Cont,
void_t<decltype(declval<Cont&>().emplace_front(declval<convertible_to_any<typename Cont::value_type>>()))>> = true;

namespace {
struct S2 {};
} // namespace

// Was emitting "C5046 Symbol involving type with internal linkage not defined" as a consequence of our use of return
// type deduction for the pertinent container functions.
STATIC_ASSERT(has_emplace_back<vector<S2>>);
STATIC_ASSERT(has_emplace_back<deque<S2>>);
STATIC_ASSERT(has_emplace_front<deque<S2>>);
STATIC_ASSERT(has_emplace_front<forward_list<S2>>);
STATIC_ASSERT(has_emplace_back<list<S2>>);
STATIC_ASSERT(has_emplace_front<list<S2>>);
STATIC_ASSERT(has_emplace<queue<S2>>);
STATIC_ASSERT(has_emplace<stack<S2>>);
STATIC_ASSERT(has_emplace_back<vector<bool>>); // Cannot trigger this bug, but for consistency

0 comments on commit 6209e0b

Please sign in to comment.