0

In my header file I have:

CPTSDatabase();
virtual ~CPTSDatabase();
void    CloseDatabase();

In my source file I have:

CPTSDatabase::~CPTSDatabase()
{
    CloseDatabase();
}

void CPTSDatabase::CloseDatabase()
{
    if (m_dbDatabase.IsOpen())
        m_dbDatabase.Close();
}

Code analysis is saying:

C26432: If you define or delete any default operation in the type class CPTSDatabase, define or delete them all (c.21).

I have read up on this but can't see the issue. I have a constructor and destructor and I never use the word delete`. What have I missed?

Andrew Truckle
  • 17,769
  • 16
  • 66
  • 164
  • 1
    It's the destructor that (implicitly) has `noexcept` specification: "**[except.spec]/8** The exception specification for an implicitly-declared destructor, or a destructor without a *noexcept-specifier*, is potentially-throwing if and only if any of the destructors for any of its potentially constructed subojects is potentially throwing." – Igor Tandetnik Oct 22 '21 at 14:34
  • 1
    `C26432` appears to complain about a [rule-of-three](https://en.cppreference.com/w/cpp/language/rule_of_three) violation: you have defined a destructor, but not a copy constructor nor copy assignment operator. What happens when an instance of `CPTSDatabase` is copied? – Igor Tandetnik Oct 22 '21 at 14:36
  • @IgorTandetnik I did not realise that. Nor did I realise that `noexcept` could have parameters. I added `noexcept(false)` to the destructor in the header / source and that resolves the first one. What i think I will do is reword this question use for the other issue and we can tidy these commands up. – Andrew Truckle Oct 22 '21 at 17:14
  • 1
    Note that a throwing destructor is a [really bad idea](https://stackoverflow.com/questions/130117/throwing-exceptions-out-of-a-destructor). There's a reason why destructors are `noexcept` by default. – Igor Tandetnik Oct 22 '21 at 17:43
  • @IgorTandetnik Then *where* should I put that code? It has to tidy up .... surely loads of destructors tidy up? #confused. – Andrew Truckle Oct 22 '21 at 18:02
  • 1
    The destructor is the correct place to do cleanup. If you cannot help it and have to call functions from the destructor, that aren't themselves `noexcept`, you'll need to wrap them up in a `try`/`catch`. – IInspectable Oct 22 '21 at 18:07
  • @IgorTandetnik in an attempt to keep this question concise I have removed the noexcept code. Besides, my app would not compile with it like that. We can simplify the comments and limit it to just wants needed. – Andrew Truckle Oct 22 '21 at 18:44
  • @IInspectable As per my other comment, I have taken out the stuff related to the noexcept. I’ll adjust the code to use a try / catch block for now. Leaving just the issue at hand. – Andrew Truckle Oct 22 '21 at 18:45

1 Answers1

2

First, remember that some of those /analyze warnings including C26432 are informational and optional. If you don't want that particular C++ Core Guidelines check enabled, you can disable it. See Microsoft Docs

All it's really telling you is "Hey, have you thought about how you want to handle copy or move for your class?"

The general recommendations are that if you define a dtor, then you should address the copy constructor, move constructor, copy assignment operator, and move assignment operator. For example, it seems logical that your class can support move operations, but not copy operations since it owns a system resource (some sort of database handle), so you should probably make it:

CPTSDatabase();

// Default move ctor and move operator
CPTSDatabase(CPTSDatabase&&) = default;
CPTSDatabase& operator= (CPTSDatabase&&) = default;

// Delete the copy ctor and copy operator
CPTSDatabase(CPTSDatabase const&) = delete;
CPTSDatabase& operator=(CPTSDatabase const&) = delete;

// If using virtual for a class that's not just an interface,
// it's recommended to use a virtual dtor.
virtual ~CPTSDatabase();

That said, if your class itself contains variables that do not support move (like std::mutex or std::atomic), then you should delete those as well.

CPTSDatabase();

CPTSDatabase(CPTSDatabase&&) = delete;
CPTSDatabase& operator= (CPTSDatabase&&) = delete;

CPTSDatabase(CPTSDatabase const&) = delete;
CPTSDatabase& operator=(CPTSDatabase const&) = delete;

virtual ~CPTSDatabase();

Keep in mind that deleting these basic operations makes your class more restricted in how you can use it, but it's better than your class being used in ways that break it.

Chuck Walbourn
  • 38,259
  • 2
  • 58
  • 81
  • Hi !, would you mind updating this answer with code comments to make it clear what each is doing? I think the && ones are Move and the & ones are Copy?. – Andrew Truckle Feb 04 '23 at 17:02