[libcxx-commits] [PATCH] D105962: [libcxx] [test] Fix mismatches between aligned operator new and std::free

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/delete_align_val_t_replace.pass.cpp:61
 
-void operator delete(void* p, const std::nothrow_t&) TEST_NOEXCEPT
+void operator delete [] (void* p, const std::nothrow_t&) TEST_NOEXCEPT
 {
----------------
mstorsjo wrote:
> Quuxplusone wrote:
> > Your commit summary mentions that the old test relied on
> > 
> > > the fallback from `operator delete[]` to user defined `operator delete`
> > 
> > Are you sure that this was intentional, and not just a typo in the test? (Personally I didn't know that any such fallback rule existed! And wouldn't that be more of a compiler issue than a libc++ issue, anyway?)
> > I'd feel better if we did the `delete->delete[]` renaming in one commit/PR and then whatever Windows fixup is still needed, in a separate PR.
> I presume it was just a typo in the test, yes. (There's such a fallback, but we shouldn't be testing that implicitly everywhere but only in a test set out to specifically test that, yeah, as not all systems might support it.)
> 
> @ldionne WDYT, keep the `delete` -> `delete[]` change here or split to a separate patch?
I'm fine with it being done here, since you called it out in the commit message. Pedantically, a separate patch would be better, but I'm trying to balance pedantic correctness and productivity :-).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105962/new/

https://reviews.llvm.org/D105962