[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

zoecarver added a comment. @Quuxplusone if this fixes 2843 can you update it in `www/cxx2a_status.html`? Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47344/new/ https://reviews.llvm.org/D47344 ___ cfe-commits mai

[PATCH] D47109: LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()"

zoecarver added a comment. Herald added subscribers: libcxx-commits, ldionne. We also need to mark this off in the status. I can do that in D58879 or create a separate patch. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D47109

[PATCH] D43159: Modernize: Use nullptr more.

zoecarver added inline comments. Comment at: include/memory:1259 template static __two __test(...); template static char __test(typename _Xp::template rebind<_Up>::other* = 0); public: This could be `nullptr` too. Repository: rCXX libc++ CHANG

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

zoecarver added a comment. A few minor things: - the spacing is different from the rest of libc++ (a lot of it was that way before). - since this is a paper it should probably be guarded with `#if _LIBCPP_STD_VER > 17` - update the status in `www`. Also, the standard says explicitly, "A varian

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

zoecarver added inline comments. Comment at: include/variant:1128 +template +struct __overload +: __overload_bool<__overload<_Types...>, bool const volatile> {}; EricWF wrote: > Do we even support volatile types in variant? [[ http://eel.is/c++draft/variant.

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver added a comment. Ping. Anything else I am missing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66862/new/ https://reviews.llvm.org/D66862 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

zoecarver created this revision. zoecarver added reviewers: EricWF, eli.friedman, rsmith, craig.topper, mclow.lists. zoecarver added projects: clang, libc++. zoecarver updated this revision to Diff 220226. zoecarver added a comment. - diff from D67052 , not master

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

zoecarver updated this revision to Diff 220226. zoecarver added a comment. - diff from D67052 , not master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67588/new/ https://reviews.llvm.org/D67588 Files: clang/inc

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

zoecarver updated this revision to Diff 220227. zoecarver added a comment. Generate diff based on D67052 (arc wasn't working so I had to do it manually this time). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67588/new/ https://reviews.llvm.org/D67588

[PATCH] D67588: Add builtin trait for add/remove cv (and similar)

zoecarver added a comment. @lebedev.ri after adding _only_ these builtins to libc++ the type trait tests run several seconds faster. I think if we update _all_ the type traits to use builtins then, it could increase speed of the type trait tests by as much as 50% (if not more). CHANGES SINCE

[PATCH] D67052: Add reference type transformation builtins

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/include/clang/Parse/Parser.h:2606 + DeclSpec::TST ReferenceTransformTokToDeclSpec(); + void ParseAddReferenceTypeSpecifier(DeclSpec &DS); void ParseAtomicSpecifier(DeclSpec &DS); -

[PATCH] D67052: Add reference type transformation builtins

zoecarver marked 5 inline comments as done. zoecarver added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:1612 break; + case DeclSpec::TST_addLValueReferenceType: + case DeclSpec::TST_addRValueReferenceType: EricWF wrote: > This should be merged

[PATCH] D67052: Add reference type transformation builtins

zoecarver updated this revision to Diff 221014. zoecarver added a comment. Herald added a subscriber: erik.pilkington. - address review comments - fix warnings in build Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67052/new/ https://reviews.llvm.o

[PATCH] D67052: Add reference type transformation builtins

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:3412 break; + case UnaryTransformType::RemoveReferenceType: +Out << "3err"; Eric, I looked at your comment, but it seems

[PATCH] D67897: Fix __is_signed builtin

zoecarver created this revision. zoecarver added reviewers: EricWF, rsmith, erichkeane, craig.topper, efriedma. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch fixes the __is_signed builtin type trait to work with floating point types. Now, the builtin will retur

[PATCH] D67897: Fix __is_signed builtin

zoecarver added a comment. In D67897#1678388 , @Quuxplusone wrote: > But `std::is_signed_v` needs to yield `false`. Isn't it cleaner to > leave the compiler builtin matching the library type-trait, so that the > library doesn't have to check for integral

[PATCH] D67899: Fix __is_fundamental to accept nullptr_t

zoecarver created this revision. zoecarver added reviewers: rsmith, EricWF, efriedma, craig.topper, erichkeane. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch updates the __is_fundamental builtin type trait to return true for nullptr_t. Repository: rG LLVM G

[PATCH] D67897: Fix __is_signed builtin

zoecarver added a comment. > (Can I interest you in fixing the misbehaviour for enumeration types too?) Certainly. You mean that `__is_signed` should return false for enumeration types? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67897/new/ htt

[PATCH] D67897: Fix __is_signed builtin

zoecarver updated this revision to Diff 221247. zoecarver added a comment. - fix docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67897/new/ https://reviews.llvm.org/D67897 Files: clang/docs/LanguageExtensions.rst clang/lib/Sema/SemaExprCXX.

[PATCH] D67897: Fix __is_signed builtin

zoecarver updated this revision to Diff 221253. zoecarver added a comment. - fix behavior when passed an enumeration type Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67897/new/ https://reviews.llvm.org/D67897 Files: clang/docs/LanguageExtensio

[PATCH] D67897: Fix __is_signed builtin

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/docs/LanguageExtensions.rst:1165 Note that this currently returns true for enumeration types if the underlying - type is signed, and returns false for floating-point types, in viola

[PATCH] D67897: Fix __is_signed builtin

zoecarver closed this revision. zoecarver added a comment. Resolved by rL372621 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67897/new/ https://reviews.llvm.org/D67897 ___

[PATCH] D67899: Fix __is_fundamental to accept nullptr_t

zoecarver closed this revision. zoecarver added a comment. Resolved by rL372624 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67899/new/ https://reviews.llvm.org/D67899 ___

[PATCH] D69062: Resolve LWG issue 2426

zoecarver created this revision. zoecarver added reviewers: mclow.lists, EricWF, ldionne, rsmith. Herald added subscribers: libcxx-commits, cfe-commits, dexonsmith, christof. Herald added projects: clang, libc++. This patch checks that `expected` is loaded before it is used. Libc++ already does t

[PATCH] D67052: Add reference type transformation builtins

zoecarver added a comment. Herald added a reviewer: mclow.lists. Friendly ping. Other than type mangling, does anything need to be fixed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67052/new/ https://reviews.llvm.org/D67052 __

[PATCH] D69363: [www] Change URLs to HTTPS.

zoecarver added a comment. GitHub PRs can't come soon enough :) There was a round table meeting about that earlier today which I am anxious to hear about. Comment at: clang/www/cxx_status.html:78 Rvalue references - http://www.open-std.org/jtc1/sc22/wg21/docs/paper

[PATCH] D69363: [www] Change URLs to HTTPS.

zoecarver added a comment. Thanks for updating all these links! LGTM :) Comment at: clang/www/cxx_status.html:78 Rvalue references - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2118.html";>N2118 + https://wg21.link/n2118";>N2118 Clang 2.9 --

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

zoecarver created this revision. Herald added projects: clang, libc++. Herald added subscribers: libcxx-commits, cfe-commits. Herald added a reviewer: libc++. zoecarver added reviewers: ldionne, rsmith, EricWF. Herald added a subscriber: dexonsmith. 5ade17e

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

zoecarver marked an inline comment as done. zoecarver added a comment. @ldionne of course! Comment at: libcxx/include/type_traits:901 +// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types. +#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000 +

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

zoecarver updated this revision to Diff 255361. zoecarver added a comment. Fix based on review: - Use static_assert directly in the test - Update run command to work on non-objc platforms (e.g. linux) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

zoecarver updated this revision to Diff 255362. zoecarver added a comment. Diff from master not last commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77519/new/ https://reviews.llvm.org/D77519 Files: clang/lib/Sema/SemaExprCXX.cpp clang/t

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

zoecarver marked 2 inline comments as done. zoecarver added inline comments. Comment at: clang/test/SemaObjCXX/type-traits-is-pointer.mm:1 +// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -fobjc-runtime-has-weak -verify %s +// expected-no-diagnostics ldionne wrote: >

[PATCH] D77519: Fix __is_pointer builtin type trait to work with Objective-C pointer types.

This revision was automatically updated to reflect the committed changes. Closed by commit rGb25ec45809fb: Fix __is_pointer builtin type trait to work with Objective-C pointer types. (authored by zoecarver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D82392: [CodeGen] Add public function to emit C++ destructor call.

This revision was automatically updated to reflect the committed changes. Closed by commit rGe7c5da57a5f3: [CodeGen] Add public function to emit C++ destructor call. (authored by zoecarver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82392/new/ h

[PATCH] D82392: [CodeGen] Add public function to emit C++ destructor call.

zoecarver created this revision. zoecarver added reviewers: rjmccall, mboehme. Herald added a project: clang. Herald added a subscriber: cfe-commits. Adds `CodeGen::emitCXXDestructorCall`, a function that creates a CodeGenFunction using the arguments provided, then invokes CodeGenFunction::EmitC

[PATCH] D82392: [CodeGen] Add public function to emit C++ destructor call.

zoecarver updated this revision to Diff 274616. zoecarver added a comment. - Remove `emitCXXDestructorCall` and add `getCXXDestructorImplicitParam`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82392/new/ https://reviews.llvm.org/D82392 Files:

[PATCH] D82392: [CodeGen] Add public function to emit C++ destructor call.

zoecarver added a comment. @rjmccall I updated this patch to introduce the function `getCXXDestructorImplicitParam` instead. This should be closer to D79942 . It seems like there is never more than one implicit parameter so, I just have it return a single `llvm:

[PATCH] D67052: Add reference type transformation builtins

zoecarver marked 6 inline comments as done. zoecarver added a comment. @EricWF and @miscco thanks for the review comments. Sorry for the delay, I forgot about this patch. All your comments have been addressed/fixed. Comment at: clang/lib/AST/TypePrinter.cpp:1020 switch (T

[PATCH] D67052: Add reference type transformation builtins

zoecarver updated this revision to Diff 258836. zoecarver added a comment. - Format using clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67052/new/ https://reviews.llvm.org/D67052 Files: clang/include/clang/AST/Type.h clang/include

[PATCH] D67052: Add reference type transformation builtins

zoecarver updated this revision to Diff 258834. zoecarver marked 2 inline comments as done. zoecarver added a comment. - Rebase - Fix based on review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67052/new/ https://reviews.llvm.org/D67052

[PATCH] D129384: [objcxx] Fix `std::addressof` for `id`.

zoecarver created this revision. Herald added a project: All. zoecarver requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D129384 Files: clang/lib/Sema/SemaChecking.cpp clan

[PATCH] D129384: [objcxx] Fix `std::addressof` for `id`.

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 rG22c7a6ec: [objcxx] Fix `std::addressof` for `id`. (authored by zoecarver). Repository: rG LLVM Github Monorepo CHA

[PATCH] D106394: [clang][pp] adds '#pragma include_instead'

zoecarver added a comment. Can't wait to start using this! (Note: this is not a full review, just some thoughts.) Do you have a test case where // private: header one *exists* // private: header two #include<__header_one.h> // public: header three #include<__header_two.h> I t

[PATCH] D130446: [apinotes] Upstream changes to `APINotesYAMLCompiler.cpp`.

zoecarver created this revision. zoecarver added a reviewer: compnerd. Herald added a project: All. zoecarver requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D130446 Files:

[PATCH] D130446: [apinotes] Upstream changes to `APINotesYAMLCompiler.cpp`.

zoecarver added a comment. > Can you please add a round trip test as well? There is no testing infrastructure for this upstream. I have tests downstream. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130446/new/ https://reviews.llvm.org/D130446 _

[PATCH] D66822: Hardware cache line size builtins

zoecarver created this revision. Herald added a reviewer: jfb. Herald added subscribers: cfe-commits, kbarton, aheejin, javed.absar, nemanjai. Herald added a project: clang. Creates the `__builtin_hardware_destructive_interference_size` and `__builtin_hardware_constructive_interference_size` buil

[PATCH] D66822: Hardware cache line size builtins

zoecarver added a comment. > Passing-by remark: i'm not sure it is possible to **guarantee** that this > will be always correct and that no ABI break will happen. You're right. I should have said, "least-likely to cause an ABI break." And I completely agree that there is **no way** to gaurentee

[PATCH] D66822: Hardware cache line size builtins

zoecarver added a comment. It may be a good idea only to enable this when `-march=native`. And _maybe_ remove the constexpr requirement (though it would make this feature significantly less useful it would also make it significantly more accurate). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D66822: Hardware cache line size builtins

zoecarver added a comment. We should probably tell people never to use this, period. That being said, I like your idea of having it be a constant. The only issue would be when, in the next few years, people start shipping CPUs with 256-byte-wide cache lines. Repository: rG LLVM Github Monore

[PATCH] D66822: Hardware cache line size builtins

zoecarver added a comment. > BTW, I note that facebook uses 128 bytes for x86 They also use 64 for arm which is interesting (the opposite of this patch). Also, see this comment in the same snippet: > We assume a cache line size of 64, so we use a cache line pair size of 128 Which would indicat

[PATCH] D66839: Fix stack address builtin for negative numbers

zoecarver created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. zoecarver added reviewers: kparzysz, eli.friedman. zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/test/Sema/builtin-stackaddress

[PATCH] D66839: Fix stack address builtin for negative numbers

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/test/Sema/builtin-stackaddress.c:10 +// expected-error@+1 {{argument to '__builtin_return_address' must be a constant integer}} +return __builtin_return_address(x); }

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

zoecarver added a comment. Seems like a very useful function. `__max_representable_int_for_float` also seems useful. Should this work in C++03? If so there are a few changes that need to be made. It would also be great if this could be a `constexpr` (but, obviously, not necessary). =

[PATCH] D66839: Fix stack address builtin for negative numbers

zoecarver added a comment. Thanks for the explanation. Should I then not try to "fix" the issue? Or should I update sema checking? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D66862 Files: clang/lib/AST/ExprConstant.cpp clang/test/SemaCXX/math-builtins.cpp Index: clang/test/SemaCXX/math-builtins.c

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

zoecarver added inline comments. Comment at: include/math.h:1582 + const _RealT __trunc_r = __builtin_trunc(__r); + if (__trunc_r >= ::nextafter(static_cast<_RealT>(_MaxVal), INFINITY)) { +return _Lim::max(); scanon wrote: > EricWF wrote: > > scanon wrote:

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

zoecarver added a comment. > Dead link. Here: https://godbolt.org/z/AjBHYq CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66836/new/ https://reviews.llvm.org/D66836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

zoecarver added a comment. Yes, that sounds right. I can't think of any reason that the condition couldn't be `if (__r > static_cast(numeric_limits::max()))`. The information lost from shifting the value around is never more than the information lost from static_casting the value (as far as I

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver added a comment. (from IRC) Add tests for: - out of range - nan - rounding up _and_ down Also, use the builtin APFloat methods. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66862/new/ https://reviews.llvm.org/D66862

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver updated this revision to Diff 217973. zoecarver added a comment. - remove rint updates (rint uses runtime-defined rounding method) - add more tests (nan, overflow, round down) - use APFloat rounding methods - if rounding fails, bail out to runtime Repository: rG LLVM Github Monorepo

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver marked 3 inline comments as done. zoecarver added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:9612 + case Builtin::BIlround: + case Builtin::BI__builtin_lround: { rsmith wrote: > It's non-conforming to accept calls to these non-`__bu

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver marked 2 inline comments as done. zoecarver added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:9616 +return EvaluateFloat(E->getArg(0), Val, Info) && + Success(lround(Val.convertToDouble()), E); + } lebedev.ri wrote: > zoec

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:9616 +return EvaluateFloat(E->getArg(0), Val, Info) && + Success(lround(Val.convertToDouble()), E); + } craig.topper wrote: > zo

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver updated this revision to Diff 217977. zoecarver added a comment. - fix: formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66862/new/ https://reviews.llvm.org/D66862 Files: clang/lib/AST/ExprConstant.cpp clang/test/SemaCXX/math

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver updated this revision to Diff 217976. zoecarver added a comment. - fix expected error - fix APInt width - fix nan tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66862/new/ https://reviews.llvm.org/D66862 Files: clang/lib/AST/ExprC

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:9617 +APFloat FPVal(0.0); +APSInt IVal(Info.Ctx.getIntWidth(E->getType()), 0); +bool isExact = true; rsmith wrote: > Please use `/*

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver added a comment. > Are you intentionally excluding __builtin_lroundl/__builtin_llroundl? I //was// because `convertToDouble` could only return up to 64 bytes. But now that I am using the builtin APFloat round function, that works. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D66862: Make lround builtin constexpr (and others)

zoecarver updated this revision to Diff 218231. zoecarver added a comment. - add roundl builtins - add more tests - address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66862/new/ https://reviews.llvm.org/D66862 Files: clang/lib

[PATCH] D67052: Add reference type transformation builtins

zoecarver created this revision. zoecarver added reviewers: EricWF, eli.friedman, rsmith, craig.topper. Herald added subscribers: libcxx-commits, cfe-commits, jfb, christof. Herald added projects: clang, libc++. This patch adds builtin type traits to transform reference types. Specifically, it ad

[PATCH] D67052: Add reference type transformation builtins

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp:13 +// variants listed in the RUN script. +// +// Impl Compile Time Object Size

[PATCH] D67052: Add reference type transformation builtins

zoecarver updated this revision to Diff 218276. zoecarver added a comment. - remove accedentally added file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67052/new/ https://reviews.llvm.org/D67052 Files: clang/include/clang/AST/Type.h clang/in

[PATCH] D67052: Add reference type transformation builtins

zoecarver added a comment. In D67052#1656319 , @mclow.lists wrote: > If you're going to do `__add__lvalue_reference`, `__add_rvalue_reference`, > and `__remove_reference`, why not go all the way and add `__is_reference`, > `__is_lvalue_reference` and `__

[PATCH] D67052: Add reference type transformation builtins

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1095 + default: +assert(false && "Not a reference type specifier"); + } Mordante wrote: > Wouldn't it be better to use `llvm_unreachabl

[PATCH] D67052: Add reference type transformation builtins

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_remove_reference.sh.cpp:13 +// variants listed in the RUN script. +// +// Impl Compile Time Object Size

[PATCH] D87974: Summary: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. zoecarver requested review of this revision. Adds `__builtin_zero_non_value_bits` to zero all padding bits of a struct. Currently does not support unions or bitfields. Repository: rG LLVM G

[PATCH] D87974: Summary: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added inline comments. Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits.cpp:160 + +int main() { + testAllForType<32, 16, char>(11, 22, 33, 44); jfb wrote: > Usually CodeGen tests will use lit to check the emitted IR matches > expectations

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver updated this revision to Diff 293606. zoecarver edited the summary of this revision. zoecarver added a comment. - Add more test cases. - Fix typo. - Add codegen tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87974/new/ https://revie

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver updated this revision to Diff 294603. zoecarver marked 3 inline comments as done. zoecarver added a comment. - Add UnsizedTail codegen test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87974/new/ https://reviews.llvm.org/D87974 Files:

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver marked 3 inline comments as done. zoecarver added inline comments. Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp:16 + Bar f; +}; + jfb wrote: > It would be helpful to have a comment with the final layout of the struct, > in

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a comment. In D87974#2438682 , @BillyONeal wrote: >> Are they actually the same, with the same handling of corner cases like >> unions and tail padding? >> There's more to this than just the name, and if they aren't the same, it >> seems

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a comment. > If the implementation-specific builtins don't match on these, then maybe they > should have different names, is his argument I think. That's a fair point. And I agree, if they don't match, maybe it would be best to have different names. I'm hoping that we can all ag

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

zoecarver updated this revision to Diff 38. zoecarver added a comment. - Add lots of tests for S0. - Implement discussed change for handling S0. - Update docs to reflect change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92361/new/ https://r

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

zoecarver added a comment. Sorry for the slow update. This patch now implements the suggested change (to "inherit" the trivial-abi attribute if there are no user-defined special members and its copy/move constructor is deleted solely because of a subobject with the attribute). Let me know what

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

zoecarver added a comment. > I think that as long as the class leaves a copy/move constructor defaulted, > there's no need for a new trivial_abi attribute. Sorry, I'm not sure I follow. Could you elaborate a bit or provide an example? What do you mean by "new" trivial_abi attribute? Repositor

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

zoecarver added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6502 + // except that it has a non-trivial member *with* the trivial_abi attribute. + for (auto Base : D->bases()) { +if (auto CxxRecord = Base.getType()->getAsCXXRecordDecl()) ahat

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

zoecarver added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6502 + // except that it has a non-trivial member *with* the trivial_abi attribute. + for (auto Base : D->bases()) { +if (auto CxxRecord = Base.getType()->getAsCXXRecordDecl()) ahat

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

zoecarver added a comment. In D92361#2459655 , @rjmccall wrote: > In D92361#2459513 , @zoecarver wrote: > >>> I think that as long as the class leaves a copy/move constructor defaulted, >>> there's no need for a ne

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

zoecarver updated this revision to Diff 312629. zoecarver added a comment. - Fix base-derived case. - Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92361/new/ https://reviews.llvm.org/D92361 Files: clang/include/clang/Basic/AttrDocs.td

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

zoecarver added a comment. @ahatanak this patch should now be a nfc for types that don't have anything to do with the trivial-abi attribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92361/new/ https://reviews.llvm.org/D92361

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

zoecarver added a comment. > We could probably do something like what this patch is doing and determine > whether a class can be passed in registers based on whether its subobjects > can be passed in registers. If all of the subobjects can be passed in > registers, the current class can be pass

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/test/CodeGenCXX/builtin-zero-non-value-bits-codegen.cpp:46 +void test(Baz *baz) { + __builtin_zero_non_value_bits(baz); +} zoecarver wrote: > jfb wrote: > > It would b

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver updated this revision to Diff 295445. zoecarver added a comment. - Support constant arrays - Format changes with clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87974/new/ https://reviews.llvm.org/D87974 Files: clang/include

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1652 +auto Element = CGF.Builder.CreateGEP(I8Ptr, Index); +CGF.Builder.CreateAlignedStore(Zero, Element, MaybeAlign()); + }; jfb wrote: > You should use `alignmentAtOffset` here

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1704 +->getArrayElementTypeNoTypeQual() +->isRecordType()) { + auto FieldElement = CGF.Builder.CreateStructGEP(Ptr, Index); Is it OK to possibly create hundr

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a comment. In D87974#2408119 , @jwakely wrote: > As of a few hours ago, GCC has `__builtin_clear_padding`, see > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005fclear_005fpadding > for the docs. Great.

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a comment. > Let's make sure that we follow the same semantics that GCC does, particularly > w.r.t. union, bitfields, and padding at the end of an object (whether it's in > an array or not). Agreed. I'm planning to run some tests tomorrow once the nightly build has updated. R

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a comment. > How should I check for support? Is it going to be e.g. > __has_feature(__builtin_clear_padding)? I'm not sure `__has_feature` will work but `__has_builtin` should work on both Clang and GCC. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a comment. @jwakely It looks like `UnsizedTail` causes a crash. Other than that all the tests in this PR pass. It also looks like there's (at least) some support for unions and bitfields. This patch doesn't support those but I'm planning to add support as a follow-up. Reposito

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a comment. > Filed https://gcc.gnu.org/PR97943 for that. Avoiding the crash is trivial, > deciding what we want to do exactly for flexible array members (which are > beyond the C++ standard) is harder. Yes, and we should try to do the same thing in this case. Currently, this im

[PATCH] D92361: [trivial-abi] Support types without a copy or move constructor.

zoecarver created this revision. Herald added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. zoecarver requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D92361 Files: clang/include/clang/Basic/At

  1   2   >