Interdire la création des objets temporaires
Lors du crash du débogage dans une application multithread, j'ai finalement localisé le problème dans cette Déclaration:
CSingleLock(&m_criticalSection, TRUE);
Notez qu'il crée un objet sans nom de la classe CSingleLock et que l'objet de section critique est donc déverrouillé immédiatement après cette instruction. Ce n'est évidemment pas ce que le codeur voulait. Cette erreur a été causée par une simple erreur de frappe. Ma question Est, est-ce que je peux empêcher l'objet temporaire d'une classe d'être créé au moment de la compilation elle-même c'est-à-dire que le type de code ci-dessus devrait générer une erreur de compilateur. En général, je pense que chaque fois qu'une classe essaie de faire une sorte d'acquisition de ressources alors l'objet temporaire de la classe ne devrait pas être autorisé. Est-il possible de les faire respecter?
7 réponses
Edit:, Comme j_random_hacker notes, il est possible de forcer l'utilisateur à déclarer un objet nommé afin de prendre un verrou.
Cependant, même si la création de temporaires était en quelque sorte interdite pour votre classe, alors l'utilisateur pourrait faire une erreur similaire:
// take out a lock:
if (m_multiThreaded)
{
CSingleLock c(&m_criticalSection, TRUE);
}
// do other stuff, assuming lock is held
En fin de Compte, l'utilisateur doit comprendre l'impact d'une ligne de code qu'il écrit. Dans ce cas, ils doivent savoir qu'ils créent un objet et ils doivent savoir combien de temps cela dure.
Un autre erreur probable:
CSingleLock *c = new CSingleLock(&m_criticalSection, TRUE);
// do other stuff, don't call delete on c...
Ce qui vous amènerait à demander "Est-il possible d'empêcher l'utilisateur de ma classe de l'allouer sur le tas"? À laquelle la réponse serait la même.
En C++0x, il y aura une autre façon de faire tout cela, en utilisant lambdas. Définir une fonction:
template <class TLock, class TLockedOperation>
void WithLock(TLock *lock, const TLockedOperation &op)
{
CSingleLock c(lock, TRUE);
op();
}
Cette fonction capture L'utilisation correcte de CSingleLock. Maintenant, laissez les utilisateurs faire ceci:
WithLock(&m_criticalSection,
[&] {
// do stuff, lock is held in this context.
});
C'est beaucoup plus difficile pour l'utilisateur de bousiller. La syntaxe semble étrange au début, mais [&] suivi de un bloc de code signifie "définir une fonction qui ne prend pas d'args, et si je me réfère à quelque chose par nom et que c'est le nom de quelque chose en dehors (par exemple une variable locale dans la fonction contenant), laissez-moi y accéder par référence non const, donc je peux le modifier.)
Tout d'abord, Earwicker fait quelques bons points - vous ne pouvez pas empêcher toute mauvaise utilisation accidentelle de cette construction.
Mais pour votre cas spécifique, cela peut en fait être évité. C'est parce que C++ fait une distinction (étrange) concernant les objets temporaires: les fonctions libres ne peuvent pas prendre de références non const à des objets temporaires. donc, afin d'éviter les verrous qui se glissent dans et hors de l'existence, il suffit de déplacer le code de verrouillage hors du constructeur CSingleLock et dans un fonction (que vous pouvez faire un ami pour éviter d'exposer les internes en tant que méthodes):
class CSingleLock {
friend void Lock(CSingleLock& lock) {
// Perform the actual locking here.
}
};
Le déverrouillage est toujours effectué dans le destructeur.
À utiliser:
CSingleLock myLock(&m_criticalSection, TRUE);
Lock(myLock);
Oui, c'est un peu plus compliqué à écrire. Mais maintenant, le compilateur se plaindra si vous essayez:
Lock(CSingleLock(&m_criticalSection, TRUE)); // Error! Caught at compile time.
Parce que le paramètre non-const ref de Lock() ne peut pas se lier à un temporaire.
Peut-être étonnamment, les méthodes de classe peuvent fonctionner sur des temporaires - c'est pourquoi Lock() doit être libre fonction. Si vous supprimez le spécificateur friend et le paramètre de fonction dans l'extrait supérieur pour faire de Lock() une méthode, le compilateur vous permettra volontiers d'écrire:
CSingleLock(&m_criticalSection, TRUE).Lock(); // Yikes!
MS COMPILER NOTE: les versions MSVC++ jusqu'à Visual Studio. net 2003 autorisaient incorrectement les fonctions à se lier à des références non const dans les versions antérieures à VC++ 2005. ce comportement a été corrigé dans VC++ 2005 et au-dessus.
Non, il n'y a aucun moyen de le faire. Cela casserait presque tout le code C++ qui repose fortement sur la création de temporaires sans nom. Votre seule solution pour des classes spécifiques est de rendre leurs constructeurs privés, puis de les construire toujours via une sorte d'usine. Mais je pense que le remède est pire que la maladie!
Je ne pense pas.
Bien que ce ne soit pas une chose sensée à faire - comme vous l'avez découvert avec votre bug - il n'y a rien de "illégal" dans la déclaration. Le compilateur n'a aucun moyen de savoir si la valeur de retour de la méthode est "vitale" ou non.
Le compilateur ne devrait pas interdire la création d'objets temporaires, à mon humble avis.
Spécialement des cas comme la réduction d'un vecteur, vous avez vraiment besoin d'un objet temporaire à créer.
std::vector<T>(v).swap(v);
Bien que ce soit un peu difficile, mais la révision du code et les tests unitaires devraient résoudre ces problèmes.
Sinon, voici la solution d'un pauvre homme:
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
Qu'en est-il de ce qui suit? Abuse légèrement du préprocesseur, mais c'est assez intelligent que je pense qu'il devrait être inclus:
class CSingleLock
{
...
};
#define CSingleLock class CSingleLock
Maintenant, oublier de nommer les résultats temporaires dans une erreur, car alors que ce qui suit est valide c++:
class CSingleLock lock(&m_criticalSection, true); // Compiles just fine!
Le même code, mais en omettant le nom, n'est pas:
class CSingleLock(&m_criticalSection, true); // <-- ERROR!
Je vois que dans 5 ans, personne n'a trouvé la solution la plus simple:
#define LOCK(x) CSingleLock lock(&x, TRUE);
...
void f() {
LOCK(m_criticalSection);
Et maintenant n'utilisez cette macro que pour créer des verrous. Plus aucune chance de créer des temporaires! Cela a l'avantage supplémentaire que la macro peut être facilement augmentée pour effectuer n'importe quel type de vérification dans les builds de débogage, par exemple détecter un verrouillage récursif inapproprié, enregistrer le fichier et la ligne du verrou, et bien plus encore.