[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320500: workaround PR 28385 in __find_exactly_one_checked 
(authored by CaseyCarter, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41048?vs=126559=126564#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41048

Files:
  libcxx/trunk/include/tuple


Index: libcxx/trunk/include/tuple
===
--- libcxx/trunk/include/tuple
+++ libcxx/trunk/include/tuple
@@ -1012,10 +1012,10 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+static constexpr bool __matches[sizeof...(_Args)] = {is_same<_T1, 
_Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type 
list");
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type 
list");
 };
 
 template 


Index: libcxx/trunk/include/tuple
===
--- libcxx/trunk/include/tuple
+++ libcxx/trunk/include/tuple
@@ -1012,10 +1012,10 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+static constexpr bool __matches[sizeof...(_Args)] = {is_same<_T1, _Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type list");
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type list");
 };
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter added a comment.

This new-and-improved workaround - thanks to @lichray - is unintrusive enough 
that I can't imagine it being unnacceptable to anyone. I'm going to go ahead 
and check this in.


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter updated this revision to Diff 126559.
CaseyCarter added a comment.

Better/simpler workaround from Zhihao.


https://reviews.llvm.org/D41048

Files:
  include/tuple


Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -1012,10 +1012,10 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+static constexpr bool __matches[sizeof...(_Args)] = {is_same<_T1, 
_Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type 
list");
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type 
list");
 };
 
 template 


Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -1012,10 +1012,10 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+static constexpr bool __matches[sizeof...(_Args)] = {is_same<_T1, _Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type list");
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type list");
 };
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Zhihao Yuan via Phabricator via cfe-commits]

lichray accepted this revision.
lichray added a comment.
This revision is now accepted and ready to land.

Reproduced with

  lit -sv --param=cxx_under_test="$HOME/bin/clang++" test/std/utilities/tuple/
  lit: [...] note: Using available_features: ['libc++', 'verify-support', 
'clang-6', 'modules-support', 'locale.en_US.UTF-8', 'diagnose-if-support', 
'long_tests', 'fdelayed-template-parsing', '-faligned-allocation', 
'locale.zh_CN.UTF-8', 'c++2a', 'locale.fr_CA.ISO8859-1', 'c++filesystem', 
'c++experimental', 'clang', 'locale.fr_FR.UTF-8', 'locale.ru_RU.UTF-8', 
'fsized-deallocation', 'freebsd11', 'fcoroutines-ts', 'locale.cs_CZ.ISO8859-2', 
'clang-6.0', 'thread-safety']


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter marked 2 inline comments as done.
CaseyCarter added a comment.

This unconditional workaround addresses Marshall's concerns about the naked 
version test.


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter updated this revision to Diff 126406.
CaseyCarter added a comment.

Make the workaround unconditional.


https://reviews.llvm.org/D41048

Files:
  include/tuple


Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -1012,10 +1012,15 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
-static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type 
list");
+inline _LIBCPP_INLINE_VISIBILITY
+static constexpr size_t __index()
+{
+constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+return __find_detail::__find_idx(0, __matches);
+}
+static constexpr size_t value = __index();
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type 
list");
 };
 
 template 


Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -1012,10 +1012,15 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
-static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type list");
+inline _LIBCPP_INLINE_VISIBILITY
+static constexpr size_t __index()
+{
+constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+return __find_detail::__find_idx(0, __matches);
+}
+static constexpr size_t value = __index();
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type list");
 };
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter added inline comments.



Comment at: include/tuple:1015
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+#if defined(__clang__) && __clang_major__ > 5 && __cplusplus > 201402L
+// Workaround https://bugs.llvm.org/show_bug.cgi?id=28385

mclow.lists wrote:
> We try not to have naked tests like this in the code.
> 
> If we have to, maybe:
> 
> `#if defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER > 500 && 
> _LIBCPP_STD_VER > 14`
> 
> but I would rather define a new macro in <__config> and give it a meaningful 
> name.
> 
Better?


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter updated this revision to Diff 126297.
CaseyCarter added a comment.

Hide the ugly version test in `<__config>`, define a slightly-more-meaningful 
macro `_LIBCPP_WORKAROUND_CLANG_28385`.


https://reviews.llvm.org/D41048

