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

zoecarver added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:2986 purpose of calls. -A class annotated with ``trivial_abi`` can have non-trivial destructors or -copy/move constructors without automatically becoming non-trivial for the -purposes of calls. Fo

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

zoecarver added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:2986 purpose of calls. -A class annotated with ``trivial_abi`` can have non-trivial destructors or -copy/move constructors without automatically becoming non-trivial for the -purposes of calls. Fo

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

zoecarver updated this revision to Diff 308523. zoecarver edited the summary of this revision. zoecarver added a comment. - Fix wording in AttrDocs - Add more tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92361/new/ https://reviews.llvm.org/D

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

zoecarver added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:2986 purpose of calls. -A class annotated with ``trivial_abi`` can have non-trivial destructors or -copy/move constructors without automatically becoming non-trivial for the -purposes of calls. Fo

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

zoecarver updated this revision to Diff 308526. zoecarver added a comment. - Update wording in AttrDocs 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 clang

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

zoecarver added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6416 + if (D->hasAttr()) +return true; + rjmccall wrote: > I don't think we can just unconditionally check for the attribute. The rule > for `trivial_abi` is that it overrides dire

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

zoecarver added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6416 + if (D->hasAttr()) +return true; + rjmccall wrote: > zoecarver wrote: > > rjmccall wrote: > > > I don't think we can just unconditionally check for the attribute. The > > > r

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

zoecarver updated this revision to Diff 308699. zoecarver added a comment. - Add comment in SemaDeclCXX. - Add tests for non-trivial bases/members. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92361/new/ https://reviews.llvm.org/D92361 Files: c

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

zoecarver added a comment. > If it is not possible to copy or move the type at all, it is not possible to > copy or move it trivially. This was a bit confusing to me because I think this changed between C++14 and 17. In C++14, a class was trivially copyable if it had no non-trivial copy/move c

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

zoecarver added a comment. @rjmccall Thanks for all that information. You're right; thinking about it in the context of four value operations is helpful. > Hmm. My first instinct is to say that a type that "adds new information" > about its special members — i.e. that explicitly declares its de

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a subscriber: BillyONeal. zoecarver added a comment. So, it looks like GCC already uses `__builtin_clear_padding` and MSVC already uses `__builtin_zero_non_value_bits`. This patch (obviously) is currently implementing `__builtin_zero_non_value_bits ` but, I had planned to update

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

zoecarver added a comment. > I think perhaps we are talking past each other and have reached the limits of > what this sub-thread can hope to achieve. I agree. I am sure that we (or at least you two) could talk about the semantics of "teleportation" and "destructively moving" objects for many m

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

zoecarver added a comment. @jfb (and others), friendly ping. Are there any other changes (especially to the logic) that you'd like me to make? Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1647 +QualType Ty) { + auto *I8Ptr = CGF.Builder.

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

zoecarver updated this revision to Diff 324874. zoecarver added a comment. - Fix the case where an object inherits from two objects, one with a deleted copy constructor and the other with a deleted move constructor. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

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

zoecarver added a comment. Sorry for the delay in updating. @ahatanak let me know if you've found another way to break this patch :P 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 updated this revision to Diff 324875. zoecarver added a comment. - * Format with clang-format. 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 clan

