[PATCH] D28222: [libcxx] Re-implement LWG 2770 again: Fix tuple_size to work with structured bindings

EricWF updated this revision to Diff 82832. EricWF added a comment. Use the correct patch file this time. https://reviews.llvm.org/D28222 Files: include/__tuple test/libcxx/test/config.py test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_incomplete.fail.cpp test/std/utiliti

[PATCH] D28220: provide Win32 native threading

EricWF added a comment. Could you upload this patch with more context? Repository: rL LLVM https://reviews.llvm.org/D28220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28220: provide Win32 native threading

EricWF added inline comments. Comment at: include/__threading_support:44 +#define WIN32_LEAN_AND_MEAN +#define VC_EXTRA_LEAN +#include Do these definitions have any affect when `` has already been included? Also are these definitions required before including t

[PATCH] D28226: threading_support: introduce __libcpp_recursive_mutex_t

EricWF added inline comments. Comment at: include/__threading_support:79 + + template (__value); + } Wouldn't a `static_cast` be valid here? Comment at: include/__threading_support:110 +int __libcpp_mutex_lock(__libcpp_mutex_reference&& __m);

[PATCH] D28220: provide Win32 native threading

EricWF added inline comments. Comment at: include/__threading_support:44 +#define WIN32_LEAN_AND_MEAN +#define VC_EXTRA_LEAN +#include compnerd wrote: > EricWF wrote: > > Do these definitions have any affect when `` has already been > > included? > > Also are t

[PATCH] D28222: [libcxx] Re-implement LWG 2770 again: Fix tuple_size to work with structured bindings

EricWF added inline comments. Comment at: include/__tuple:32 template -class _LIBCPP_TYPE_VIS_ONLY tuple_size -: public __tuple_size_base_type<_Tp>::type {}; +class _LIBCPP_TYPE_VIS_ONLY tuple_size<__enable_if_tuple_size_imp::type>::value)>> +: public integral_constant:

[PATCH] D28222: [libcxx] Re-implement LWG 2770 again: Fix tuple_size to work with structured bindings

EricWF planned changes to this revision. EricWF added a comment. I thought this formulation worked for GCC and Clang but it appears it does not. I'll update with another attempt. https://reviews.llvm.org/D28222 ___ cfe-commits mailing list cfe-comm

[PATCH] D28222: [libcxx] Re-implement LWG 2770 again: Fix tuple_size to work with structured bindings

EricWF updated this revision to Diff 82850. EricWF added a comment. This revision is now accepted and ready to land. @rsmith Could you re-review this please? It's a mostly different implementation. https://reviews.llvm.org/D28222 Files: include/__tuple test/libcxx/test/config.py test/std/

[PATCH] D28222: [libcxx] Re-implement LWG 2770 again: Fix tuple_size to work with structured bindings

EricWF updated this revision to Diff 82851. EricWF added a comment. - Remove redundant SFINAE. https://reviews.llvm.org/D28222 Files: include/__tuple test/libcxx/test/config.py test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size.pass.cpp test/std/utilities/tuple/tuple.tuple/tu

[PATCH] D28223: clean up use of _WIN32

EricWF added inline comments. Comment at: include/__config:158 +#if defined(_WIN32) +# define _LIBCPP_WIN32 1 # define _LIBCPP_LITTLE_ENDIAN 1 smeenai wrote: > Perhaps `_LIBCPP_WIN32API` instead, to be clear that this is specific to the > usage of Win

[PATCH] D26830: [libcxx] Add string_view literals

