Запрещение создания временных объектов

При отладке сбоев в многопоточном приложении я, наконец, обнаружил проблему в этом утверждении:

CSingleLock(&m_criticalSection, TRUE);

Обратите внимание, что он создает неназванный объект класса CSingleLock и, следовательно, объект критического раздела разблокируется сразу после этого оператора. Это явно не то, что хотел кодер. Эта ошибка была вызвана простой ошибкой ввода. Мой вопрос заключается в том, могу ли я каким-то образом предотвратить создание временного объекта класса во время самой компиляции, то есть приведенный выше тип кода должен вызвать ошибку компилятора. В общем, я думаю, что всякий раз, когда класс пытается выполнить какое-то приобретение ресурса, временный объект этого класса не должен быть разрешен. Есть ли способ обеспечить это?

Источник

7 ответов

Решение

Редактировать: Как отмечает j_random_hacker, можно заставить пользователя объявить именованный объект, чтобы снять блокировку.

Однако, даже если создание временных классов было каким-то образом запрещено для вашего класса, то пользователь может совершить аналогичную ошибку:

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

// do other stuff, assuming lock is held

В конечном счете, пользователь должен понимать влияние строки кода, которую они пишут. В этом случае они должны знать, что они создают объект, и они должны знать, как долго он длится.

Еще одна вероятная ошибка:

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

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

Что заставит вас спросить: "Можно ли как-то помешать пользователю моего класса разместить его в куче"? На что ответ будет таким же.

В C++0x будет еще один способ сделать все это, используя лямбды. Определить функцию:

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

Эта функция фиксирует правильное использование CSingleLock. Теперь пусть пользователи делают это:

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

Это намного сложнее для пользователя испортить. Синтаксис сначала выглядит странно, но [&], за которым следует блок кода, означает "Определить функцию, которая не принимает аргументов, и если я ссылаюсь на что-либо по имени, и это имя чего-то извне (например, локальная переменная в содержащей функция) позвольте мне получить доступ к нему по неконстантной ссылке, чтобы я мог изменить его.)

Источник

Во-первых, Earwicker делает несколько хороших замечаний - вы не можете предотвратить любое случайное неправильное использование этой конструкции.

Но для вашего конкретного случая этого на самом деле можно избежать. Это потому, что в C++ есть одно (странное) различие в отношении временных объектов: свободные функции не могут принимать неконстантные ссылки на временные объекты. Поэтому, чтобы избежать блокировок, которые появляются и исчезают, просто переместите код блокировки из CSingleLock конструктор и в свободную функцию (которую вы можете сделать другом, чтобы не показывать внутренности как методы):

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

Разблокировка все еще выполняется в деструкторе.

Использовать:

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

Да, это немного громоздко писать. Но теперь компилятор будет жаловаться, если вы попробуете:

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

Потому что неконстантный параметр ref Lock() не может привязаться к временному.

Возможно, что удивительно, методы класса могут работать с временными файлами - вот почему Lock() должна быть свободная функция. Если вы бросите friend спецификатор и параметр функции в верхнем фрагменте, чтобы сделать Lock() метод, тогда компилятор с радостью позволит вам написать:

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

ПРИМЕЧАНИЕ КОМПИЛЕРА MS: версии MSVC++ до Visual Studio .NET 2003 некорректно допускали привязку функций к неконстантным ссылкам в версиях до VC++ 2005. Это поведение было исправлено в VC++ 2005 и выше.

Источник

Нет, это невозможно сделать. Это нарушит почти весь код C++, который в значительной степени зависит от создания безымянных временных. Ваше единственное решение для конкретных классов - сделать их конструкторы частными, а затем всегда конструировать их через какую-то фабрику. Но я думаю, что лекарство хуже, чем болезнь!

Источник

Старый вопрос, но я хочу добавить пару моментов.

Определив макрос-функцию с тем же именем, что и у класса, вы можете вызвать статическое утверждение с полезным сообщением, когда кто-то забудет имя переменной. живи здесь

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?")

Макрос не будет соответствовать, если есть имя переменной. Однако это не помогает в случае единой инициализации; ты не можешь пойматьCSingleLock{&m, true}. Ответ PfhorSlayer работает с унифицированной инициализацией, поэтому его безопаснее использовать за счет более запутанного сообщения об ошибке. Я все равно рекомендую это решение вместо своего. К сожалению, все эти макрорешения не работают, когда тип находится в пространстве имен.

Другое решение - создать предупреждение компилятора, используя [[nodiscard]]

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

Если вы создадите временный лязг, вас предупредит, говоря:

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

У GCC 9.2 проблема с [[nodiscard]]на конструкторах. Он по-прежнему дает дополнительные предупреждения, если вы не используете результат. Проблема исправлена ​​на голове, которая на момент написания это gcc-10 20191217 на wandbox.

Источник

Я так не думаю.

Хотя это не очень разумно - как вы выяснили с помощью вашей ошибки - в этом утверждении нет ничего "незаконного". Компилятор не может знать, является ли возвращаемое значение из метода "жизненно важным" или нет.

Источник

Компилятор не должен запрещать создание временных объектов, IMHO.

Особенно в таких случаях, как сжатие вектора, вам действительно нужен временный объект для создания.

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
Источник

Я вижу, что за 5 лет никто не придумал самое простое решение:

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

И теперь используйте только этот макрос для создания замков. Больше нет никаких шансов создать временных! Это дает дополнительное преимущество, заключающееся в том, что макрос может быть легко дополнен для выполнения любого вида проверки в сборках отладки, например, для обнаружения неподходящей рекурсивной блокировки, записи файла и строки блокировки, и многого другого.

Источник

Как насчет следующего? Немного злоупотребляет препроцессором, но он достаточно умен, поэтому я думаю, что он должен быть включен:

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

Теперь забываем назвать временные результаты в виде ошибки, потому что пока действует следующий C++:

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

Тот же код, но без имени, не является:

class CSingleLock(&m_criticalSection, true); // <-- ERROR!
Источник
Другие вопросы по тегам