[Lldb-commits] [PATCH] D79251: Fix overloaded operator new cases in TestCppOperators.py which currently work by accident

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik created this revision.
shafik added reviewers: teemperor, aprantl, labath.

The overloaded `new operator` in `TestCppOperators.py` are working by accident. Currently we have:

  void *operator new(std::size_t size) { C* r = ::new C; r->custom_new = true; return r; }
  void *operator new[](std::size_t size) { C* r = static_cast<C*>(std::malloc(size)); r->custom_new = true; return r; }

Which allocate a C using global new which invokes the constructor on the allocated memory, it each one then proceeds to set the member variable `custom_new` to `true` and returns the allocated memory.

This is incorrect code b/c the allocation function is supposed to allocate memory, while the new expression which invokes this has the responsibility of then invoking the constructor (if any) on the newly allocated object e.g.:

  new struct C

So the constructor should be invoked again. IIUC currently because LLDB does not create the implicit constructor when parsing the `DWARF` because it is artificial, this results in construction being a no-op. If we OTOH explicitly default the constructor:

  C() = default;

The test no longer works since the in-class member initializer will be run and sets the field back to false:

  bool custom_new = false;

This behavior is a little surprising since I believe Sema should create the constructor for us when running the expression but either that is not really the case or there is some other piece that is missing.


https://reviews.llvm.org/D79251

Files:
  lldb/test/API/lang/cpp/operators/main.cpp


Index: lldb/test/API/lang/cpp/operators/main.cpp
===================================================================
--- lldb/test/API/lang/cpp/operators/main.cpp
+++ lldb/test/API/lang/cpp/operators/main.cpp
@@ -4,8 +4,8 @@
 
 struct B { int dummy = 2324; };
 struct C {
-  void *operator new(std::size_t size) { C* r = ::new C; r->custom_new = true; return r; }
-  void *operator new[](std::size_t size) { C* r = static_cast<C*>(std::malloc(size)); r->custom_new = true; return r; }
+  void *operator new(std::size_t size) { void *p = ::operator new(size); side_effect = 3; return p; }
+  void *operator new[](std::size_t size) { void *p = ::operator new(size); side_effect = 4; return p; }
   void operator delete(void *p) { std::free(p); side_effect = 1; }
   void operator delete[](void *p) { std::free(p); side_effect = 2; }
 
@@ -171,8 +171,8 @@
   //% self.expect("expr static_cast<long>(c)", endstr=" 12\n")
   //% self.expect("expr c.operatorint()", endstr=" 13\n")
   //% self.expect("expr c.operatornew()", endstr=" 14\n")
-  //% self.expect("expr (new struct C)->custom_new", endstr=" true\n")
-  //% self.expect("expr (new struct C[1])->custom_new", endstr=" true\n")
+  //% self.expect("expr (new struct C); side_effect", endstr=" = 3\n")
+  //% self.expect("expr (new struct C[1]); side_effect", endstr=" = 4\n")
   //% self.expect("expr delete c2; side_effect", endstr=" = 1\n")
   //% self.expect("expr delete[] c3; side_effect", endstr=" = 2\n")
   delete c2;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79251.261493.patch
Type: text/x-patch
Size: 1479 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200502/0a944b31/attachment.bin>