Files:
  include/__config
  include/tuple


Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -1012,10 +1012,20 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+#if defined(_LIBCPP_WORKAROUND_CLANG_28385) && _LIBCPP_WORKAROUND_CLANG_28385
+inline _LIBCPP_INLINE_VISIBILITY
+static constexpr size_t __index()
+{
+constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+return __find_detail::__find_idx(0, __matches);
+}
+static constexpr size_t value = __index();
+#else // _LIBCPP_WORKAROUND_CLANG_28385
+static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type 
list");
+#endif // _LIBCPP_WORKAROUND_CLANG_28385
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type 
list");
 };
 
 template 
Index: include/__config
===
--- include/__config
+++ include/__config
@@ -456,7 +456,7 @@
 // Allow for build-time disabling of unsigned integer sanitization
 #if !defined(_LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK) && 
__has_attribute(no_sanitize)
 #define _LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK 
__attribute__((__no_sanitize__("unsigned-integer-overflow")))
-#endif 
+#endif
 
 #if __has_builtin(__builtin_launder)
 #define_LIBCPP_COMPILER_HAS_BUILTIN_LAUNDER
@@ -1273,6 +1273,14 @@
 # endif
 #endif // defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_BUILDING_LIBRARY)
 
+#ifndef _LIBCPP_WORKAROUND_CLANG_28385
+ #if defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER >= 600 && 
_LIBCPP_STD_VER > 14
+  #define _LIBCPP_WORKAROUND_CLANG_28385 1
+ #else
+  #define _LIBCPP_WORKAROUND_CLANG_28385 0
+ #endif
+#endif
+
 #endif // __cplusplus
 
 #endif // _LIBCPP_CONFIG


Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -1012,10 +1012,20 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+#if defined(_LIBCPP_WORKAROUND_CLANG_28385) && _LIBCPP_WORKAROUND_CLANG_28385
+inline _LIBCPP_INLINE_VISIBILITY
+static constexpr size_t __index()
+{
+constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+return __find_detail::__find_idx(0, __matches);
+}
+static constexpr size_t value = __index();
+#else // _LIBCPP_WORKAROUND_CLANG_28385
+static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type list");
+#endif // _LIBCPP_WORKAROUND_CLANG_28385
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type list");
 };
 
 template 
Index: include/__config
===
--- include/__config
+++ include/__config
@@ -456,7 +456,7 @@
 // Allow for build-time disabling of unsigned integer sanitization
 #if !defined(_LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK) && __has_attribute(no_sanitize)
 #define _LIBCPP_DISABLE_UBSAN_UNSIGNED_INTEGER_CHECK __attribute__((__no_sanitize__("unsigned-integer-overflow")))
-#endif 
+#endif
 
 #if __has_builtin(__builtin_launder)
 #define	_LIBCPP_COMPILER_HAS_BUILTIN_LAUNDER
@@ -1273,6 +1273,14 @@
 # endif
 #endif // defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_BUILDING_LIBRARY)
 
+#ifndef _LIBCPP_WORKAROUND_CLANG_28385
+ #if defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER >= 600 && _LIBCPP_STD_VER > 14
+  #define _LIBCPP_WORKAROUND_CLANG_28385 1
+ #else
+  #define _LIBCPP_WORKAROUND_CLANG_28385 0
+ #endif
+#endif
+
 #endif // __cplusplus
 
 #endif // _LIBCPP_CONFIG
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter added a comment.

In https://reviews.llvm.org/D41048#950635, @mclow.lists wrote:

> Ah - that was the factor I was missing.
>  The tests pass for me with `-std=c++2a`, but fail for `std=c++17`
>
> Casey's original post said they fail with `2a`, and I'm *still* not seeing 
> that.


They fail for me both with -std=c++2a and -std=c++17.


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Ah - that was the factor I was missing.
The tests pass for me with `-std=c++2a`, but fail for `std=c++17`

Casey's original post said they fail with `2a`, and I'm *still* not seeing that.


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D41048#950606, @CaseyCarter wrote:

