[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

MaskRay added inline comments. Comment at: clang/include/clang/Basic/SizedDeallocation.h:23 +namespace clang { +inline llvm::VersionTuple sizedDeallocMinVersion(llvm::Triple::OSType OS) { + switch (OS) { wangpc wrote: > MaskRay wrote: > > Does this need to be

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

aaron.ballman reopened this revision. aaron.ballman added a comment. >> In D112921#4624674 , >> @aaron.ballman wrote: >> Because the issues have been ongoing for a few hours now, I think it'd make >> sense to revert these changes while trying to

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. In D112921#4624674 , @aaron.ballman wrote: > Because the issues have been ongoing for a few hours now, I think it'd make > sense to revert these changes while trying to determine what the appropriate > fix is. @wangpc would you

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

aaron.ballman added a comment. Because the issues have been ongoing for a few hours now, I think it'd make sense to revert these changes while trying to determine what the appropriate fix is. @wangpc would you mind doing the revert? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

TWeaver added a comment. Good afternoon from the UK! It looks as though this change has caused the following buildbot to start failing: https://lab.llvm.org/buildbot/#/builders/216/builds/26407 are you able to take a look? Thanks in advance, Tom W Repository: rG LLVM Github Monorepo

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

uabelho added a comment. I've no idea what's happening but it seems like linking compiler-rt/lib/memprof/tests/MemProfUnitTests fails with this commit: [17/17] Linking CXX executable compiler-rt/lib/memprof/tests/MemProfUnitTests FAILED: compiler-rt/lib/memprof/tests/MemProfUnitTests :

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

jhuber6 added a subscriber: sivachandra. jhuber6 added a comment. In D112921#4624580 , @wangpc wrote: > It seems that the linker can't find sized deallocation (no support in the > environment or AMDGPU libraries?). We should have some implementations

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. In D112921#4624141 , @nikic wrote: > FYI this resulted in some pretty wild code size swings, in particular between > -10% and -15% for tramp3d-v4 >

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. In D112921#4624547 , @jhuber6 wrote: > This caused some linking errors with the GPU libc test suite, see > https://lab.llvm.org/staging/#/builders/247/builds/5659. > > clang++: error: ld.lld command failed with exit code 1 (use

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. In D112921#4624488 , @thakis wrote: > Looks like this breaks tests on windows: > http://45.33.8.238/win/83485/step_7.txt > > Please take a look and revert for now if it takes a while to fix. > > (Also, if the patch doesn't

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

jhuber6 added a comment. This caused some linking errors with the GPU libc test suite, see https://lab.llvm.org/staging/#/builders/247/builds/5659. clang++: error: ld.lld command failed with exit code 1 (use -v to see invocation) [331/473] Linking CXX executable

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

thakis added a comment. Looks like this breaks tests on windows: http://45.33.8.238/win/83485/step_7.txt Please take a look and revert for now if it takes a while to fix. (Also, if the patch doesn't already do it, it probably shouldn't change defaults in clang-cl mode?) Repository: rG LLVM

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

nikic added a comment. FYI this resulted in some pretty wild code size swings, in particular between -10% and -15% for tramp3d-v4 (http://llvm-compile-time-tracker.com/compare.php?from=6cde64a94986165547ae5237ac7dd4bddfc9f2a7=2916b125f686115deab2ba573dcaff3847566ab9=size-text). Not sure

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2916b125f686: [clang] Enable sized deallocation by default in

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. Thanks all! I will land this patch later. If there are some failures (especially `libcxx` part @Mordante :-) ), please help me to fix them. Thanks in advance! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 554197. wangpc marked 2 inline comments as done. wangpc added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc marked 2 inline comments as done. wangpc added inline comments. Comment at: clang/include/clang/Basic/SizedDeallocation.h:23 +namespace clang { +inline llvm::VersionTuple sizedDeallocMinVersion(llvm::Triple::OSType OS) { + switch (OS) { MaskRay wrote: >

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/include/clang/Basic/SizedDeallocation.h:23 +namespace clang { +inline llvm::VersionTuple sizedDeallocMinVersion(llvm::Triple::OSType OS) { + switch (OS) { Does this need to be in

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

ldionne accepted this revision. ldionne added a comment. Sorry for missing a couple of direct pings. I have no concerns with the patch, perhaps I should not have marked my review as blocking in the first place. Thanks for doing this work, it's about time we updated this default! Repository:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

aaron.ballman added a comment. In D112921#4621039 , @wangpc wrote: > In D112921#4621019 , @aaron.ballman > wrote: > >> In D112921#4620399 , @wangpc wrote: >> >>> Gentle

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. In D112921#4621019 , @aaron.ballman wrote: > In D112921#4620399 , @wangpc wrote: > >> Gentle ping. Can I move forward and land this? > > What name and email address would you like us to

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

aaron.ballman added a comment. In D112921#4620399 , @wangpc wrote: > Gentle ping. Can I move forward and land this? What name and email address would you like us to use for patch attribution? @ldionne do you have any remaining concerns (I think all the

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. Gentle ping. Can I move forward and land this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 ___ cfe-commits mailing list

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. @ldionne Sorry for bothering, what do you think about landing this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 ___

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 551890. wangpc added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files: clang-tools-extra/clangd/unittests/FindTargetTests.cpp

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 551702. wangpc added a comment. - Rebase. - Remove abilist changes. - Add clang-18. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 551367. wangpc added a comment. Rebase and fix failed tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. In D112921#4595918 , @Mordante wrote: > In D112921#4594092 , @wangpc wrote: > >> - Rebase. >> - Update tests, remove clang-17. > > The removal of the markers in the libc++ tests breaks

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante added a comment. In D112921#4594092 , @wangpc wrote: > - Rebase. > - Update tests, remove clang-17. The removal of the markers in the libc++ tests breaks these test. FYI the tests are using a Clang build of apt.llvm.org and are not using this

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 ___ cfe-commits mailing list

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 550983. wangpc marked 10 inline comments as done. wangpc added a comment. - Rebase. - Update tests, remove clang-17. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante accepted this revision. Mordante added a comment. I'm happy with the libc++ changes, please wait a few days for @ldionne to react. In D112921#4584704 , @aaron.ballman wrote: > In D112921#4581641 , @rnk

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

aaron.ballman added a comment. In D112921#4581641 , @rnk wrote: > What are the next steps here? Have concerns been addressed? The CI failures > seem unrelated (libcxx tests is an AIX bot out of disk, clang CI is an > interpreter test that seems

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

rnk added a comment. What are the next steps here? Have concerns been addressed? The CI failures seem unrelated (libcxx tests is an AIX bot out of disk, clang CI is an interpreter test that seems unrelated, but please confirm). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 546760. wangpc added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files: clang-tools-extra/clangd/unittests/FindTargetTests.cpp

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante added a comment. In D112921#4541063 , @wangpc wrote: > - Rebase. > - Unsupport clang-18. I see the libc++ build failed. It seems like timeouts uploading artifacts, so not related to your patch. Can you rebase and give it another run on the CI?

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

aaron.ballman added a comment. In D112921#4535716 , @wangpc wrote: > I am not familiar with libcxx, can you please help me to fix these tests? I > hope we can catch up with the release of LLVM 17. I think it's too late to put this into Clang 17. This

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 545006. wangpc added a comment. - Rebase. - Unsupport clang-18. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante added a comment. I see one libc++ failure, I have not looked into the other failures. Comment at: libcxx/test/libcxx/language.support/support.dynamic/libcpp_deallocate.sh.cpp:29 +// TODO(mordante) fix this test after updating clang in Docker +// UNSUPPORTED: clang-17

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. In D112921#4536088 , @Mordante wrote: > In D112921#4535716 , @wangpc wrote: > >> In D112921#4532378 , @Mordante >> wrote: >> >>> In

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 544640. wangpc added a comment. - Remove usages of `-fsized-deallocation` in some tests. - Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante added a comment. In D112921#4535716 , @wangpc wrote: > In D112921#4532378 , @Mordante > wrote: > >> In D112921#4530916 , @wangpc wrote: >> >>> In

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. In D112921#4532378 , @Mordante wrote: > In D112921#4530916 , @wangpc wrote: > >> In D112921#4529182 , @Mordante >> wrote: >> >>> I noticed some

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante added a comment. In D112921#4530916 , @wangpc wrote: > In D112921#4529182 , @Mordante > wrote: > >> I noticed some of the CI jobs are still failing with the patch, I didn't >> look into them. > > I

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. In D112921#4529182 , @Mordante wrote: > I noticed some of the CI jobs are still failing with the patch, I didn't look > into them. I don't think they are related to this patch, so I rebased again. If still failed, I will try to

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 543828. wangpc added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files: clang-tools-extra/clangd/unittests/FindTargetTests.cpp

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante added a comment. I noticed some of the CI jobs are still failing with the patch, I didn't look into them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 541415. wangpc added a comment. Rebase and fix libcxx tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 540915. wangpc added a comment. - Add comments. - clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

wangpc updated this revision to Diff 540913. wangpc added a comment. - Rebase - Fix tests. @ldionne I don't know the details about DriverKit so I assumed that sized deallocation is available just like aligned allocation. @Mordante I will leave libcxx tests to folks who are more familiar with

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante added inline comments. Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp:16 +// Sized deallocation was added in macOS 10.12 and aligned OSes. +// XFAIL: use_system_cxx_lib &&

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

MaskRay added a comment. Changing the default improves GCC compatibility (`Enabled by default under -std=c++14 and above.`) and I think is the right direction. @wangpc You can use your new username to commandeer this revision? To other reviewers, what's still blocked now that the Apple target

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead added a comment. In D112921#4003939 , @Romain-Geissler-1A wrote: > Hi, > > Is there any update about this ? Currently, no. If someone is interesting in this, please feel free to commandeer. :-) Repository: rG LLVM Github Monorepo

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Romain-Geissler-1A added a comment. Herald added a subscriber: steakhal. Hi, Is there any update about this ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead updated this revision to Diff 425443. pcwang-thead added a comment. Herald added subscribers: dexonsmith, arichardson. - Add handling of Apple targets. - Update libc++ tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead added a comment. In D112921#3474964 , @rjmccall wrote: > Hmm. Allowing a version on `-stdlib` is intuitively appealing, but I'm not > sure it actually gives us the information we need. As I recall, `-stdlib` > selects the high-level

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

ldionne added a comment. In D112921#3473632 , @pcwang-thead wrote: > In D112921#3473022 , @ldionne wrote: > >> (BTW I strongly support this patch, I just think we should do it properly on >> all platforms from

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

rjmccall added a comment. Hmm. Allowing a version on `-stdlib` is intuitively appealing, but I'm not sure it actually gives us the information we need. As I recall, `-stdlib` selects the high-level stdlib and not the low-level one, and those are related in code but not necessarily at

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead added a comment. In D112921#3473022 , @ldionne wrote: > (BTW I strongly support this patch, I just think we should do it properly on > all platforms from the start :-) I couldn't agree with you more, but I have no idea how to implement it.

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

rjmccall added a comment. Ideally, I think, we would set this up to work something like `ObjCRuntime`, where we're making queries to a common place that contains all the information necessary to decide what runtime features are available. In particular, we shouldn't treat Apple platforms as

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

ldionne added a comment. (BTW I strongly support this patch, I just think we should do it properly on all platforms from the start :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

ldionne added a subscriber: ahatanak. ldionne added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4078-4082 + if (!Args.getLastArg(options::OPT_fsized_deallocation, + options::OPT_fno_sized_deallocation)) +

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead added inline comments. Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp:17 +// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11}} #include Mordante wrote:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante added a comment. In D112921#3463887 , @pcwang-thead wrote: > In D112921#3462592 , @Mordante > wrote: > >> I didn't look at the code, but I have some hints how we can test libc++. > > Thanks! I ran

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead added a comment. In D112921#3462592 , @Mordante wrote: > I didn't look at the code, but I have some hints how we can test libc++. Thanks! I ran tests with no error occurred on my local machine and I really want to know how to test it!

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante added a comment. I didn't look at the code, but I have some hints how we can test libc++. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 ___

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead added a comment. In D112921#3453114 , @MaskRay wrote: > Beside the concern raised by platform maintainers: the cc1 default switch > part should be made separately from the patch. > This makes revert easy and leaves fewer churn to the test

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead updated this revision to Diff 423555. pcwang-thead added a comment. - Disable sized deallocation for Apple targets. - Update tests and don't use `-fno-sized-deallocation` any more. - With one exception: `clang/test/SemaCXX/builtin-operator-new-delete.cpp`, which will generate two

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

MaskRay added a comment. Beside the concern raised by platform maintainers: the cc1 default switch part should be made separately from the patch. This makes revert easy and leaves fewer churn to the test suite. If -fno-sized-deallocation is a better cc1 default (but not Driver's), I can make

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

ldionne requested changes to this revision. ldionne added a comment. Regarding the back-deployment target for Apple, I believe the first OSes where `libc++.dylib` had sized deallocation are `macOS 10.12`, `iOS 10.0`, `watchOS 3.0`, and `tvOS 10.0`. We should be able to enable reliance on sized

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

rjmccall added a comment. In D112921#3127767 , @rnk wrote: > Let's not bring back the weak function thunk approach. That approach caused > problems described in my commit, D8467 , > which is why the code was removed.

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

rnk added a comment. Let's not bring back the weak function thunk approach. That approach caused problems described in my commit, D8467 , which is why the code was removed. The next steps are to sort out right defaults for Apple and understand the libc++ test

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

rjmccall requested changes to this revision. rjmccall added a comment. This revision now requires changes to proceed. Yeah, I think this either needs deployment restriction on Apple platforms or it needs the thunk / weak-linking approach from the original patch when the target OS isn't recent

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

Mordante added a comment. In D112921#3126492 , @zixuan-wu wrote: > Is this going to be reviewed again or committed? This patch still requires approval by the libc++ group. The last build failed for multiple libc++ CI jobs, these should be fixed. (I

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

kwk added a comment. You've disabled sized deallocation in some tests by providing `-fno-sized-deallocation `. I admit I haven't looked at those tests in detail but I would like to understand if those tests would otherwise fail. If they fail, is there work that needs to be done later to make

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

zixuan-wu added a comment. Is this going to be reviewed again or committed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 ___ cfe-commits mailing list

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead updated this revision to Diff 384344. pcwang-thead marked an inline comment as done. pcwang-thead added a comment. Adds `ADDITIONAL_COMPILE_FLAGS` and guard macros to: - `libcxx\test\std\language.support\support.dynamic\new.delete\new.delete.single\sized_delete14.pass.cpp` -

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

rnk accepted this revision. rnk added a comment. Looks good from my end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 ___ cfe-commits mailing list

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead updated this revision to Diff 384089. pcwang-thead added a comment. Makes required changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921 Files:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

ldionne requested changes to this revision. ldionne added inline comments. This revision now requires changes to proceed. Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp:12 // UNSUPPORTED:

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead updated this revision to Diff 383981. pcwang-thead marked an inline comment as done. pcwang-thead added a comment. - Regenerate `clang\test\AST\ast-dump-stmt-json.cpp` and `clang\test\AST\ast-dump-expr-json.cpp`. - Format code. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead updated this revision to Diff 383970. pcwang-thead added a comment. Herald added a project: libc++. Herald added a subscriber: libcxx-commits. Herald added a reviewer: libc++. Removes unnecessary -fno-sized-deallocation and some comments. Repository: rG LLVM Github Monorepo

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead updated this revision to Diff 383953. pcwang-thead added a comment. - Changes to `Args.AddLastArg(CmdArgs, ...)` - Adds a note to `clang/docs/ReleaseNotes.rst` - Fixs clangd test failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

rnk added a comment. Please add a note to clang/docs/ReleaseNotes.rst about the behavior change. The clangd test failure seems related to this change, and the other ones could be as well. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6405 + // by default. + if (Arg *A

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead updated this revision to Diff 383789. pcwang-thead added a comment. Herald added subscribers: usaxena95, kadircet, arphaman. Add `-fno-sized-allocation` to: - `clang\test\AST\ast-dump-expr-json.cpp` - `clang\test\AST\ast-dump-stmt-json.cpp` - `clang\test\CodeGenCXX\dllimport.cpp` -

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

aaron.ballman added reviewers: rsmith, rjmccall, erichkeane, aaron.ballman. aaron.ballman added a comment. Adding some reviewers for visibility. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112921/new/ https://reviews.llvm.org/D112921

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead updated this revision to Diff 383781. pcwang-thead added a comment. Herald added a project: clang-tools-extra. Fix errors in clang-tools-extra\test\clang-tidy\checkers\misc-new-delete-overloads.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

pcwang-thead created this revision. Herald added subscribers: lxfind, dang. pcwang-thead requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Since C++14 has been released for about seven years and most standard libraries have implemented sized