I like using sentry classes in c++, but I seem to have a mental affliction that results in repeatedly writing bugs like the following:
{
MySentryClass(arg);
// ... other code
}
Needless to say, this fails because the sentry dies immediately after creation, rather than at the end of the scope, as intended. Is there some way to prevent MySentryClass from being instantiated as a temporary, so that the above code either fails to compile, or at least aborts with an error message at runtime?
I can't think of an automatic way to detect if you make this mistake or not. You could always create a macro that expands to the correct thing and use that to declare the sentry instead if you keep using it wrong.
#define MY_SENTRY_CLASS(_X) MySentryClass _sentry(_X)
and then use
MY_SENTRY_CLASS(arg);
or put a post-it on your monitor to remind you.
The only thing you could do is make the constructors private and force access through a helper function. This is far less similar than the initial construction syntax and less likely to be mistaken. You could also allocate on the heap (still a waste) but it's much easier to spot. However, if you want your class to be constructible, you can't stop people constructing rvalues of that type.
Edit: IF you know that MySentryClass always takes an argument, you could disallow construction AND and only allow operator=(arguments). This would force you to do
MySentryClass x;
x = arg;
You could do some kind of method chain for it.
MySentryClass x;
x.SetArg1(arg).SetArg2(arg2).construct();
No, there is no exit from this problem. To make objects on the stack, you have to have public constructors, and if you have public constructors, you can make the mistake you are reporting.
Not sure you'll like this solution, but the solution may well be grep:
find /path/to/project -type f -name \*.cpp -print0 | xargs grep -0 'MySentryClass('
Another thing you could do is use sed or perl to preprocess your source file, replacing MySentryClass( with \n#error MySentryClass used incorrectly\n, which hopefully will give you a line number that's close to where the error is. How to do this depends on your build system.
I think the #define is the best method.
But just as an option for not using #define:
int main()
{
try
{
S arg1;
// This will not compile
// MySentry x1 = MySentry::CreateSentry(arg1);
S arg3;
MySentry x2(MySentry::CreateSentry(arg3));
S arg2;
// This will not compile
// MySentry(arg2);
S arg4;
// This will generate a runtime exception
// It will never call start() or end()
//MySentry::CreateSentry(arg4);
}
catch(std::exception const& e)
{
std::cout << "Exception : " << e.what() << "\n";
}
}
#include <stdexcept>
#include <iostream>
class S
{
public:
void start() {std::cout << "Start\n";}
void end() {std::cout << "End\n";}
};
class MySentry
{
struct Init
{
Init(S& s) : arg(s),bad(true) {}
~Init() {if (bad) {throw std::runtime_error("Bad usage of MySentry");}}
S& arg;
mutable bool bad;
};
public:
static Init CreateSentry(S& arg) { return Init(arg);}
explicit MySentry(Init const& arg)
: obj(arg.arg)
, bad(false)
{
arg.bad = false;
std::cout << "Created\n";
obj.start();
}
MySentry(MySentry const& rhs)
: obj(rhs.obj)
, bad(false)
{
std::cout << "Copied (this may not appear)\n";
std::cout << "If the optimizer kicks in then the copy may be elided.\n";
// But if it did not optimize out then
// We have to mark the temporaty as bad
// And not call end() in its destructor.
// Note: Never call start() here as it will always be called in the
// main private constrctor above
rhs.bad = true;
}
~MySentry()
{
if (!bad)
{
// Everything working
obj.end();
}
std::cout << "Destroyed\n";
}
private:
S& obj;
mutable bool bad;
};
What you are trying to do is perfectly legal in C++ and I don't think there is a way to disallow it.