> In https://reviews.llvm.org/D41048#950601, @mclow.lists wrote:
>
> > These tests don't fail for me. (using a clang I built two days ago)
>
>
> What about the repro for #35578 
> ? That repro and these four 
> tests trigger the bug for me (no matching function for call to 
> '__find_idx'...could not match "const bool" against "const bool") with clang 
> version 6.0.0-svn320282-1~exp1 (trunk) freshly updated from apt.llvm.org on 
> Ubuntu trusty running atop Windows Subsystem for Linux. (We've been shipping 
> a workaround for 28385  in MSFT 
> `` forever, and in range-v3 for even longer 
> .)
>  Both Godbolt  and Wandbox 
>  agree.


What we've been trying to convey is, none of the **current** tests with 
**current** default params trigger the problem.
The difference being `-std=c++1z`. So some new tests are needed.


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter added a comment.

In https://reviews.llvm.org/D41048#950601, @mclow.lists wrote:

> These tests don't fail for me. (using a clang I built two days ago)


What about the repro for #35578 ? 
That repro and these four tests trigger the bug for me (...could not match 
"const bool" against "const bool") with clang version 6.0.0-svn320282-1~exp1 
(trunk) freshly updated from apt.llvm.org on Ubuntu trusty running atop Windows 
Subsystem for Linux. (We've been shipping a workaround for 28385 
 in MSFT `` forever, and 
in range-v3 for even longer 
.)
 Both Godbolt  and Wandbox 
 agree.


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/tuple:1015
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+#if defined(__clang__) && __clang_major__ > 5 && __cplusplus > 201402L
+// Workaround https://bugs.llvm.org/show_bug.cgi?id=28385

We try not to have naked tests like this in the code.

If we have to, maybe:

`#if defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER > 500 && 
_LIBCPP_STD_VER > 14`

but I would rather define a new macro in <__config> and give it a meaningful 
name.



https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

These tests don't fail for me. (using a clang I built two days ago)


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter added a comment.

In https://reviews.llvm.org/D41048#950424, @lebedev.ri wrote:

> Test?


Four of the existing variant tests fail without this change (Using the clang-6 
dailies from apt.llvm.org and -std=c++2a):

Failing Tests (4):

libc++ :: std/utilities/variant/variant.get/get_if_type.pass.cpp
libc++ :: std/utilities/variant/variant.get/get_type.pass.cpp
libc++ :: std/utilities/variant/variant.get/holds_alternative.pass.cpp
libc++ :: 
std/utilities/variant/variant.variant/variant.mod/emplace_type_init_list_args.pass.cpp
  
  Expected Passes: 36
  Unexpected Failures: 4

I think that's plenty of test coverage.


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Test?


https://reviews.llvm.org/D41048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41048: [libcxx] workaround PR 28385 in __find_exactly_one_checked

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter created this revision.

Fixes #35578.


https://reviews.llvm.org/D41048

Files:
  include/tuple


Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -1012,10 +1012,21 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+#if defined(__clang__) && __clang_major__ > 5 && __cplusplus > 201402L
+// Workaround https://bugs.llvm.org/show_bug.cgi?id=28385
+inline _LIBCPP_INLINE_VISIBILITY
+static constexpr size_t __index()
+{
+constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+return __find_detail::__find_idx(0, __matches);
+}
+static constexpr size_t value = __index();
+#else
+static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type 
list");
+#endif
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type 
list");
 };
 
 template 


Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -1012,10 +1012,21 @@
 
 template 
 struct __find_exactly_one_checked {
-  static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+#if defined(__clang__) && __clang_major__ > 5 && __cplusplus > 201402L
+// Workaround https://bugs.llvm.org/show_bug.cgi?id=28385
+inline _LIBCPP_INLINE_VISIBILITY
+static constexpr size_t __index()
+{
+constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
+return __find_detail::__find_idx(0, __matches);
+}
+static constexpr size_t value = __index();
+#else
+static constexpr bool __matches[] = {is_same<_T1, _Args>::value...};
 static constexpr size_t value = __find_detail::__find_idx(0, __matches);
-static_assert (value != __not_found, "type not found in type list" );
-static_assert(value != __ambiguous,"type occurs more than once in type list");
+#endif
+static_assert(value != __not_found, "type not found in type list" );
+static_assert(value != __ambiguous, "type occurs more than once in type list");
 };
 
 template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits