13

I had no clue why this doesn't work. The following Function is created by placement new. A function is provided that checks whether it should be destructed, and if so, calls its destructor manually.

Here is the testcase where it seems the destructor is never called:

/* Represents a function at runtime */ 
class Function {
public:
  /* Creates an invalid function */
  Function():codeptr(0) { }

  /* Creates a function with the given code pointer */
  Function(void *codeptr):codeptr(codeptr) { }

  /* Frees the function machine code */
  ~Function() {
    if(*this) {
      /* <- I explicitly put a debug output here! */
      destroyLLVMCode(codeptr);
    }
  }

public:
  /* Returns true if the function is valid 
   * (if the code pointer is non-null)
   */
  operator bool() const { return codeptr != 0; }

  /* Destroy this function by calling its destructor */
  void destroy() { ~Function(); }

private:
  void *codeptr;
};

I used this like the following. Cut down the code below to the minimum that still exhibits the problem. In my real program, of course, the memory is allocated in another manner, from an allocator.

#include <new>
#include <cstdlib>

int main() { 
  void *buffer = std::malloc(sizeof(Function));
  Function *f = new (buffer) Function(someExecutableLLVMCode);
  /* more code .. register with symbol tables etc.. */
  f->destroy();
}

You can see I'm calling the destructor in the line reading ~Function(). The compiler accepts, but it doesn't end up calling it: I verified it by checking whether it really deletes the LLVM code I gave it (put some code into the destructor before deleting the LLVM code that the codeptr points to, in case the Function is valid).

I found out later on what is causing that. Could you please provide me with an explanation?

Johannes Schaub - litb
  • 496,577
  • 130
  • 894
  • 1,212
  • This code never creates a Function in any way, and never calls any methods of Function, so its not suprising it also never destroys any Function objects... – Chris Dodd Dec 13 '10 at 05:18
  • @Johannes, how do you create the instance here? Where's that placement `new`? – Nikolai Fetissov Dec 13 '10 at 05:19
  • @Johannes: take a break, a nap, whatever. ;-) your code example doesn't illustrate the problem. you're saying you've found the cause but you're asking for an explanation. this is just muddled. can you delete the question and post tomorrow? – Cheers and hth. - Alf Dec 13 '10 at 05:20
  • Please add the complete code so we can have a look – Arunmu Dec 13 '10 at 05:21
  • I'm just placement newing it and calling the destroy function somewhere in external code. I didn't deem it important. I will include it into the question. Hold on. – Johannes Schaub - litb Dec 13 '10 at 05:22
  • How the function "new" has been used is not clear. and how did you verify that Destructor is not getting called? – Arunmu Dec 13 '10 at 05:34
  • I can't post the entire code because it's closed-source. So I post a testcase. But this testcase exhibits that problem too. It's a tough one, I have to admit. – Johannes Schaub - litb Dec 13 '10 at 05:36
  • 10
    By night Johannes is a helpful, productive member of the Stack Overflow community. By day, he writes code with `bool` conversion functions. – James McNellis Dec 13 '10 at 05:50
  • 1
    @Johannes: Safe Bool Idiom at http://www.artima.com/cppsource/safebool.html --> I think you'll find it an interesting read :) – Matthieu M. Dec 13 '10 at 07:41

3 Answers3

24

This is because ~Function(); in not a destructor call syntactically here. Use this->~Function(); instead.

~Function(); is parsed as an operator ~ and creation of the Function object on the stack. Function class has an operator bool that's why this will be compiled.

Kirill V. Lyadvinsky
  • 97,037
  • 24
  • 136
  • 212
  • 6
    But if it's not, if you have to write `Function::~Function()`, say, then what is `~Function()` parsed as? Applying bitwise negation to a temporary `Function` object? That shouldn't compile, I think. OH UPDATE: he has an implicit conversion to `bool`. J O H A N N E S, don't do that! – Cheers and hth. - Alf Dec 13 '10 at 05:44
  • `Function::~Function()` will not work as well. Standard says you should write `ptr->~Function()`. – Kirill V. Lyadvinsky Dec 13 '10 at 05:47
  • Well, Function::~Function() will work fine. But you are right about `~Function()`. It's described in 5.3.1/9 in the holy C++ Standard. – Johannes Schaub - litb Dec 13 '10 at 05:49
  • @Kirill: I think `Function::~Function()` is just fine, it's a *qualified-id* and the standard says [ Note: an explicit destructor call must always be written using a member access operator (5.2.5) **or a qualified-id** (5.1); in particular, the *unary-expression* `˜X()` in a member function is not an explicit destructor call (5.3.1). — end note ] (section 12.4 `[class.dtor]` of the n3225 draft) – Ben Voigt Dec 13 '10 at 05:49
  • Damn. I should have put a control print statement outside of the `if(*this)`. Then, people would have tested it and said "Dude, the destructor *is* called!". But actually, it would have been the destructor of the temporary haha. – Johannes Schaub - litb Dec 13 '10 at 05:51
  • 1
    The C++ Standard 12.4 says: [Note: an explicit destructor call must always be written using a member access operator (5.2.5); in particular, the unary-expression ̃X() in a member function is not an explicit destructor call (5.3.1). ] – Kirill V. Lyadvinsky Dec 13 '10 at 05:54
  • 2
    @Kirill V. Lyadvinsky : +1 for the nice explanation as it why it gets compiled, and how to call the destructor. – Nawaz Dec 13 '10 at 05:56
  • @Ben Voigt, "or a qualified-id" part is not in the current Standard. – Kirill V. Lyadvinsky Dec 13 '10 at 05:56
  • 2
    Holy cow. Beautiful day! I just learnt something new about C++! – wilhelmtell Dec 13 '10 at 05:56
  • @Kirill: Looks like a standard bug. A qualified-id IS accepted, and the footnote had to be changed to reflect that (in current drafts of C++0x it does), but the footnote was always informational and non-binding. Section 3.4.3 `[basic.lookup.qual]` controls lookup of destructors as qualified-ids. – Ben Voigt Dec 13 '10 at 05:58
  • @Kirill that doesn't matter. That's a note so it has entirely no effect on validity of C++ programs. `Class::~Class()` is a call to a destructor because `Class::~Class` refers to the destructor (5.1/7). – Johannes Schaub - litb Dec 13 '10 at 05:58
  • I think you could write `(~Function)()` and it would call the destructor instead of doing the bitwise negation :) – Johannes Schaub - litb Dec 13 '10 at 06:03
  • And also section 5.2.4 `[expr.pseudo]` may come into play. – Ben Voigt Dec 13 '10 at 06:03
  • @Johannes: The parentheses are definitely allowed in the declaration, (section 12.4 p1), not sure about the call site. I think the grammar would still consider that to be an application of the `~` bitwise-complement operator – Ben Voigt Dec 13 '10 at 06:04
  • @Johannes, Ok, I've just tested on GNU C++ 4.4.4. It still requires a member access operator, `Function::~Function()` without `this->` is not being compiled. – Kirill V. Lyadvinsky Dec 13 '10 at 06:07
  • @Ben a type name is not an expression, so i don't think it would :) – Johannes Schaub - litb Dec 13 '10 at 06:09
  • @Johannes: Sure it is. Try this breakdown: " *unary-expression*: *unary-operator* *cast-expression* ", " *unary-operator*: ~ " and " *cast-expression*: *unary-expression* ", " *unary-expression*: *postfix-expression* ", " *postfix-expression*: *primary-expression* ", " *primary-expression*: *id-expression* ", " *id-expression*: *unqualified-id* ", " *unqualified-id*: *identifier* " ... and a type name is an identifier. – Ben Voigt Dec 13 '10 at 06:16
  • @Ben seems I misunderstood the matter since all compilers agree with you. This one appears to be ambiguous then, though: `string();`. If a type name would be primary-expression, then you have an ambiguity between a function call and a functional cast here. I don't see this ambiguity resolved by the text. – Johannes Schaub - litb Dec 13 '10 at 06:25
  • Therefor, I'm (or was, If you have a satisfactory explanation for the upcoming issues) in the impression it's the other way around: An identifier can be a type name, but a type name is not an id-expression. I derived that from 5.1.1/6, which says "An identifier is an id-expression provided it has been *suitably* declared ..." (emphasize mine). – Johannes Schaub - litb Dec 13 '10 at 06:32
  • Not only will you have a problem there, you will also have a problem with sizeof. Will sizeof (string) be the version that measures size of a (type-id) or size of an unary-expression? What about `string * s;` - is it a multiplication or a declaration? The thing is, by only knowing that `string` is a type, it's enough to know how to parse it. That's the whole point of `typename` in templates, for example. – Johannes Schaub - litb Dec 14 '10 at 14:24
  • @Ben in between I talked to the committee and they said it is intended to reject `(~T)()` too. I hope this matter will be clarified. – Johannes Schaub - litb Dec 18 '10 at 04:44
8

Change your explicit destructor call to

this->~Function();

Currently the ~Function is constructing a "Function" and then calling the ~ bitwise operator, (legal because you have a conversion to bool), and then destructing that, not the called object.

Keith
  • 6,756
  • 19
  • 23
-1

As I recall the destructor cannot be called explicitely. Try moving the cleanup code from destructor to other function and call it instead.

ruslik
  • 14,714
  • 1
  • 39
  • 40
  • You can call destructors explicitly (it's more difficult to discuss explicit constructor calls since there's no syntax for constructor calls at all, it's at the semantic level, but destructor calls don't even have that terminological issue: it's all very clear). – Cheers and hth. - Alf Dec 13 '10 at 05:39
  • we can call a destructor expicitly only like below :: Obj o; o.~Obj(); Not like how it is done in "mayBeDestroy" method. It should have given compilation error. Else it must be called as Function::~Function(); – Arunmu Dec 13 '10 at 05:46