From 6209e0bbbe8a5fc9891c0fb5c2912735f58e2ff7 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 17 Sep 2024 16:44:19 -0700 Subject: [PATCH 1/3] Don't deduce the return type of `emplace`{,`_back`,`_front`} 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 --- stl/inc/deque | 4 +- stl/inc/forward_list | 2 +- stl/inc/list | 4 +- stl/inc/queue | 2 +- stl/inc/stack | 2 +- stl/inc/vector | 4 +- stl/inc/xmemory | 6 ++ tests/std/test.lst | 1 + .../std/tests/VSO_2252142_wrong_C5046/env.lst | 4 ++ .../test.compile.pass.cpp | 56 +++++++++++++++++++ 10 files changed, 76 insertions(+), 9 deletions(-) create mode 100644 tests/std/tests/VSO_2252142_wrong_C5046/env.lst create mode 100644 tests/std/tests/VSO_2252142_wrong_C5046/test.compile.pass.cpp diff --git a/stl/inc/deque b/stl/inc/deque index 457136014d4..abf9a0eeaa4 100644 --- a/stl/inc/deque +++ b/stl/inc/deque @@ -1160,7 +1160,7 @@ private: public: template - 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 @@ -1169,7 +1169,7 @@ public: } template - decltype(auto) emplace_back(_Valty&&... _Val) { + _CONTAINER_EMPLACE_RETURN emplace_back(_Valty&&... _Val) { _Orphan_all(); _Emplace_back_internal(_STD forward<_Valty>(_Val)...); diff --git a/stl/inc/forward_list b/stl/inc/forward_list index f55da31fd95..62c888e47ba 100644 --- a/stl/inc/forward_list +++ b/stl/inc/forward_list @@ -700,7 +700,7 @@ public: } template - 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 diff --git a/stl/inc/list b/stl/inc/list index 5d42e9fe73d..033a43c60c7 100644 --- a/stl/inc/list +++ b/stl/inc/list @@ -981,7 +981,7 @@ public: } template - 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 @@ -992,7 +992,7 @@ public: } template - 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 diff --git a/stl/inc/queue b/stl/inc/queue index 2d8b6bd0df1..9c0262663c8 100644 --- a/stl/inc/queue +++ b/stl/inc/queue @@ -135,7 +135,7 @@ public: #endif // _HAS_CXX23 template - 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 diff --git a/stl/inc/stack b/stl/inc/stack index 02c404594c1..2640586df5a 100644 --- a/stl/inc/stack +++ b/stl/inc/stack @@ -120,7 +120,7 @@ public: #endif // _HAS_CXX23 template - 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 diff --git a/stl/inc/vector b/stl/inc/vector index 8a123af1f31..52822076c51 100644 --- a/stl/inc/vector +++ b/stl/inc/vector @@ -853,7 +853,7 @@ private: public: template - _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 @@ -2979,7 +2979,7 @@ public: } template - _CONSTEXPR20 decltype(auto) emplace_back(_Valty&&... _Val) { + _CONSTEXPR20 _CONTAINER_EMPLACE_RETURN emplace_back(_Valty&&... _Val) { bool _Tmp(_STD forward<_Valty>(_Val)...); push_back(_Tmp); diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 4944352f3a6..e8d373100d7 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -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) diff --git a/tests/std/test.lst b/tests/std/test.lst index fde8f2a1d2c..3958e8f11be 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -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 diff --git a/tests/std/tests/VSO_2252142_wrong_C5046/env.lst b/tests/std/tests/VSO_2252142_wrong_C5046/env.lst new file mode 100644 index 00000000000..19f025bd0e6 --- /dev/null +++ b/tests/std/tests/VSO_2252142_wrong_C5046/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_matrix.lst diff --git a/tests/std/tests/VSO_2252142_wrong_C5046/test.compile.pass.cpp b/tests/std/tests/VSO_2252142_wrong_C5046/test.compile.pass.cpp new file mode 100644 index 00000000000..5b1de10dd26 --- /dev/null +++ b/tests/std/tests/VSO_2252142_wrong_C5046/test.compile.pass.cpp @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include +#include +#include +#include + +// 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 +struct convertible_to_any { + operator T() &&; // not defined, only used in unevaluated context +}; + +template +constexpr bool has_emplace = false; +template +constexpr bool has_emplace().emplace(declval>()))>> = true; + +template +constexpr bool has_emplace_back = false; +template +constexpr bool has_emplace_back().emplace_back(declval>()))>> = true; + +template +constexpr bool has_emplace_front = false; +template +constexpr bool has_emplace_front().emplace_front(declval>()))>> = 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>); +STATIC_ASSERT(has_emplace_back>); +STATIC_ASSERT(has_emplace_front>); +STATIC_ASSERT(has_emplace_front>); +STATIC_ASSERT(has_emplace_back>); +STATIC_ASSERT(has_emplace_front>); +STATIC_ASSERT(has_emplace>); +STATIC_ASSERT(has_emplace>); +STATIC_ASSERT(has_emplace_back>); // Cannot trigger this bug, but for consistency From b7a4bb3c05613b5830176d7a9d8c7b6d8e69ac56 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 18 Sep 2024 16:51:08 -0700 Subject: [PATCH 2/3] Fix typo! --- stl/inc/xmemory | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index e8d373100d7..d30280d0f60 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -2625,7 +2625,7 @@ _STD_END #define _CONTAINER_EMPLACE_RETURN reference #else // ^^^ _HAS_CXX17 ^^^ / vvv !_HAS_CXX17 vvv #define _CONTAINER_EMPLACE_RETURN void -#endif // ^^^ _HAS_CXX17 ^^^ +#endif // ^^^ !_HAS_CXX17 ^^^ #pragma pop_macro("new") _STL_RESTORE_CLANG_WARNINGS From c1ff13f4b4780bfa81b4c7557641999c80775fcd Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 18 Sep 2024 16:59:01 -0700 Subject: [PATCH 3/3] Quote the warning message more literally. --- tests/std/tests/VSO_2252142_wrong_C5046/test.compile.pass.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/VSO_2252142_wrong_C5046/test.compile.pass.cpp b/tests/std/tests/VSO_2252142_wrong_C5046/test.compile.pass.cpp index 5b1de10dd26..8fa63feda4f 100644 --- a/tests/std/tests/VSO_2252142_wrong_C5046/test.compile.pass.cpp +++ b/tests/std/tests/VSO_2252142_wrong_C5046/test.compile.pass.cpp @@ -43,8 +43,8 @@ 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. +// Was emitting "warning 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>); STATIC_ASSERT(has_emplace_back>); STATIC_ASSERT(has_emplace_front>);