[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty

zoecarver accepted this revision. zoecarver added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/CodeGen/CGObjCMac.cpp:6683 + values.add(classMethodList); + isEmptyCategory &= + instanceMethodList->isNullValue() && classMethodList->

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

zoecarver created this revision. zoecarver added reviewers: ldionne, rsmith, tmatheson. zoecarver requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Updates __is_unsigned to have the same behavior as the standard specifies. This is in line wit

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

zoecarver added inline comments. Comment at: clang/docs/LanguageExtensions.rst:1199 + types. Note, before Clang 10, returned true for enumeration types if the + underlying type was signed, and returned false for floating-point types. * ``__is_standard_layout`` (C++, GNU, Micro

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

zoecarver added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4837 +// Enum types should always return false (same as UTT_IsSigned). +return !T->isEnumeralType() && T->isUnsignedIntegerType(); Quuxplusone wrote: > FWIW, I'd lose the parent

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

zoecarver updated this revision to Diff 328905. zoecarver added a comment. - Address Arthur's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98104/new/ https://reviews.llvm.org/D98104 Files: clang/docs/LanguageExtensions.rst clang/lib

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

zoecarver added a comment. Sorry for the delay in updating. All review comments have been addressed. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98104/new/ https://reviews.llvm.org/D98104 ___

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

zoecarver updated this revision to Diff 329761. zoecarver added a comment. - Fix review comments: add colon, reflow line, and fix typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98104/new/ https://reviews.llvm.org/D98104 Files: clang/docs/La

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa89ac0dd185d: Update __is_unsigned builtin to match the Standard. (authored by zoecarver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver added a comment. In D76525 I've added a builtin that uses this API. We can use that builtin in libc++. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/ https://reviews.llvm.org/D74918

[PATCH] D76525: Expose cache line size in __builtin_get_cpu_cache_line_size

zoecarver created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. zoecarver added reviewers: jfb, EricWF, efriedma, jyknight, echristo, __simt__. Herald added a subscriber: dexonsmith. This patch creates the __builtin_get_cpu_cache_line_size to wrap the cpu ca

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver marked 2 inline comments as done. zoecarver added a comment. I've made the suggested changes. Is there a consensus that this should be moved to LLVM? I think it's fairly clang-specific (given the use case). We can also always move it to LLVM if the need arises in the future. Reposito

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver updated this revision to Diff 251758. zoecarver added a comment. Fix based on review: - update comments - return 64 for Core2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/ https://reviews.llvm.org/D74918 Files: clang/includ

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/lib/Basic/Targets/X86.cpp:1786 +// i386 +case CK_i386: +// Netburst craig.topper wrote: > zoecarver wrote: > > craig.topper wrote: > > > I found the documen

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver added a comment. @lebedev.ri Where should I put this? Could you maybe point me to another LLVM target API that clang uses that I could base this off? Maybe it should go inside the triple or just be a static function? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver updated this revision to Diff 252079. zoecarver added a comment. - Rebase off master - For i386, return 16 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/ https://reviews.llvm.org/D74918 Files: clang/include/clang/Basic/Target

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver added a comment. @craig.topper that would mean that it couldn't be constexpr though, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/ https://reviews.llvm.org/D74918 ___ cfe-co

[PATCH] D76525: Expose cache line size in __builtin_get_cpu_cache_line_size

zoecarver added a comment. @craig.topper thanks for the comments! I'm going to hold off on updating this until we figure out what's happening with D74918 , though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7652

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver added a comment. I don't know enough about clang and llvm to know what is both possible and preferable. @craig.topper @lebedev.ri what is the best course of action here? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/ https://re

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver added a comment. @lebedev.ri my misunderstanding. I'll commit :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/ https://reviews.llvm.org/D74918 ___ cfe-commits mailing list cfe-comm

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

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 rGb915aec6b591: Add method to TargetInfo to get CPU cache line size (authored by zoecarver). Repository: rG LLVM Github M

[PATCH] D76525: Expose cache line size in __builtin_get_cpu_cache_line_size

zoecarver added a comment. @jyknight yes, the current plan is to use it in libc++'s implementation. I can put that implementation (which uses this builtin) up for review before this lands. That way we can discuss the implementation before adding a (possibly unneeded) builtin. Sound good? Repo

[PATCH] D76525: Expose cache line size in __builtin_get_cpu_cache_line_size

zoecarver updated this revision to Diff 252742. zoecarver added a comment. - Diff from master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76525/new/ https://reviews.llvm.org/D76525 Files: clang/include/clang/Basic/Builtins.def clang/lib/AST/

[PATCH] D76525: Expose cache line size in __builtin_get_cpu_cache_line_size

zoecarver updated this revision to Diff 252741. zoecarver added a comment. - Fix based on review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76525/new/ https://reviews.llvm.org/D76525 Files: clang/include/clang/Basic/Builtins.def cl

[PATCH] D76525: Expose cache line size in __builtin_get_cpu_cache_line_size

zoecarver added a comment. @craig.topper I'm not sure what should happen. It should probably just use that CPU (although that's not a great solution). Is there a way to detect if an attribute was used to change the target in which case we could error? What do you think should happen? Reposito

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver created this revision. Herald added subscribers: cfe-commits, kristof.beyls. Herald added a project: clang. This patch adds a virtual method `getCPUCacheLineSize()` to `TargetInfo`. Currently, I've only (partially) implemented the method in `X86TargetInfo`. It's extremely important tha

[PATCH] D66822: Hardware cache line size builtins

zoecarver added a comment. In D66822#1656476 , @jfb wrote: > 1. Add to target infrastructure Refs D74918 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66822/new/ https:/

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver added a comment. @lebedev.ri thanks! I'll add those. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/ https://reviews.llvm.org/D74918 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver updated this revision to Diff 245708. zoecarver added a comment. - Add AMD cache line sizes based on @lebedev.ri's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/ https://reviews.llvm.org/D74918 Files: clang/include/c

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver marked 7 inline comments as done. zoecarver added a comment. To address the question, what will this be used for: this could be used for several things in the library and compiler, but the primary use will be to create a builtin that can be used to implement P0154

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/lib/Basic/Targets/X86.cpp:1834 +case CK_Tigerlake: +case CK_Lakemont: + craig.topper wrote: > zoecarver wrote: > > craig.topper wrote: > > > I think Lakemont is

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver updated this revision to Diff 245897. zoecarver added a comment. - Update getCPUCacheLineSize to return optional Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/ https://reviews.llvm.org/D74918 Files: clang/include/clang/Basic/

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver updated this revision to Diff 245896. zoecarver marked 6 inline comments as done. zoecarver added a comment. - Update values returned by getCPUCacheLineSize based on review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/

[PATCH] D66178: Remove std::shared_ptr::make_shared and std::shared_ptr::allocate_shared

zoecarver updated this revision to Diff 245908. zoecarver added a comment. - Remove std::shared_ptr::allocate_shared Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66178/new/ https://reviews.llvm.org/D66178 Files: libcxx/include/memory Index: l

[PATCH] D66178: Remove std::shared_ptr::make_shared and std::shared_ptr::allocate_shared

zoecarver updated this revision to Diff 245907. zoecarver edited the summary of this revision. zoecarver added a comment. Herald added projects: clang, libc++. Herald added a subscriber: cfe-commits. - Remove std::shared_ptr::allocate_shared Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D66178: Remove std::shared_ptr::make_shared and std::shared_ptr::allocate_shared

zoecarver added a comment. @ldionne friendly ping. This patch now //only// removes `std::shared_ptr::allocate_shared`. Should be an easy change. And I think we fixed @phosek's issue by breaking `__hold2.get()->get()` into a variable so that compilers that parse args backwards still work here.

[PATCH] D66178: Remove std::shared_ptr::make_shared and std::shared_ptr::allocate_shared

zoecarver updated this revision to Diff 245918. zoecarver added a comment. - Use __create_with_control_block in allocate_shared Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66178/new/ https://reviews.llvm.org/D66178 Files: libcxx/include/memory

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/lib/Basic/Targets/X86.cpp:1834 +case CK_Tigerlake: +case CK_Lakemont: + craig.topper wrote: > zoecarver wrote: > > craig.topper wrote: > > > zoecarver wrote: >

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

zoecarver updated this revision to Diff 246277. zoecarver added a comment. - move verification to SemaChecking Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 Files: clang/lib/Sema/SemaChecking.cpp clan

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

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

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

zoecarver reopened this revision. zoecarver added a comment. This revision is now accepted and ready to land. Reverted: 698078257285a044110620d7dab2fb4451a3fa29 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

zoecarver updated this revision to Diff 246486. zoecarver added a comment. - Error if SemaBuiltinConstantArgRange returns true, not false Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 Files: clang/test/

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

zoecarver added a comment. I was using `SemaBuiltinConstantArgRange` incorrectly. I was returning an error for `!SemaBuiltinConstantArgRange` instead of `SemaBuiltinConstantArgRange`. Also, I only modified the fail tests so I wasn't able to catch the error. Updated and am running both the Sema

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

zoecarver updated this revision to Diff 246488. zoecarver added a comment. - diff from master (arg phab...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 Files: clang/lib/Sema/SemaChecking.cpp clang/t

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG6201f6601dec: Check args passed to __builtin_frame_address and __builtin_return_address. (authored by zoecarver). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D66178: Remove std::shared_ptr::make_shared and std::shared_ptr::allocate_shared

This revision was automatically updated to reflect the committed changes. Closed by commit rG28d38a25e963: Remove std::shared_ptr::allocate_shared (authored by zoecarver). Changed prior to commit: https://reviews.llvm.org/D66178?vs=245918&id=246599#toc Repository: rG LLVM Github Monorepo CH

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver added a comment. Friendly ping. Any other comments on this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74918/new/ https://reviews.llvm.org/D74918 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver added a comment. Here's what I think we should do: continue to have this method just return the cache line size. Then have another method that returns `true` or `false` for whether the given architecture supports aligned pairs of cache lines then, users of this (either in clang or lib

[PATCH] D69981: [www] More HTTPS and outdated link fixes.

zoecarver added a comment. This LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69981/new/ https://reviews.llvm.org/D69981 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D67052: Add reference type transformation builtins

zoecarver added a comment. @EricWF ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67052/new/ https://reviews.llvm.org/D67052 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

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

zoecarver added a comment. @Jim Thanks, I'll move it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66839/new/ https://reviews.llvm.org/D66839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver added a comment. Herald added a subscriber: danielkiss. There are a lot of different ways we could implement the feature. We may want to only enable it when `-march=native`, or maybe only in the unstable ABI, and maybe we want to support aligned pairs on some architectures. I think tha

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver added a comment. @kristof.beyls I'll try to bring that up in my libc++ patch, thanks. I'm planning on committing this today unless anyone still has concerns. @jyknight is this path OK with you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D74918: Add method to TargetInfo to get CPU cache line size

zoecarver marked 3 inline comments as done. zoecarver added a comment. @lebedev.ri LLVM may be better, I'm not sure. If you feel strongly I can move it. @jyknight I'm planning on adding a builtin that uses this method. I can put the patch up for that first if you would like.

[PATCH] D67052: Add reference type transformation builtins

zoecarver updated this revision to Diff 243834. zoecarver added a comment. - Combine D67052 and D67588 . - Remove `__add*` type traits. - Update mangling/dumping/encoding of all traits. - Remove stress tests. - Address comments in

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

zoecarver abandoned this revision. zoecarver added a comment. Closing in favor of D67052 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67588/new/ https://reviews.llvm.org/D67588 ___ cfe-commits mailing lis

[PATCH] D67052: Add reference type transformation builtins

zoecarver marked 3 inline comments as done. zoecarver added inline comments. Comment at: clang/lib/AST/TypePrinter.cpp:1028 +case UnaryTransformType::RemoveReferenceType: + OS << "__remove_reference"; + print(T->getBaseType(), OS, StringRef()); Thes

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

zoecarver added a comment. This patch looks awesome, Chris. Does it make sense to have builtins for `add_const`, etc.? Isn't `T const` already kind of a builtin for this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116203/new/ https://reviews.l

[PATCH] D120160: [Clang] Add `-funstable` flag to enable unstable and experimental features

zoecarver accepted this revision. zoecarver added a comment. This revision is now accepted and ready to land. This looks great, thanks Egor! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120160/new/ https://reviews.llvm.org/D120160 ___

[PATCH] D101598: [clang][Sema] adds `[[clang::no_address]]` attribute

zoecarver added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5972 + Although opt-in, the attribute poisons the entire overload set, meaning that only one overload + needs the attribute per set (although such usage is discouraged). Until there is confiden

[PATCH] D98904: Instantiate static constexpr data members on MS ABI.

zoecarver created this revision. zoecarver added a reviewer: rsmith. zoecarver requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Because MSVC will not treat the definition of a static data member as part of the declaration, it will not get i

[PATCH] D98904: Instantiate static constexpr data members on MS ABI.

zoecarver added a comment. So, as I mentioned in the description of this patch, the reason that these members are treated as definitions is that Clang implicitly marks them as inline when targeting MS. This is sort of interesting a) because it only applies to constexpr vars, and b) because this

[PATCH] D98904: Instantiate static constexpr data members on MS ABI.

zoecarver added a comment. > (I think you're missing something here to trigger the instantiation of the > class, such as Invalid x;.) Yes, you're right. > The above example is valid in C++17 onwards, because in C++17 onwards, static > constexpr data members are implicitly inline, and the delay

[PATCH] D98904: Instantiate static constexpr data members on MS ABI.

zoecarver added a comment. > So. I think the status quo is OK but not great; we accept invalid code, in > the name of MSVC compatibility, that MSVC doesn't accept. I don't think > following MSVC would be a good thing, as that'd lead to our rejecting valid > code (that MSVC also rejects but that

<    1   2