c++ - 不允许创建临时对象

Question

在多线程应用程序中调试崩溃时，我终于在以下语句中找到了问题：

CSingleLock(&m_criticalSection, TRUE);

请注意，它正在创建一个未命名的 CSingleLock 类对象，因此临界区对象在此语句之后立即解锁。这显然不是程序员想要的。这个错误是由一个简单的打字错误引起的。我的问题是，有没有办法阻止在编译时创建类的临时对象，即上述类型的代码应该生成编译器错误。一般来说，我认为每当一个类试图进行某种资源获取时，就不应该允许该类的临时对象。有没有办法强制执行？

Answer 1

Edit: As j_random_hacker notes, it is possible to force the user to declare a named object in order to take out a lock.

However, even if creation of temporaries was somehow banned for your class, then the user could make a similar mistake:

// take out a lock:
if (m_multiThreaded)
{
    CSingleLock c(&m_criticalSection, TRUE);
}

// do other stuff, assuming lock is held

Ultimately, the user has to understand the impact of a line of code that they write. In this case, they have to know that they're creating an object and they have to know how long it lasts.

Another likely mistake:

 CSingleLock *c = new CSingleLock(&m_criticalSection, TRUE);

 // do other stuff, don't call delete on c...

Which would lead you to ask "Is there any way I can stop the user of my class from allocating it on the heap"? To which the answer would be the same.

In C++0x there will be another way to do all this, by using lambdas. Define a function:

template <class TLock, class TLockedOperation>
void WithLock(TLock *lock, const TLockedOperation &op)
{
    CSingleLock c(lock, TRUE);
    op();
}

That function captures the correct usage of CSingleLock. Now let users do this:

WithLock(&m_criticalSection, 
[&] {
        // do stuff, lock is held in this context.
    });

This is much harder for the user to screw up. The syntax looks weird at first, but [&] followed by a code block means "Define a function that takes no args, and if I refer to anything by name and it is the name of something outside (e.g. a local variable in the containing function) let me access it by non-const reference, so I can modify it.)

Answer 2

First, Earwicker makes some good points -- you can't prevent every accidental misuse of this construct.

But for your specific case, this can in fact be avoided. That's because C++ does make one (strange) distinction regarding temporary objects: Free functions cannot take non-const references to temporary objects. So, in order to avoid locks that blip into and out of existence, just move the locking code out of the CSingleLock constructor and into a free function (which you can make a friend to avoid exposing internals as methods):

class CSingleLock {
    friend void Lock(CSingleLock& lock) {
        // Perform the actual locking here.
    }
};

Unlocking is still performed in the destructor.

To use:

CSingleLock myLock(&m_criticalSection, TRUE);
Lock(myLock);

Yes, it's slightly more unwieldy to write. But now, the compiler will complain if you try:

Lock(CSingleLock(&m_criticalSection, TRUE));   // Error! Caught at compile time.

Because the non-const ref parameter of Lock() cannot bind to a temporary.

Perhaps surprisingly, class methods can operate on temporaries -- that's why Lock() needs to be a free function. If you drop the friend specifier and the function parameter in the top snippet to make Lock() a method, then the compiler will happily allow you to write:

CSingleLock(&m_criticalSection, TRUE).Lock();  // Yikes!

MS COMPILER NOTE: MSVC++ versions up to Visual Studio .NET 2003 incorrectly allowed functions to bind to non-const references in versions prior to VC++ 2005. This behaviour has been fixed in VC++ 2005 and above.

Answer 3

不，没有办法做到这一点。这样做会破坏几乎所有严重依赖于创建无名临时文件的 C++ 代码。您对特定类的唯一解决方案是将它们的构造函数设为私有，然后始终通过某种工厂构造它们。但我认为治疗比疾病更糟糕！

Answer 4

我不这么认为。

虽然这不是一件明智的事情——正如你已经发现你的错误——但声明没有什么“非法”的。编译器无法知道该方法的返回值是否“重要”。

Answer 5

编译器不应该禁止创建临时对象，恕我直言。

特别是像缩小矢量这样的特殊情况，您确实需要创建临时对象。

std::vector<T>(v).swap(v);

虽然有点困难，但代码审查和单元测试仍然应该抓住这些问题。

否则，这是一个穷人的解决方案：

CSingleLock aLock(&m_criticalSection); //Don't use the second parameter whose default is FALSE

aLock.Lock();  //an explicit lock should take care of your problem

Answer 6

What about the following? Slightly abuses the preprocessor, but it's clever enough that I think it should be included:

class CSingleLock
{
    ...
};
#define CSingleLock class CSingleLock

Now forgetting to name the temporary results in an error, because while the following is valid C++:

class CSingleLock lock(&m_criticalSection, true); // Compiles just fine!

The same code, but omitting the name, is not:

class CSingleLock(&m_criticalSection, true); // <-- ERROR!

Answer 7

I see that in 5 years nobody has come up with the most simple solution:

#define LOCK(x) CSingleLock lock(&x, TRUE);
...
void f() {
   LOCK(m_criticalSection);

And now only use this macro for creating locks. No chance to create temporaries any more! This has the added benefit that the macro can be easily augmented to perform any kind of checking in debug builds, for example detecting inappropriate recursive locking, recording file and line of the lock, and much more.

Answer 8

Old question, but I have a couple of points to add.

By define a macro function with the same name as the class, you can trigger a static assertion with a helpful message when someone forgets the variable name. live here

class CSingleLock {
 public:
  CSingleLock (std::mutex*, bool) { }
};

// must come after class definition
#define CSingleLock(...) static_assert(false, \
    "Temporary CSingleLock objects are forbidden, did you forget a variable name?")

The macro won't match when there is a variable name. However, this doesn't help in the case of uniform initialization; you can't catch CSingleLock{&m, true}. PfhorSlayer's answer works with uniform initialization so it is safer to use, at the cost of a more confusing error message. I would still reccomend that solution over mine. Unfortunately all these macro solutions fail when the type is in a namespace.

---

Another solution is to produce a compiler warning using [[nodiscard]]

class CSingleLock {
 public:
  [[nodiscard]] CSingleLock (std::mutex*, bool) { }
};

If you create a temporary clang will warn you saying:

warning: ignoring temporary created by a constructor declared with 'nodiscard' attribute [-Wunused-value]
  CSingleLock(&m, true);
  ^~~~~~~~~~~~~~~~~~~~~

GCC 9.2 seems to have a problem with [[nodiscard]] on constructors though. It still gives additional warnings if you don't use the result. The problem is fixed on head which at the time of writing this is gcc-10 20191217 on wandbox.