================ @@ -0,0 +1,62 @@ +.. title:: clang-tidy - bugprone-unsafe-crtp + +bugprone-unsafe-crtp +==================== + +Finds CRTP used in an error-prone way. + +If the constructor of a class intended to be used in a CRTP is public, then +it allows users to construct that class on its own. + +Example: + +.. code-block:: c++ + + template <typename T> class CRTP { + public: + CRTP() = default; + }; + + class Good : CRTP<Good> {}; + Good GoodInstance; + + CRTP<int> BadInstance; + +If the constructor is protected, the possibility of an accidental instantiation +is prevented, however it can fade an error, when a different class is used as +the template parameter instead of the derived one. + +Example: + +.. code-block:: c++ + + template <typename T> class CRTP { + protected: + CRTP() = default; + }; + + class Good : CRTP<Good> {}; + Good GoodInstance; + + class Bad : CRTP<Good> {}; + Bad BadInstance; + +To ensure that no accidental instantiation happens, the best practice is to make +the constructor private and declare the derived class as friend. + +Example: + +.. code-block:: c++ + + template <typename T> class CRTP { + CRTP() = default; + friend T; + }; + + class Good : CRTP<Good> {}; + Good GoodInstance; + + class Bad : CRTP<Good> {}; + Bad CompileTimeError; + + CRTP<int> AlsoCompileTimeError; ---------------- isuckatcs wrote:
If the constructor is protected as in your example, the following will also compile, which is not desired. ```c++ struct Decl {}; struct Stmt : ASTNode<Decl> { protected: Stmt() : ASTNode() { } }; ``` This is exactly the kind of usage, this check wants to catch. Also note that with the suggested fix, your example still compiles. ```c++ template <class NodeFamily> struct ASTNode { private: ASTNode() { } friend NodeFamily; }; struct Stmt : ASTNode<Stmt> { protected: Stmt() : ASTNode() { } }; struct IfStmt : Stmt { IfStmt() : Stmt() { } }; ``` `IfStmt` calls the constructor of `Stmt`, and not the constructor of `ASTNode<>`. The latter constructor is called by `Stmt`, which has access to it either way. For the record, AFAIK the CRTP is supposed to describe `has a` relationships and not `is a` relationships, so making ASTNode a CRTP might not be an ideal design choice. https://github.com/llvm/llvm-project/pull/82403 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits