[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

ldionne added a comment. This is a very tricky change since it touches all the input stream operations, both formatted and unformatted. However, I think it's an important change as it fixes extremely broken behavior. The paper I'm working on to fix this in the Standard is https://github.com/ld

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

This revision was automatically updated to reflect the committed changes. Closed by commit rCXX338122: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY (authored by ldionne, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://rev

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

ldionne added a comment. Note: I resolved Eric's comments before pushing. Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

ldionne created this revision. ldionne added reviewers: EricWF, mclow.lists. Herald added subscribers: cfe-commits, dexonsmith, christof. ldionne added a comment. This is the second step of the proposal aiming to remove uses of `__always_inline__` in the visibility macros. This proposal is docume

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

ldionne added a comment. This is the second step of the proposal aiming to remove uses of `__always_inline__` in the visibility macros. This proposal is documented on the list here: http://lists.llvm.org/pipermail/cfe-dev/2018-July/058633.html Repository: rCXX libc++ https://reviews.llvm.or

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

ldionne updated this revision to Diff 157696. ldionne added a comment. Document the macro in the official documentation instead of the header, per Eric's comment on a previous patch of mine. Repository: rCXX libc++ https://reviews.llvm.org/D49914 Files: libcxx/docs/DesignDocs/VisibilityMac

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

ldionne added inline comments. Comment at: libcxx/include/__config:798 -// Just so we can migrate to _LIBCPP_HIDE_FROM_ABI gradually. -#define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI - -#ifndef _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY -# if !defined(_LIBCPP_DISABLE_VI

[PATCH] D49927: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

ldionne added a comment. Just to make sure I understand properly: this means we will use newlib's implementation of `iswcntrl_l` & friends instead of our own emulation (which is an ODR violation currently going unnoticed)? And this is OK because newlib provides `iswcntrl_l` & friends starting a

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

ldionne added a comment. It would be nice if all the TMP required to determine whether to call `__move_construct_forward(..., true_type)` or `__move_construct_forward(..., false_type)` was done in `__move_construct_forward` itself (or a helper). This way, callers wouldn't have to do it themselv

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM Comment at: include/vector:296 +_LIBCPP_INLINE_VISIBILITY +inline void +__copy_construct_forward(_Alloc& __a, _Iter __begin1, _Iter __end1, Quuxpluson

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

ldionne added a comment. In https://reviews.llvm.org/D49317#1178767, @Quuxplusone wrote: > @ldionne: I don't know if your "LGTM" is necessarily sufficient to commit > this or not; but either way, I don't have commit privs, so could I ask you > (or someone else) to commit this on my behalf? Than

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

ldionne added a comment. In https://reviews.llvm.org/D49317#1180200, @mclow.lists wrote: > I am not in favor of this patch. > > I'm in favor of fixing the problem that Arthur has described, but not like > this, for the following reasons: > > - Conceptually, these are (similar to) "Allocator-base

[PATCH] D49317: Move __construct_forward (etc.) out of std::allocator_traits.

ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. After thinking about this for some more, I'm not sure this patch is worth doing in its current form. The minimal patch for allowing specializations of `allocator_traits` would be:

[PATCH] D50008: [libc++] Remove _LIBCPP_BUILDING_XXX macros, which are redundant since _LIBCPP_BUILDING_LIBRARY

ldionne created this revision. ldionne added a reviewer: mclow.lists. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, christof. As suggested by Marshall in https://reviews.llvm.org/D49914 Repository: rCXX libc++ https://reviews.llvm.org/D50008 Files: lib

[PATCH] D50106: [libc++] Fix tuple assignment from type derived from a tuple-like

ldionne created this revision. ldionne added reviewers: EricWF, mclow.lists. Herald added subscribers: cfe-commits, dexonsmith, christof. The implementation of operator= for tuple currently diverges from the way the Standard specifies them, which leads to subtle cases where the behavior is not as

[PATCH] D50008: [libc++] Remove _LIBCPP_BUILDING_XXX macros, which are redundant since _LIBCPP_BUILDING_LIBRARY

This revision was automatically updated to reflect the committed changes. Closed by commit rL338475: [libc++] Remove _LIBCPP_BUILDING_XXX macros, which are redundant since… (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D49997: [libcxx] Fix _LIBCPP_NO_EXCEPTIONS redefined warning

ldionne closed this revision. ldionne added a comment. Committed as r32f108a0736 Repository: rCXX libc++ https://reviews.llvm.org/D49997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D50205: [libc++] Add availability markup for aligned new/delete

ldionne created this revision. ldionne added reviewers: EricWF, vsapsai. Herald added subscribers: cfe-commits, dexonsmith, christof. The aligned allocation and deallocation functions were added in the dylib in Mac OSX 10.13. This commit does two things: first, it adds availability markup to those

[PATCH] D50205: [libc++] Add availability markup for aligned new/delete

ldionne added a comment. In https://reviews.llvm.org/D50205#1186419, @ahatanak wrote: > I just wanted to make sure that this doesn't have the same problem as > https://reviews.llvm.org/D34556. Is that correct? > > The patch was reverted in r306859. https://reviews.llvm.org/D34574#791158 > expla

[PATCH] D37302: [Headers] Define *_HAS_SUBNORM for FLT, DBL, LDBL

ldionne accepted this revision. ldionne added a comment. What are we waiting for to move forward with this change? Repository: rC Clang https://reviews.llvm.org/D37302 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D50276: [clang] Fix broken include_next in float.h

ldionne created this revision. ldionne added a reviewer: jfb. Herald added subscribers: cfe-commits, dexonsmith. The code defines __FLOAT_H and then includes the next , which is guarded on __FLOAT_H so it gets skipped entirely. This commit uses the header guard __CLANG_FLOAT_H, like other headers

[PATCH] D50205: [libc++] Add availability markup for aligned new/delete

ldionne updated this revision to Diff 159110. ldionne added a comment. Only include availability markup in this patch, do not change whether and when aligned allocation functions are used by the library. This will be handled in a separate patch. Repository: rCXX libc++ https://reviews.llvm.

[PATCH] D49914: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro

This revision was automatically updated to reflect the committed changes. Closed by commit rL339012: [libc++] Add the _LIBCPP_HIDE_FROM_ABI_AFTER_V1 macro (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D49914?vs=1

[PATCH] D52675: [clang] Properly apply attributes on explicit instantiations of static data members

This revision was automatically updated to reflect the committed changes. Closed by commit rL344146: [clang] Properly apply attributes on explicit instantiations of static data… (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://revie

[PATCH] D50739: Clean up macros to detect underling C library functionality

ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. Marking as "request changes" to get off my review queue until the problem I pointed w.r.t. `TEST_HAS_C11_FEATURES` and `_LIBCPP_HAS_C11_FEATURES` always being undefined is resolved

[PATCH] D53839: [CMake][Fuchsia] Drop the LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT

ldionne accepted this revision. ldionne added a comment. Please LMK if you see nice consequences of this. I'm trying to gather "real stories" where this helped. Repository: rC Clang https://reviews.llvm.org/D53839 ___ cfe-commits mailing list cf

[PATCH] D56984: [libunwind] Silence warnings about unused parameters

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM, but would it make sense to have a macro like `_LIBUNWIND_MAYBE_UNUSED(var)` instead? Repository: rUNW libunwind CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56984/new/ ht

[PATCH] D53985: Use C++11 fallthrough attribute syntax when available and add a break

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. Would it make sense to add the GNU spelling to the attribute in Clang? https://reviews.llvm.org/D53985 ___ cfe-commits mailing list cfe-commits

[PATCH] D53700: Support _Clang as a scoped attribute identifier

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. I'm fine with `_Clang` or anything else too. Thanks for doing this! Comment at: lib/Basic/Attributes.cpp:18 if (ScopeName == "__gnu__") -ScopeName = ScopeName.slice(

[PATCH] D51762: First part of the calendar stuff

ldionne added a comment. Can this be closed? My understanding is that this has been committed now. https://reviews.llvm.org/D51762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54630: Move detection of libc++ include dirs to Driver on MacOS

ldionne added a comment. In https://reviews.llvm.org/D54630#1301283, @arphaman wrote: > I don't think we want to move the logic to add a libc++ path to the driver. My opinion is not very educated because I don't have a lot of experience/knowledge of Clang, but from my perspective it does make

[PATCH] D51268: [libc++] Implement P0487R1 - Fixing operator>>(basic_istream&, CharT*)

ldionne accepted this revision. ldionne added inline comments. This revision is now accepted and ready to land. Comment at: include/istream:601 +streamsize __n = __is.width(); +if (__n <= 0) +__n = numeric_limits::max() / sizeof(_CharT) - 1; Shoul

[PATCH] D51268: [libc++] Implement P0487R1 - Fixing operator>>(basic_istream&, CharT*)

ldionne added a comment. Please also update https://libcxx.llvm.org/cxx2a_status.html. Repository: rCXX libc++ https://reviews.llvm.org/D51268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D51268: [libc++] Implement P0487R1 - Fixing operator>>(basic_istream&, CharT*)

ldionne accepted this revision. ldionne added a comment. LGTM Repository: rCXX libc++ https://reviews.llvm.org/D51268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53207: Fix bug 26547 - alignof should return ABI alignment, not preferred alignment

ldionne added subscribers: dexonsmith, ldionne. ldionne added a comment. I'm surprised that nobody discussed the potential for ABI breakage of this change, which changes the result of the `alignof` operator. I'm trying to evaluate what to do in libc++ as a result of this change (now `std::align

[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.

ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. Herald added a subscriber: jkorous. 1. Just to make sure I understand; this patch has nothing to do with https://reviews.llvm.org/D48342, they are entirely orthogonal. Is this corre

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

ldionne requested changes to this revision. ldionne added a comment. This revision now requires changes to proceed. Herald added a subscriber: jkorous. I believe this patch fixes an important QOI bug: see http://llvm.org/PR37574. I wholeheartedly agree with Eric that allocators-over-const are an

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

ldionne added a comment. If you use some of my suggestions, please make sure it compiles in C++03 mode. I might be using some C++11 features (not sure whether default argument for template parameters is C++03). Comment at: libcxx/test/std/containers/sequences/vector/vector.c

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

ldionne added inline comments. Comment at: libcxx/include/memory:1645 -template +template _LIBCPP_INLINE_VISIBILITY Quuxplusone wrote: > ldionne wrote: > > Coming at it from a slightly different angle, I would think this is what we > > want:

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

ldionne added inline comments. Comment at: libcxx/include/memory:1658 +|| !__has_construct::value) && + is_trivially_move_constructible<_DestTp>::value, void Quuxplusone wrote: > Shouldn't this be something like `is_trivia

[PATCH] D58921: [CMake] Tell libc++ that we're using compiler-rt on Apple platforms

ldionne created this revision. ldionne added reviewers: beanz, arphaman. Herald added subscribers: cfe-commits, jdoerfert, dexonsmith, jkorous, mgorny, dberris. Herald added a reviewer: EricWF. Herald added a project: clang. Repository: rC Clang https://reviews.llvm.org/D58921 Files: clang/

[PATCH] D58966: [libc++] Fix failures on GCC

ldionne created this revision. ldionne added a reviewer: EricWF. Herald added a reviewer: jfb. Herald added subscribers: cfe-commits, jdoerfert, jfb, dexonsmith, jkorous, christof. Herald added a project: clang. In https://reviews.llvm.org/D58201, we turned memory_order into an enum class in C++2

[PATCH] D58921: [CMake] Tell libc++ that we're using compiler-rt on Apple platforms

ldionne added a comment. Ping. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58921/new/ https://reviews.llvm.org/D58921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D58921: [CMake] Tell libc++ that we're using compiler-rt on Apple platforms

ldionne added a comment. In D58921#1423294 , @smeenai wrote: > Is this to work around > https://github.com/llvm/llvm-project/blob/f84083b4dbb1ddb6d2783400f11121f490cdb5a8/libcxx/lib/CMakeLists.txt#L312? Yes, precisely. Note that I think this option shou

[PATCH] D31739: Add markup for libc++ dylib availability

ldionne added inline comments. Comment at: libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/lit.local.cfg:1 +if 'availability' in config.available_features: +config.unsupported = True mehdi_amini wrote: > thakis wrote: > > Did you mea

[PATCH] D58921: [CMake] Tell libc++ that we're using compiler-rt on Apple platforms

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC355927: [CMake] Tell libc++ that we're using compiler-rt on Apple platforms (authored by ldionne, committed by ). Changed

[PATCH] D59394: [Sema] De-duplicate some availability checking logic

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. I can confirm this fixed my original problem. It's also close to the patch I attempted writing myself earlier this week -- but this one is better of course. Repository: rC Clang CHANGES

[PATCH] D59621: [libcxx] [test] Add (void) casts to operator new calls, to suppress warnings generated by [[nodiscard]].

ldionne added a comment. In D59621#1439870 , @BillyONeal wrote: > This didn't work out, we'll just skip the tests for MSVC++ for now. I'd like to understand what's breaking on MSVC so we can address the problem properly -- but I won't push if you just w

[PATCH] D60069: Declare std::tuple_element as struct instead of class

ldionne added a comment. In D60069#1449937 , @jdoerrie wrote: > Right, there shouldn't be any inheritance. Some of the `public:` access > specifications are now redundant, though. Can you please fix those? Repository: rCXX libc++ CHANGES SINCE LAST

[PATCH] D60023: [libcxx] [test] Fix inability to rebind poca_alloc in string.cons/copy_alloc.pass.cpp.

ldionne requested changes to this revision. ldionne added inline comments. This revision now requires changes to proceed. Comment at: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp:125 test_assign(s1, s2); assert(s1 == p1); assert(s2 == p2);

[PATCH] D60069: Declare std::tuple_element as struct instead of class

This revision was automatically updated to reflect the committed changes. Closed by commit rL357411: [libc++] Declare std::tuple_element as struct instead of class (authored by ldionne, committed by ). Herald added subscribers: llvm-commits, christof. Herald added a project: LLVM. Changed prior t

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

ldionne reopened this revision. ldionne added a comment. This revision is now accepted and ready to land. In D49863#1452126 , @aprantl wrote: > It looks like this may have broken some LLDB data formatters: > http://green.lab.llvm.org/green/view/LLDB/job/l

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

ldionne added a comment. In D49863#1452154 , @ldionne wrote: > In D49863#1452126 , @aprantl wrote: > > > It looks like this may have broken some LLDB data formatters: > > http://green.lab.llvm.org/green/view/LLDB/j

[PATCH] D60626: [clang] Aligned allocation is actually supported in macosx 10.13

ldionne created this revision. ldionne added a reviewer: ahatanak. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. In r350649, I changed aligned allocation from being available starting in macosx10.13 to macosx10.14. However, aligned allocation is indeed

[PATCH] D60626: [clang] Aligned allocation is actually supported in macosx 10.13

This revision was automatically updated to reflect the committed changes. Closed by commit rL358409: [clang] Aligned allocation is actually supported in macosx 10.13 (authored by ldionne, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commi

[PATCH] D60626: [clang] Aligned allocation is actually supported in macosx 10.13

ldionne added a comment. Just as a note to someone that might get here through CI failures: I expect this will break back-deployment CIs for libc++ that use recent Clangs (because the XFAILs in libc++ won't be correct anymore). I have a patch ready to address that but I want to see exactly how

[PATCH] D60626: [clang] Aligned allocation is actually supported in macosx 10.13

ldionne added a comment. Reverted in r358437 because of unforeseen CI failures. The failures I was expecting are in libc++, not in compiler-rt. I'll investigate locally and re-apply. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60626/new/ https://reviews.llvm.o

[PATCH] D60417: [libunwind] Add support for ARMv7-M architecture which uses the Thumb 2 ISA (unified syntax)

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. I'm relaying approval to @mstorsjo, who seems okay with the change. Repository: rUNW libunwind CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60417/new/ https://reviews.llvm.org/D

[PATCH] D61032: [clang] Avoid defining duplicate header search paths for libc++ on Darwin

ldionne created this revision. ldionne added a reviewer: jfb. Herald added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. Before this patch, we would add both /usr/include/c++/v1 and /usr/include/c++/v1 to the header search paths for

[PATCH] D61032: [clang] Avoid defining duplicate header search paths for libc++ on Darwin

ldionne added a comment. I will build a significant codebase with the change and report on the results, since we lack good tests for the removal of `/usr/include/c++/v1`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61032/new/ https://reviews.llv

[PATCH] D48292: use modern type trait implementations when available

ldionne added a comment. I think this broke the C++03 bots: http://green.lab.llvm.org/green/view/Libcxx/job/libc++%20and%20libc++abi%20trunk/CI_ARCH=64,CI_EXCEPTIONS=ON,CI_STD=c++03/103/consoleFull Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48292/new/ http

[PATCH] D61032: [clang] Avoid defining duplicate header search paths for libc++ on Darwin

ldionne abandoned this revision. ldionne added a comment. I decided to use a different approach and first refactor the header search logic from CC1 to the driver. A patch should come eventually when I've tested it properly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D48292: use modern type trait implementations when available

ldionne added a comment. In D48292#1490060 , @thakis wrote: > This test is broken with Xcode 9: > > [...] > > > Does this just need an `// XFAIL: apple-clang-9` or is this unexpected? This seems to have been fixed in LLVM Clang trunk since the 8 releas

[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

ldionne added a comment. I've only been lurking but FWIW (1) above makes the most sense to me, unless the Standard clearly draws a distinction between *constructed* and *initialized* in the way that was described, in which case (3) is the right approach. However, I would wait for at least a CWG

[PATCH] D49587: [CMake] Support exporting runtimes using the CMake export

ldionne added a comment. In D49587#1493585 , @ldionne wrote: > I think the right way to do this was to use `install(export)`? Oh, and you are doing this, but in `llvm/runtimes`. Why don't we handle this per project? I don't think we should assume that t

[PATCH] D49587: [CMake] Support exporting runtimes using the CMake export

ldionne added a comment. I think the right way to do this was to use `install(export)`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49587/new/ https://reviews.llvm.org/D49587 ___ cfe-commits mailing

[PATCH] D61667: Assume `__cxa_allocate_exception` returns an under-aligned memory on Darwin if the version of libc++abi isn't new enough to include the fix in r319123

ldionne added inline comments. Comment at: include/clang/Basic/AlignedExceptionObject.h:15 + +#ifndef LLVM_CLANG_BASIC_ALIGNED_ALLOCATION_H +#define LLVM_CLANG_BASIC_ALIGNED_ALLOCATION_H The guards look wrong. Comment at: include/clang/Basic/Al

[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

ldionne added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4683-4686 + "Member '%0' marked with 'exclude_from_explicit_instantiation' attribute is " + "not defined but an explicit template instantiation declaration exists. " + "Reliance on this

[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

ldionne updated this revision to Diff 165766. ldionne added a comment. Reformat warning message and run clang-format on changed lines. Repository: rC Clang https://reviews.llvm.org/D51789 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang

[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

ldionne updated this revision to Diff 165768. ldionne added a comment. Add example of how to use the attribute, per Aaron's comment. Repository: rC Clang https://reviews.llvm.org/D51789 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/include/clang/B

[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

ldionne added a comment. I'm thinking about this some more, and I'm wondering whether it would be a viable solution to just exclude members marked with hidden visibility from explicit template instantiations (and declarations thereof). I thought I had been convinced by Hubert

[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

ldionne added a comment. In https://reviews.llvm.org/D51789#1238396, @rjmccall wrote: > That may work for libc++'s purposes, but it's clearly inappropriate as a > compiler rule. There are good reasons why something with hidden visibility > would need to be explicitly instantiated. I take you

[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

ldionne updated this revision to Diff 166590. ldionne added a comment. Fix warnings on undefined entities Repository: rC Clang https://reviews.llvm.org/D51789 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/AttrDocs.td clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaDec

[PATCH] D52393: [libunwind][NFC] Suppress unused parameter warnings

ldionne created this revision. ldionne added a reviewer: EricWF. Herald added subscribers: cfe-commits, dexonsmith, chrib, christof. Repository: rUNW libunwind https://reviews.llvm.org/D52393 Files: libunwind/src/Registers.hpp Index: libunwind/src/Registers.hpp

[PATCH] D52394: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin

ldionne updated this revision to Diff 166613. ldionne added a comment. Remove the change to the Darwin ABI list file, which was meant for a different commit. Repository: rCXX libc++ https://reviews.llvm.org/D52394 Files: libcxx/lib/abi/CMakeLists.txt Index: libcxx/lib/abi/CMakeLists.txt

[PATCH] D52394: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin

ldionne created this revision. ldionne added reviewers: dim, EricWF. Herald added subscribers: libcxx-commits, cfe-commits, dexonsmith, christof, krytarowski, mgorny, emaste. ldionne updated this revision to Diff 166613. ldionne added a comment. Remove the change to the Darwin ABI list file, whic

[PATCH] D52393: [libunwind][NFC] Suppress unused parameter warnings

This revision was automatically updated to reflect the committed changes. Closed by commit rL342811: [libunwind][NFC] Suppress unused parameter warnings (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52393?vs=166

[PATCH] D52396: [libcxx] Document new symbols __u64toa and __u32toa on Darwin

ldionne created this revision. ldionne added a reviewer: EricWF. Herald added subscribers: libcxx-commits, cfe-commits, dexonsmith, christof. This is the counterpart for https://reviews.llvm.org/D50130 and https://reviews.llvm.org/D52391 on Darwin. Repository: rCXX libc++ https://reviews.llvm

[PATCH] D52394: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin

This revision was automatically updated to reflect the committed changes. Closed by commit rL342813: [libcxx] Fix the definition of the check-cxx-abilist target on Darwin (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm

[PATCH] D52397: [libc++] Remove Fuchsia-specific knowledge to pick the ABI version

ldionne created this revision. ldionne added reviewers: phosek, EricWF. Herald added subscribers: libcxx-commits, cfe-commits, dexonsmith, christof, mgorny. The ABI version used by libc++ is a configuration option just like any other configuration option. It is a knob that can be used by vendors

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. In https://reviews.llvm.org/D45639#1242444, @phosek wrote: > In https://reviews.llvm.org/D45639#1193112, @ldionne wrote: > > > @phosek I don't understand how you can expect code compiled with new > > headers to link against an old dylib, unless you're setting the target

[PATCH] D52397: [libc++] Remove Fuchsia-specific knowledge to pick the ABI version

ldionne updated this revision to Diff 166621. ldionne added a comment. Clarify that version 2 of the ABI is _currently_ unstable, it is not _the_ unstable version. Repository: rCXX libc++ https://reviews.llvm.org/D52397 Files: libcxx/CMakeLists.txt libcxx/include/__config Index: libcx

[PATCH] D47395: [libcxx] [test] Remove nonportable locale assumption in basic.ios.members/narrow.pass.cpp

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM and has been committed -- you can close this. https://reviews.llvm.org/D47395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D45639: [Driver] Support default libc++ library location on Darwin

ldionne added a comment. In https://reviews.llvm.org/D45639#1243017, @kristina wrote: > I think on Darwin it would **not** make sense to have `libc++fs.a` ship in > `libc++.dylib` especially considering that it ends up in the dyld cache and > that has a lot of other implications. It would make

[PATCH] D52401: Remove redundant null pointer check in operator delete

ldionne added a comment. Was this true pre-C11 too? If not, then this needs to be guarded by `#if _LIBCPP_STD_VER >= 17`, because C++ is only on top of C11 in C++17 and above (Marshall can double-check this). Repository: rCXX libc++ https://reviews.llvm.org/D52401 ___

[PATCH] D52396: [libcxx] Document new symbols __u64toa and __u32toa on Darwin

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rCXX342849: [libcxx] Document new symbols __u64toa and __u32toa on Darwin (authored by ldionne, committed by ). Changed pri

[PATCH] D46443: Add missing cstdalign header

ldionne added a subscriber: mclow.lists. ldionne added a comment. I believe this header has been deprecated. According to http://eel.is/c++draft/diff.cpp17.library, the effect of this header is nothing. @mclow.lists can you chime in? Repository: rCXX libc++ https://reviews.llvm.org/D46443

[PATCH] D52401: Remove redundant null pointer check in operator delete

ldionne added a comment. You did not get a thumbs up from any of the code owners for libc++. Reverted in r342938. Repository: rL LLVM https://reviews.llvm.org/D52401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D52397: [libc++] Remove Fuchsia-specific knowledge to pick the ABI version

This revision was automatically updated to reflect the committed changes. Closed by commit rL343079: [libc++] Remove Fuchsia-specific knowledge to pick the ABI version (authored by ldionne, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.or

[PATCH] D52660: [CMake][Fuchsia] Use libc++ ABIv2 for Fuchsia toolchain

ldionne added a comment. In https://reviews.llvm.org/D52660#1249709, @kristina wrote: > LGTM. Can we formalize ABIv2 as stable and make it distinct from unstable > (soon to be ABIv3) or are there are any rough corners still left before ABIv2 > and unstable can split in a feature freeze? I'm not

[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

ldionne marked an inline comment as done. ldionne added inline comments. Comment at: clang/lib/Sema/Sema.cpp:648 + !FD->getMostRecentDecl()->isInlined() && + !FD->hasAttr()) continue; rsmith wrote: > What's the purpose of this change? I

[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

This revision was automatically updated to reflect the committed changes. Closed by commit rL343790: [clang] Add the exclude_from_explicit_instantiation attribute (authored by ldionne, committed by ). Changed prior to commit: https://reviews.llvm.org/D51789?vs=166590&id=168302#toc Repository:

[PATCH] D51789: [clang] Add the exclude_from_explicit_instantiation attribute

This revision was automatically updated to reflect the committed changes. Closed by commit rC343790: [clang] Add the exclude_from_explicit_instantiation attribute (authored by ldionne, committed by ). Changed prior to commit: https://reviews.llvm.org/D51789?vs=166590&id=168303#toc Repository:

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

ldionne added a comment. Herald added a subscriber: jkorous. I would like to merge this even though http://wg21.link/p1264 has not been voted into C++ yet, because this is arguably a bug and libc++ differs from other stdlibs in this regard. Once http://wg21.link/p1264 is merged into C++ in one

[PATCH] D58149: [clang] Make sure C99/C11 features in are provided in C++11

ldionne created this revision. ldionne added reviewers: jfb, eli.friedman. Herald added subscribers: cfe-commits, dexonsmith, jkorous. Herald added a project: clang. Previously, those #defines were only provided in C or when GNU extensions were enabled. We need those #defines in C++11 and above, t

[PATCH] D58149: [clang] Make sure C99/C11 features in are provided in C++11

ldionne added a comment. I'll ship this. @eli.friedman I think this is your playground -- if you want any changes to happen post-review, please LMK and I will gladly cooperate. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58149/new/ https://reviews.llvm.org/D58

[PATCH] D58149: [clang] Make sure C99/C11 features in are provided in C++11

This revision was automatically updated to reflect the committed changes. Closed by commit rC353970: [clang] Make sure C99/C11 features in are provided in C++11 (authored by ldionne, committed by ). Changed prior to commit: https://reviews.llvm.org/D58149?vs=186544&id=186703#toc Repo

[PATCH] D50106: [libc++] Fix tuple assignment from type derived from a tuple-like

ldionne added a comment. Herald added a subscriber: jdoerfert. In D50106#1352469 , @EricWF wrote: > Our constructors still have the same bug,. Are you planning on fixing those > as well? Doing so will require a metric butt-tonne of overloads. > If you're

[PATCH] D58149: [clang] Make sure C99/C11 features in are provided in C++11

ldionne added a comment. In D58149#1397390 , @jfb wrote: > In D58149#1397382 , @efriedma wrote: > > > Formally, I don't think C11 is a normative reference for C++11 or C++14, > > only C++17 (see [intro.refs] in the

[PATCH] D58149: [clang] Make sure C99/C11 features in are provided in C++11

ldionne added a comment. I agree with the comments. I think we should strive to be strictly conforming. I make that argument all the time for libc++, so I'll be consistent :-). https://reviews.llvm.org/D58289 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58149/n

<    1   2   3   4   5   6   7   8   >