views:

251

answers:

4

Suppose I have an RAII-style C++ class:

class StateSaver
{
  public:
    StateSaver(int i) { saveState(); }
    ~StateSaver() { restoreState(); }
};

...to be used like so in my code:

void Manipulate()
{
  StateSaver save(1);

  // ...do stuff that modifies state
}

...the goal being to enter some state, do stuff, then leave that state when I leave that scope. Is there a way to make this typo not compile (or warn, or somehow complain so that the mistake can be noticed)?

void Manipulate()
{
  StateSaver(1); // ruh-roh, state saved and immediately restored!

  // ...do stuff that modifies state
}

I'm not aware of anything in C++ itself which I could use to prevent this, but that doesn't mean it doesn't exist. If there isn't anything in C++, compiler-specific extensions would be acceptable. I'm primarily interested in anything targeting gcc and msvc (someday icc, ideas for other compilers welcome but less likely to be useful) so hacks for any of them would be useful (abstracted into appropriately #ifdef'd macro definitions, of course).

+5  A: 

SaveMatrix save(); doesn't define an object either. It declares a function.

There's very little you can do to prevent others (or yourself, FTM) from doing something else than they wanted to. The only thing I can think of is not writing the code itself, but writing a macro instead.

#define SAVE_MATRIX SaveMatrix save ## __LINE__

However, this is quite ugly. OTOH, it does catch the error at compile-time.

sbi
+7  A: 

I'm not sure if anything can be done at compile-time. For a run-time check, you could do this:

struct SaveMatrix
{
    SaveMatrix(const SaveMatrix& that) {
        assert(this == &that);
        glPushMatrix();
    }
    ~SaveMatrix() { glPopMatrix(); }
};

Which requires the client to write:

SaveMatrix sm(sm);

and there's no way to do the same for a temporary without binding it to an identifier (at which point it's no different from an auto variable).

Pavel Minaev
Nice one, looks a bit weird in the beginning, but could become part of the RAII-idiom, because the question does show a real issue with C++/RAII. I'd rather have a little bit of cruft over finding out that my lock didn't work because of this.
stefaanv
Hm, of the options I think I like this the best. It's not quite natural, but then again, RAII itself isn't entirely natural either, at least not if you think of classes primarily as structs-with-initialization-and-cleanup. Thanks!
Jeff Walden
+1  A: 

The class can never tell if it was instantiated as a temporary (SaveMatrix()) or as a variable (SaveMatrix save;). I think the best way to stop the programmer doing that without stack or macro hacks is to force a member function call after construction, eg:

class RAII
{
public:
    bool valid;
    RAII()
     : valid(false)
    {
     cout << "RAII ctor" << endl;
    }

    void Do()
    {
     valid = true;
    }

    ~RAII()
    {
     assert(valid);
     cout << "RAII dtor" << endl;
    }
};

This then works as follows:

{
 // Intended use
 RAII raii;
 raii.Do();

 cout << "Some task" << endl;
}

{
 // Woops: forgot Do()
 RAII raii;

 cout << "Some task" << endl;
}

{
 // Woops: forgot Do()
 RAII();

 cout << "Some task" << endl;
}

{
 // Programmer shot self in foot, hopefully the act of typing this would make them realise that
 RAII().Do();

 cout << "Some task" << endl;
}
AshleysBrain
That `RAII().Do()` fails doesn't seem a problem here. AFAIU, the Op tries to shut out that Murphy guy, not Machiavelli.
sbi
+2  A: 

I actually had to tweak my solution in a bunch of ways from the variant Waldo posted, but what I eventually got to is a macro-ized version of:

class GuardNotifier
{
    bool* notified;
  public:
    GuardNotifier() : notified(NULL) { }
    void init(bool* ptr) { notified = ptr; }
    ~GuardNotifier() { *notified = true; }
};
class GuardNotifyReceiver
{
    bool notified;
  public:
    GuardNotifyReceiver() : notified(false) { }
    void init(const GuardNotifier& notifier)
      { const_cast<GuardNotifier&>(notifier).init(&notified); }
    ~GuardNotifyReceiver() { assert(notified); }
};
class StateSaver
{
    GuardNotifyReceiver receiver;
  public:
    StateSaver(int i,
               const GuardNotifier& notifier = GuardNotifier())
    {
      receiver.init(notifier)
      saveState();
    }
    ~StateSaver()
    {
      restoreState();
    }
};
David Baron