EricWF added inline comments. Comment at: test/std/strings/string.view/string.view.literals/literal3.pass.cpp:16 + +int main() +{ You can move this test into `literal.pass.cpp` but putting it at another function scope. https://reviews.llvm.org/D26830 _

[PATCH] D26830: [libcxx] Add string_view literals

EricWF added a comment. This LGTM. I'll approve once I see the inline comments addressed. Comment at: test/std/strings/string.view/string.view.literals/literal.pass.cpp:10 +//===--===// + +// UNSUPPORTED: c++9

[PATCH] D28222: [libcxx] Re-implement LWG 2770 again: Fix tuple_size to work with structured bindings

EricWF updated the summary for this revision. EricWF updated this revision to Diff 82867. EricWF added a comment. - SFINAE the specializations on `sizeof(tuple_size)` instead of `decltype(tuple_size::value)`. https://reviews.llvm.org/D28222 Files: include/__tuple test/libcxx/test/config.py

[PATCH] D28222: [libcxx] Re-implement LWG 2770 again: Fix tuple_size to work with structured bindings

EricWF added inline comments. Comment at: include/__tuple:34 +const _Tp, +typename enable_if::value>::type, +decltype(tuple_size<_Tp>::value)>> Without this extra `enable_if`, GCC will diagnose each of these three specializations as ambiguous. htt

[PATCH] D28226: threading_support: introduce __libcpp_recursive_mutex_t

EricWF added a comment. Yeah, I think I agree with @majnemer that I would rather use an expanded interface than this type-erased mutex type. Thank you for working on this. Repository: rL LLVM https://reviews.llvm.org/D28226 ___ cfe-commits maili

[PATCH] D22584: constexpr array support C++1z (P0031)

EricWF resigned from this revision. EricWF removed a reviewer: EricWF. EricWF added a comment. Looks like @mclow.lists just committed another version of this paper, so this patch is no longer needed. Resigning as reviewer. https://reviews.llvm.org/D22584

[PATCH] D28316: [libc++] Cleanup and document <__threading_support>

EricWF created this revision. EricWF added reviewers: compnerd, rmaprath. EricWF added a subscriber: cfe-commits. Herald added a subscriber: mgorny. This patch attempts to clean up the macro configuration mess in `<__threading_support>`, specifically the mess involving external threading variant

[PATCH] D28253: static_assert inside make_shared when the object is not constructible

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM minus the `config.py` changes, but those were discussed offline. Also I prefer to write new `.fail.cpp` tests using clang verify. It allows fail tests to contain more than one test case,

[PATCH] D28222: [libcxx] Re-implement LWG 2770 again: Fix tuple_size to work with structured bindings

EricWF updated this revision to Diff 83145. EricWF added a comment. Updating with final changes. https://reviews.llvm.org/D28222 Files: include/__tuple test/libcxx/test/config.py test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size.pass.cpp test/std/utilities/tuple/tuple.tuple/

[PATCH] D28229: [libcxx] Fix testing of the externally-threaded library build after r290850

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. Accepting and Closing. This was committed in r290878. https://reviews.llvm.org/D28229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D28174: [libcxx] [NFC] Rename _LIBCPP_TYPE_VIS_ONLY to _LIBCPP_TEMPLATE_VIS.

EricWF accepted this revision. EricWF added a reviewer: EricWF. EricWF added a comment. This revision is now accepted and ready to land. Accepting since there were no objections. https://reviews.llvm.org/D28174 ___ cfe-commits mailing list cfe-commi

[PATCH] D28174: [libcxx] [NFC] Rename _LIBCPP_TYPE_VIS_ONLY to _LIBCPP_TEMPLATE_VIS.

EricWF closed this revision. EricWF added a comment. Committed in r291035. https://reviews.llvm.org/D28174 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28220: provide Win32 native threading

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. This LGTM. I'm sure we can flush out any bugs once we get the tests running. Repository: rL LLVM https://reviews.llvm.org/D28220 ___ cfe-comm

[PATCH] D28212: typeinfo: provide a partial implementation for Win32

EricWF added inline comments. Comment at: include/typeinfo:75 +#elif defined(_WIN32) +#define _LIBCPP_HAS_WINDOWS_TYPEINFO +#else compnerd wrote: > rnk wrote: > > Is _WIN32 the right condition? It seems like this is intended to match the > > MS ABI RTTI structur

[PATCH] D28131: [libcxx] Fix PR31402: map::__find_equal_key has undefined behavior.

EricWF accepted this revision. EricWF added a reviewer: EricWF. EricWF added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D28131#632516, @vsk wrote: > LGTM. I'm not sure what to do for a test. Have you tried checking the IR for > the affected object fil

[PATCH] D12644: Using -isysroot on Apple platform

EricWF resigned from this revision. EricWF removed a reviewer: EricWF. EricWF added a comment. I'm resigning as a reviewer. I suggested an alternative fix and explained why this fix wasn't correct in a comment above. Please re-add me as a reviewer if you disagree with my analysis. https://revi

[PATCH] D20660: Remove `auto_ptr` in C++17.

EricWF added inline comments. Comment at: include/memory:1962 +#if _LIBCPP_STD_VER <= 14 || defined(_LIBCPP_NO_REMOVE_AUTOPTR) template I would love to have a semi-consistent naming scheme for macros which re-enable removed C++17 features. Maybe `_LIBCPP_ENA

[PATCH] D28316: [libc++] Cleanup and document <__threading_support>

EricWF added a comment. @compnerd Just waiting on @rmaprath before committing. This is a breaking change for his use case since this patch changes the semantics of `_LIBCPP_HAS_THREAD_API_EXTERNAL` and `-DLIBCXX_HAS_EXTERNAL_THREAD_API`. https://reviews.llvm.org/D28316 _

[PATCH] D28316: [libc++] Cleanup and document <__threading_support>

EricWF updated this revision to Diff 83201. EricWF added a comment. - Address inline comments https://reviews.llvm.org/D28316 Files: CMakeLists.txt docs/DesignDocs/ThreadingSupportAPI.rst docs/index.rst include/__config include/__config_site.in include/__threading_support lib/CMak

[PATCH] D28316: [libc++] Cleanup and document <__threading_support>

EricWF marked 3 inline comments as done. EricWF added a comment. In https://reviews.llvm.org/D28316#636686, @rmaprath wrote: > (btw, do those docs updates land on some web URL automagically when > committed?). Assuming the builder isn't broken then yes, the docs should automagically update wi

[PATCH] D28316: [libc++] Cleanup and document <__threading_support>

EricWF updated this revision to Diff 83211. EricWF marked an inline comment as done. https://reviews.llvm.org/D28316 Files: CMakeLists.txt docs/DesignDocs/ThreadingSupportAPI.rst docs/index.rst include/__config include/__config_site.in include/__threading_support lib/CMakeLists.txt

[PATCH] D28383: build: add a heuristic to determine the C++ ABI

EricWF added a comment. In https://reviews.llvm.org/D28383#637672, @compnerd wrote: > @rnk happy to add such a macro. Would `__cpp_abi_itanium` and > `__cpp_abi_microsoft` be palatable? I would love these macros. I would prefer if the stock `__config` worked for all but the rarest of configu

[PATCH] D28407: Refer to _LIBCPP_MSVC macro where applicable

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. Although Ironically we can no longer support MSVC due to our use of `#include_next` which MSVC doesn't support. Comment at: utils/google-benchmark/include/benchmark/ma

[PATCH] D28316: [libc++] Cleanup and document <__threading_support>

EricWF updated this revision to Diff 83399. EricWF added a comment. - Merge with master https://reviews.llvm.org/D28316 Files: CMakeLists.txt docs/DesignDocs/ThreadingSupportAPI.rst docs/index.rst include/__config include/__config_site.in include/__threading_support lib/CMakeLists

[PATCH] D28220: provide Win32 native threading

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. This needs to be rebased following the `<__threading_support>` cleanup in r291275. Repository: rL LLVM https://reviews.llvm.org/D28220

[PATCH] D28212: typeinfo: provide a partial implementation for Win32

EricWF added a comment. This isn't applying against master. Could you rebase? Comment at: include/typeinfo:109 protected: +#if defined(_LIBCPP_HAS_MS_ABI_TYPEINFO) +#else Don't we need a constructor here? Repository: rL LLVM https://reviews.llvm.org/D2821

[PATCH] D28212: typeinfo: provide a partial implementation for Win32

EricWF added a comment. Nevermind. Missed the dependency. Would you be willing to kill that dependency for now and we can fix it once we get Clang support? Repository: rL LLVM https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-comm

[PATCH] D27429: [Chrono][Darwin] On Darwin use CLOCK_UPTIME_RAW instead of CLOCK_MONOTONIC

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D27429#637507, @bruno wrote: > @EricWF ok to commit? Yes sorry This LGTM. (I thought I had approved this yesterday, but I forgot to hit enter). ==

[PATCH] D26900: [cmake] Obtain LLVM_CMAKE_PATH from llvm-config

EricWF added a comment. Is there a way to write this patch so that old `llvm-config`'s continue to work, at least for a little while until everybody upgrades? https://reviews.llvm.org/D26900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D28220: provide Win32 native threading

EricWF added a comment. I'm seeing the following build errors on Windows: C:\Users\Eric\workspace\libcxx\include\__threading_support(521,3): warning: cannot delete expression with pointer-to-'void' type 'void *' [-Wdelete-incomplete] delete __data; ^ ~~ C:\Users\Eric\works

[PATCH] D28220: provide Win32 native threading

EricWF added a comment. Just tried the updated patch and I still get one build error: C:\Users\Eric\workspace\libcxx\src\thread.cpp(135,16): error: use of undeclared identifier 'nanosleep' while (nanosleep(&ts, &ts) == -1 && errno == EINTR) Repository: rL LLVM https://reviews.l

[PATCH] D28220: provide Win32 native threading

EricWF added inline comments. Comment at: include/__threading_support:29 +#include +#define WIN32_LEAN_AND_MEAN +#include I think we agreed that we cannot use this macro. Comment at: include/__threading_support:426 +// Condition Variable +_LI

[PATCH] D28220: provide Win32 native threading

EricWF added inline comments. Comment at: include/__threading_support:580 +int __libcpp_tls_create(__libcpp_tls_key* __key, +void(_LIBCPP_TLS_DESTRUCTOR_CC* __at_exit)(void*)) +{ The `_LIBCPP_TLS_DESTRUCTOR_CC` needs to appear an the initi

[PATCH] D28220: provide Win32 native threading

EricWF added inline comments. Comment at: include/__threading_support:30 +#define WIN32_LEAN_AND_MEAN +#include +#include > Can we do as Reid suggests and not expose users to windows.h? I was about to ask the same question. These includes are dragging in the

[PATCH] D28426: [libc++] Tolerate presence of __deallocate macro

EricWF created this revision. EricWF added reviewers: compnerd, smeenai, mclow.lists, majnemer, rnk, rsmith. EricWF added a subscriber: cfe-commits. On Windows the identifier `__deallocate` is defined as a macro by one of the Windows system headers. Previously libc++ worked around this by `#undef

[PATCH] D28220: provide Win32 native threading

EricWF added inline comments. Comment at: include/__threading_support:30 +#define WIN32_LEAN_AND_MEAN +#include +#include smeenai wrote: > EricWF wrote: > > > Can we do as Reid suggests and not expose users to windows.h? > > > > I was about to ask the same que

[PATCH] D28220: provide Win32 native threading

EricWF added inline comments. Comment at: include/__threading_support:30 +#define WIN32_LEAN_AND_MEAN +#include +#include EricWF wrote: > smeenai wrote: > > EricWF wrote: > > > > Can we do as Reid suggests and not expose users to windows.h? > > > > > > I was a

[PATCH] D28426: [libc++] Tolerate presence of __deallocate macro

EricWF added a comment. In https://reviews.llvm.org/D28426#638596, @smeenai wrote: > I'm guessing always defining `_LIBCPP_DISABLE_MACRO_CONFLICT_WARNINGS` on > Windows isn't considered an acceptable workaround either? Not really, since that still breaks users of the `__deallocate` since we `

[PATCH] D28220: provide Win32 native threading

EricWF added inline comments. Comment at: include/__threading_support:30 +#define WIN32_LEAN_AND_MEAN +#include +#include smeenai wrote: > EricWF wrote: > > EricWF wrote: > > > smeenai wrote: > > > > EricWF wrote: > > > > > > Can we do as Reid suggests and not

[PATCH] D28220: provide Win32 native threading

EricWF added inline comments. Comment at: include/__threading_support:30 +#define WIN32_LEAN_AND_MEAN +#include +#include EricWF wrote: > smeenai wrote: > > EricWF wrote: > > > EricWF wrote: > > > > smeenai wrote: > > > > > EricWF wrote: > > > > > > > Can we do

[PATCH] D28220: provide Win32 native threading

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. I just successfully built this on Windows. https://reviews.llvm.org/D28220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://l

[PATCH] D28442: [libc++] Implement terminate(), unexpected() and uncaught_exceptions() on Windows

EricWF created this revision. EricWF added reviewers: compnerd, smeenai, rnk, majnemer. EricWF added a subscriber: cfe-commits. This patch implements the following functions on Windows by forwarding to the MSVCRT: - `get_terminate()` - `set_terminate()` - `terminate()` - `set_unexpected()` - `ge

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

EricWF added a comment. Adding cfe-commits https://reviews.llvm.org/D28441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28442: [libc++] Implement terminate(), unexpected() and uncaught_exceptions() on Windows

EricWF marked an inline comment as done. EricWF added a comment. In https://reviews.llvm.org/D28442#638862, @smeenai wrote: > Hooray for Microsoft for putting all these in msvcrt (their **C** runtime > library) instead of msvcprt (their C++ runtime library), I guess :p Although Boo to `eh.h` f

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

EricWF added inline comments. Comment at: CMakeLists.txt:388 +# non-debug DLLs +remove_flags("/D_DEBUG" "/MTd" "/MDd" "/MT" "/Md" "/RTC1") + smeenai wrote: > cl (and presumably clang-cl) also accepts all these flags with a leading dash > instead of a leading sla

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

EricWF added inline comments. Comment at: CMakeLists.txt:43 +if (WIN32 AND NOT MINGW) + set(LIBCXX_TARGETING_WINDOWS ON) +else() smeenai wrote: > Not the biggest fan of this name, since it's not obvious why MinGW shouldn't > count as targeting Windows. I though

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

EricWF added inline comments. Comment at: CMakeLists.txt:43 +if (WIN32 AND NOT MINGW) + set(LIBCXX_TARGETING_WINDOWS ON) +else() EricWF wrote: > smeenai wrote: > > Not the biggest fan of this name, since it's not obvious why MinGW > > shouldn't count as targeti

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

EricWF added inline comments. Comment at: lib/CMakeLists.txt:113 + add_library_flags(msvcrt) # C runtime startup files + add_library_flags(iso_stdio_wide_specifiers) +endif() halyavin wrote: > EricWF wrote: > > smeenai wrote: > > > Maybe add a comment explainin

[PATCH] D28441: [libc++] [CMake] Link with /nodefaultlibs on Windows

EricWF updated this revision to Diff 83526. EricWF added a comment. - Add comment explaining need for `iso_stdio_wide_specifiers.lib`. https://reviews.llvm.org/D28441 Files: CMakeLists.txt cmake/Modules/HandleLibcxxFlags.cmake lib/CMakeLists.txt test/libcxx/test/config.py Index: test/

[PATCH] D27424: Add the diagnose_if attribute to clang.

EricWF added a comment. Awesome! I'm so excited for this! https://reviews.llvm.org/D27424 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support

EricWF added a comment. Also look in `__mutex` where libc++ defines its own macros for the annotations. https://reviews.llvm.org/D28520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D28520: Disable -Wthread-safety-analysis for some functions in __thread_support

EricWF added a comment. In https://reviews.llvm.org/D28520#641569, @dim wrote: > In https://reviews.llvm.org/D28520#641563, @EricWF wrote: > > > Also look in `__mutex` where libc++ defines its own macros for the > > annotations. > > > I assume you mean `__mutex_base`. Do we want to reuse the ma

[PATCH] D31260: [libc++] Work around C1XX bug which breaks poisoned hash tests.

EricWF created this revision. This is my attempt to work around the C1XX bug described to me by @BillyONeal. https://reviews.llvm.org/D31260 Files: test/support/poisoned_hash_helper.hpp test/support/test.workarounds/c1xx_broken_nullptr_conversion_operator.pass.cpp test/support/test_macros

[PATCH] D31163: Implement Pp0156r2 "Variadic Lock Guard, version 5"

EricWF added a comment. So Clang only very recently added support for deduction guides. We'll need to add `_LIBCPP_HAS_NO_DEDUCTION_GUIDES` and use it to guard the declarations and the tests. Also I think the explicit deduction guides are incorrect and unneeded. The deduction guides you declar

[PATCH] D31260: [libc++] Work around C1XX bug which breaks poisoned hash tests.

EricWF added a comment. In https://reviews.llvm.org/D31260#708139, @BillyONeal wrote: > (Though you might want something like this anyway if you try libc++ with > MSVC++ on Windows...) Exactly why I volunteered to do this. As libc++ and MSVC implement C++17 at different rates I suspect we'l

[PATCH] D31163: Implement Pp0156r2 "Variadic Lock Guard, version 5"

EricWF added inline comments. Comment at: include/mutex:177 +template unique_lock(unique_lock) +-> unique_lock; // C++17 + mclow.lists wrote: > And what's with the 'M'? (here and below) No idea, but I would remove the explicit deduction guides from this patc

[PATCH] D31234: Implement P0599 - noexcept for hash functions

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. Although it would be useful to test that `hash>` and `hash>` do not have noexcept call operators. Alternatively should we make `hash` conditionally noexcept for `variant` and `optional`

[PATCH] D31022: Implement P0298R3: `std::byte`

EricWF added a comment. I really dislike the circular dependencies between `` and ``. I suspect we can avoid this by only including `` in ``, which would allow `` to depend on ``. @mclow.lists Does that sound good to you? https://reviews.llvm.org/D31022 ___

[PATCH] D30765: Reexport operator new / delete from libc++abi

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. We'll hear soon enough if this starts causing problems. https://reviews.llvm.org/D30765 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D25241: [libcxx] Improve code generation for vector::clear().

EricWF accepted this revision. EricWF added a comment. Still LGTM Comment at: test/std/containers/sequences/vector/vector.modifiers/clear.pass.cpp:18 +#include "min_allocator.h" + +int main() LIBCPP_ASSERT requires including test_macros.h https://reviews.llv

[PATCH] D31163: Implement Pp0156r2 "Variadic Lock Guard, version 5"

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. I'll send something to the reflectors about the explicit deduction guides over the weekend. https://reviews.llvm.org/D31163 ___ cfe-commit

[PATCH] D31022: Implement P0298R3: `std::byte`

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. I still think we can come up with a better include structure, but we have plenty of time to figure that out before the next release. I see no reason to hold this up. @mclow.lists is there a fe

[PATCH] D31022: Implement P0298R3: `std::byte`

EricWF added inline comments. Comment at: test/std/language.support/support.types/byteops/xor.assign.pass.cpp:13 + +// XFAIL: c++98, c++03, c++11, c++14 + Nit. These should be `// UNSUPPORTED`. `XFAIL` is for bugs we need to fix. https://reviews.llvm.org/D3102

[PATCH] D31399: [coroutines] Handle get_return_object_on_allocation_failure

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. This LGTM minus inline comments. Comment at: lib/Sema/SemaCoroutine.cpp:793 + LookupResult Found(S, DN, Loc, Sema::LookupMemberName); + // Suppress diagnostics when a priva

[PATCH] D30837: [libcxx] Support for shared_ptr

EricWF added a comment. This patch seems to support constructing a `shared_ptr` without providing a non-default deleter. I don't think this should work because the default deleter will attempt to free a function pointer, which is never valid. (Although I think this case will still cause a compi

[PATCH] D30837: [libcxx] Support for shared_ptr

EricWF added inline comments. Comment at: include/memory:3933 +typedef __shared_ptr_pointer<_Yp*, default_delete<_Yp>, _AllocT > _CntrlBlk; +__cntrl_ = new _CntrlBlk(__p, default_delete<_Yp>(), _AllocT()); __hold.release(); Quuxplusone wrote: > Eric

[PATCH] D31363: [libc++] Remove cmake glob for source files

EricWF added a comment. Thanks for fixing this. Sucks that we have to manually enumerate source files but this is the correct solution according to CMake. Would it be easy to still use `glob` to verify that none of the source files have accidentally been forgotten? Comment a

[PATCH] D29818: [libcxx] Threading support: Attempt to externalize system_clock::now() and steady_clock::now() implementations

EricWF added a comment. I really dislike that `__libcpp_clock_monotonic` and `__libcpp_clock_realtime` are never declared, and are expected to be magically defined by the external threading header. This makes the configuration seem incorrect and unused. Also the previous threading support chang

[PATCH] D31178: [libcxxabi] Fix exception address alignment test for EHABI

EricWF added a comment. For the most part this LGTM other than the nits mentioned. Comment at: test/libcxxabi/test/config.py:34 +'libunwind_src', +os.path.join(self.libcxxabi_src_root, '/../libunwind/src')) I think the correct default

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

EricWF added a comment. I'm a bit confused by the description of this change. Libc++ has been enabling the new/delete definitions in its dylib since forever and I've never experienced a link error. Did you mean to say the link error occurs only when libc++abi doesn't define them? https://revi

[PATCH] D30268: Avoid copy of __atoms when char_type is char

EricWF added a comment. OK, so ABI wise this seems good. I'll need a fresh head to review the locale changes for correctness. I'll get around to that tomorrow. Comment at: libcxx/include/locale:891 +#ifndef _LIBCPP_ABI_OPTIMIZED_LOCALE // signed Would it be

[PATCH] D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early

EricWF created this revision. This patch fixes http://llvm.org/PR28954 using the `init_priority` attribute. All supported compilers accept this attribute, including clang-cl. I'm only putting this up for review because IDK how to write a test for it. Can anybody suggest a way to test this? htt

[PATCH] D29930: Add `__reference_binds_to_temporary` trait for checking safe reference initialization.

EricWF added a comment. @rsmith gentle ping. https://reviews.llvm.org/D29930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31078: [libunwind] Clean up macro usage.

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM minus the suggested cleanup. Comment at: include/__libunwind_config.h:15 !defined(__ARM_DWARF_EH__) #define _LIBUNWIND_ARM_EHABI 1 #endif This sh

[PATCH] D31375: Add docs for libunwind

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. This LGTM. We should also get a bot setup to build it. https://reviews.llvm.org/D31375 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D31375: Add docs for libunwind

EricWF added a comment. In https://reviews.llvm.org/D31375#710897, @jroelofs wrote: > In https://reviews.llvm.org/D31375#710891, @compnerd wrote: > > > What happens when you try building it in tree? > > > The docs-libunwind-html target is missing, and the docs don't get built. Shouldn't that be

[PATCH] D31399: [coroutines] Handle get_return_object_on_allocation_failure

EricWF added a comment. Woops. this patch also needed changes to `TreeTransform.h`. I'll add those tonight. Repository: rL LLVM https://reviews.llvm.org/D31399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

EricWF added a comment. In https://reviews.llvm.org/D31272#711860, @mehdi_amini wrote: > In https://reviews.llvm.org/D31272#711804, @EricWF wrote: > > > I'm a bit confused by the description of this change. Libc++ has been > > enabling the new/delete definitions in its dylib since forever and I'

[PATCH] D27387: [libc++] Add a key function for bad_function_call

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. This LGTM, but I have a question: Should we add these dylib definitions right away so that it's easier to adopt the ABI change in future? This will allow legacy dylibs to support this ABI chan

[PATCH] D26830: [libcxx] Add string_view literals

EricWF added a comment. @AntonBikineev when will you be able to make he requested changes? I would like to land this ASAP. https://reviews.llvm.org/D26830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D24371: Add diagnostics to require_constant_initialization

EricWF added a comment. @rsmith ping. Repository: rL LLVM https://reviews.llvm.org/D24371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D11781: Refactored pthread usage in libcxx

EricWF resigned from this revision. EricWF added a comment. This revision is no longer relevant after. The `__threading_support` changes have applied changes similar to this. Repository: rL LLVM https://reviews.llvm.org/D11781 ___ cfe-commits ma

[PATCH] D23796: [libcxx] Make it possible to test static builds of libc++ on OSX

EricWF added a comment. Is this still needed? https://reviews.llvm.org/D23796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

EricWF added a comment. In https://reviews.llvm.org/D31272#711876, @mehdi_amini wrote: > Strange. So installing the command line tools is not enough. It has to be > that CMAKE_OSX_SYSROOT is only defined on Apple internal install maybe? Are you sure you're starting with a clean CMake build dir

[PATCH] D31487: [coroutines] Fix rebuilding of implicit and dependent coroutine statements.

EricWF created this revision. Certain implicitly generated coroutine statements, such as the calls to 'return_value()' or `return_void()` or `get_return_object_on_allocation_failure()`, cannot be built until the promise type is no longer dependent. This means they are not built until after the

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

EricWF accepted this revision. EricWF added inline comments. This revision is now accepted and ready to land. Comment at: lib/CMakeLists.txt:155 +# We can't use the "-reexported_symbols_list" when we build the +# new/delete operators as part of the dylib: the link

[PATCH] D31487: [coroutines] Fix rebuilding of implicit and dependent coroutine statements.

EricWF updated this revision to Diff 93441. EricWF added a comment. - Remove unneeded asserts. https://reviews.llvm.org/D31487 Files: include/clang/AST/StmtCXX.h lib/AST/StmtCXX.cpp lib/Sema/CoroutineBuilder.h lib/Sema/SemaCoroutine.cpp lib/Sema/TreeTransform.h test/SemaCXX/coroutin

[PATCH] D31486: libc++ testing: allow to provide a path for `use_system_cxx_lib`

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D31486 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D17053: [libcxx]: vector: Use < instead of != to improve failure mode

EricWF resigned from this revision. EricWF added a comment. This review seems stuck and dead. Resigning as a reviewer. Please re-add me if you want this revision to proceed. https://reviews.llvm.org/D17053 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D31487: [coroutines] Fix rebuilding of implicit and dependent coroutine statements.

EricWF updated this revision to Diff 93508. EricWF added a comment. - Fix insane definition of `hasDependentPromiseType()` https://reviews.llvm.org/D31487 Files: include/clang/AST/StmtCXX.h lib/AST/StmtCXX.cpp lib/Sema/CoroutineBuilder.h lib/Sema/SemaCoroutine.cpp lib/Sema/TreeTransfo

<    2   3   4   5   6   7   8   9   10   11   >