How to avoid C++ anonymous objects

Question

I have a ScopedLock class which can help to release lock automatically when running out of scope. However, the problem is: Sometimes team members write invalid lock-code such as

{
    ScopedLock(mutex);   // anonymous
    xxx;
}

The above code is wrong because the ScopedLock object is constructed and destructed immediately, so it fails to lock the expected area (xxx). I want the compiler to give an error when trying to compile such code. Can this be done?

I have searched g++ warning options, but fail to find the right one.

Answer 1

I have seen an interesting trick in one codebase, but it only works if your scoped_lock type is not a template (std::scoped_lock is).

#define scoped_lock(x) static_assert(false, "you forgot the variable name")

If you use the class correctly, you have

scoped_lock lock(mutex);

and since the scoped_lock identifier isn't followed by an open paren, the macro won't trigger and the code will remain as it is. If you write\

scoped_lock(mutex);

the macro will trigger and the code will be substituted with

static_assert(false, "you forgot the variable name");

This will generate an informative message.

If you use a qualified name

threads::scoped_lock(mutext);

then the result will still not compile, but the message won't be as nice.

Of course, if your lock is a template, the bad code is

scoped_lock<mutex_type>(mutex);

which won't trigger the macro.

Answer 2

No, unfortunately there is no way to do this, as I explored in a blog post last year.

In it, I concluded:

I guess the moral of the story is to remember this story when using scoped_locks.

---

You can try to force all programmers in your team to use a macro, or a range-for trick, but then if you could guarantee that in every case then you'd be able to guarantee catching this bug in every case also.

You are looking for a way to programmatically catch this specific mistake when it's made, and there is none.

Answer 3

You can use a class and deleted function with the same name. Unfortunately this requires adding "class" keyword before the type.

class Guard
{
public:
  explicit Guard(void)
  {
  }
};

static void Guard(void) = delete;

int main()
{
  // Guard(); // Won't compile
  // Guard g; // Won't compile
  class Guard g;
}

Answer 4

To avoid this, introduce a macro which does this for you, always using the same name for the locker:

#define LOCK(mutex) ScopedLock _lock(mutex)

Then use it like this:

{
    LOCK(mutex);
    xxx;
}

---

As an alternative, Java's synchronize block can be simulated using a macro construct: In a for-loop running always exactly once, I instantiate such a locker in the initialization statement of the for-loop, so it gets destroyed when leaving the for-loop.

However, it has some pitfalls, unexpected behavior of a break statement being one example. This "hack" is introduced here.

---

Of course, none of the above methods fully avoid accidental code like your example. But if you're used to write locking mutexes using one of the two macros, it will less likely happen. As the name of the locker class will then never appear in the code except in the macro definition, you can even introduce a commit hook in a version control system to avoid committing invalid code.

Answer 5

AFAIK there's no such a flag in gcc. A static analyzer may better suit your needs.

Answer 6

In C++17, a type can be marked [[nodiscard]], in which case a warning is encouraged for an expression that discards a value of that type (including by the case described here that resembles a declaration of a variable). In C++20, it can be applied to individual constructors as well if only some of them cause this sort of problem.

Answer 7

replace it with macro

#define CON2(x,y) x##y
#define CON(x,y) CON2(x,y)
#define LOCK(x)  ScopedLock CON(unique_,__COUNTER__)(mutex)

usage

{
  LOCK(mutex);
  //do stuff
}

This macro will generate unique names for locks, allowing lockeng of other mutexes in inner scopes