[clang-tools-extra] [clang-tidy] Unsafe CRTP check (PR #82403)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 28 17:48:07 PST 2024
isuckatcs wrote:
I ran the checker on LLVM, Clang and Clang-Tidy (`run-clang-tidy.py` in my build directory). There are a total of 33 warnings by the checker, which can be found below. I omitted the use site, where the template parameter says `Derived`, because I think that makes it obvious that it's a CRTP.
```c++
template <typename Derived, typename KeyType, typename UnversionedDataType>
class VersionedTableInfo { ... };
```
```
.../llvm-project/clang/lib/APINotes/APINotesReader.cpp:49:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
49 | class VersionedTableInfo {
| ^
```
```c++
template <typename ImplT, typename IteratorT, typename CollectionT>
class CalcLiveRangeUtilBase {
protected:
LiveRange *LR;
protected:
CalcLiveRangeUtilBase(LiveRange *LR) : LR(LR) {}
...
}
class CalcLiveRangeUtilVector;
using CalcLiveRangeUtilVectorBase =
CalcLiveRangeUtilBase<CalcLiveRangeUtilVector, LiveRange::iterator,
LiveRange::Segments>;
class CalcLiveRangeUtilVector : public CalcLiveRangeUtilVectorBase { ... }
```
```
.../llvm-project/llvm/lib/CodeGen/LiveInterval.cpp:70:3: warning: protected contructor allows the CRTP to be inherited from as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
70 | CalcLiveRangeUtilBase(LiveRange *LR) : LR(LR) {}
| ^
```
```c++
template <typename T>
class TestVisitor : public clang::RecursiveASTVisitor<T> { ... }
class ClassDeclXVisitor : public TestVisitor<ClassDeclXVisitor> { ... }
```
```
.../llvm-project/clang/unittests/Tooling/RefactoringTest.cpp:652:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
652 | class TestVisitor : public clang::RecursiveASTVisitor<T> {
| ^
```
```c++
template <class Derived> struct DiagTextVisitor { ... }
```
```
.../llvm-project/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp:725:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
725 | DiagTextVisitor(DiagnosticTextBuilder &Builder) : Builder(Builder) {}
| ^
```
```c++
template <typename Derived> class OpSplitter { ... }
```
```
.../llvm-project/llvm/lib/Transforms/Scalar/SROA.cpp:3765:5: warning: protected contructor allows the CRTP to be inherited from as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
3765 | OpSplitter(Instruction *InsertionPoint, Value *Ptr, Type *BaseTy,
| ^
```
```c++
template <typename Derived>
class Class5Tmpl : private llvm::TrailingObjects<Derived, float, int> { ... }
```
```
.../llvm-project/llvm/unittests/Support/TrailingObjectsTest.cpp:243:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
243 | class Class5Tmpl : private llvm::TrailingObjects<Derived, float, int> {
| ^
```
```c++
template <typename Derived>
struct SimpleTransformVisitor : public TypeVisitor<Derived, QualType> { ... }
```
```
.../llvm-project/clang/lib/AST/Type.cpp:888:12: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
888 | explicit SimpleTransformVisitor(ASTContext &ctx) : Ctx(ctx) {}
| ^
```
```c++
template <typename DERIVED>
class ClusterAnalysis { ... }
```
```
.../llvm-project/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:721:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
721 | ClusterAnalysis(RegionStoreManager &rm, ProgramStateManager &StateMgr,
| ^
```
```c++
template <typename DerivedCCG, typename FuncTy, typename CallTy>
class CallsiteContextGraph { ... }
```
```
.../llvm-project/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp:150:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
150 | CallsiteContextGraph() = default;
| ^
.../llvm-project/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp:151:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
151 | CallsiteContextGraph(const CallsiteContextGraph &) = default;
| ^
.../llvm-project/llvm/lib/Transforms/IPO/MemProfContextDisambiguation.cpp:152:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
152 | CallsiteContextGraph(CallsiteContextGraph &&) = default;
| ^
```
```c++
template <typename DerivedT, typename IRUnitT,
typename AnalysisManagerT = AnalysisManager<IRUnitT>,
typename... ExtraArgTs>
class MockAnalysisHandleBase { ... }
```
```
.../llvm-project/llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp:43:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
43 | class MockAnalysisHandleBase {
| ^
```
```c++
template <typename DerivedT, typename IRUnitT,
typename AnalysisManagerT = AnalysisManager<IRUnitT>,
typename... ExtraArgTs>
class MockPassHandleBase { ... }
```
```
.../llvm-project/llvm/unittests/Transforms/Scalar/LoopPassManagerTest.cpp:154:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
154 | class MockPassHandleBase {
| ^
```
```c++
template <class Derived> struct StructVisitor { ... }
```
```
.../llvm-project/clang/lib/CodeGen/CGNonTrivialStruct.cpp:37:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
37 | StructVisitor(ASTContext &Ctx) : Ctx(Ctx) {}
| ^
```
In this next one there are no warnings for `CopiedTypeVisitor`, but there should be.
```c++
template <class Derived, bool IsMove>
struct CopyStructVisitor : StructVisitor<Derived>,
CopiedTypeVisitor<Derived, IsMove> { ... }
```
```
.../llvm-project/clang/lib/CodeGen/CGNonTrivialStruct.cpp:83:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
83 | CopyStructVisitor(ASTContext &Ctx) : StructVisitor<Derived>(Ctx) {}
| ^
```
```
.../llvm-project/clang/lib/CodeGen/CGNonTrivialStruct.cpp:150:33: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
150 | template <class Derived> struct GenFuncNameBase {
| ^
```
```c++
template <class Derived>
struct GenUnaryFuncName : StructVisitor<Derived>, GenFuncNameBase<Derived> { ... }
```
```
.../llvm-project/clang/lib/CodeGen/CGNonTrivialStruct.cpp:220:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
220 | GenUnaryFuncName(StringRef Prefix, CharUnits DstAlignment, ASTContext &Ctx)
| ^
```
```
.../llvm-project/clang/lib/CodeGen/CGNonTrivialStruct.cpp:335:33: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
335 | template <class Derived> struct GenFuncBase {
| ^
```
```c++
struct GenBinaryFunc : CopyStructVisitor<Derived, IsMove>,
GenFuncBase<Derived> { ... }
```
```
.../llvm-project/clang/lib/CodeGen/CGNonTrivialStruct.cpp:509:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
509 | GenBinaryFunc(ASTContext &Ctx) : CopyStructVisitor<Derived, IsMove>(Ctx) {}
| ^
```
```
.../llvm-project/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1983:32: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
1983 | template <class Derived> class ConstraintAssignorBase {
| ^
```
```c++
template <typename DerivedT, typename IRUnitT,
typename AnalysisManagerT = AnalysisManager<IRUnitT>,
typename... ExtraArgTs>
class MockAnalysisHandleBase { ... }
```
```
.../llvm-project/llvm/unittests/IR/PassBuilderCallbacksTest.cpp:44:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
44 | class MockAnalysisHandleBase {
| ^
```
```c++
template <typename DerivedT, typename IRUnitT,
typename AnalysisManagerT = AnalysisManager<IRUnitT>,
typename... ExtraArgTs>
class MockPassHandleBase { ... }
```
```
.../llvm-project/llvm/unittests/IR/PassBuilderCallbacksTest.cpp:121:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
121 | class MockPassHandleBase {
| ^
```
In the above base class there is one use of a CRTP, `PassInfoMixin<>`, for which the check fails to report warnings, but it should be.
```c++
class MockPassHandleBase {
public:
class Pass : public PassInfoMixin<Pass> { ... }
...
}
```
```c++
template <class Derived>
struct DestroyNRVOVariable : EHScopeStack::Cleanup { ... }
```
```
.../llvm-project/clang/lib/CodeGen/CGDecl.cpp:518:5: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
518 | DestroyNRVOVariable(Address addr, QualType type, llvm::Value *NRVOFlag)
| ^
```
Here there are no warnings for `ConstStmtVisitor<>`, but there should be.
```c++
template <class Derived>
class ExprEvaluatorBase
: public ConstStmtVisitor<Derived, bool> { ... }
```
```
.../llvm-project/clang/lib/AST/ExprConstant.cpp:7673:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
7673 | ExprEvaluatorBase(EvalInfo &Info) : Info(Info) {}
| ^
```
```c++
template<class Derived>
class LValueExprEvaluatorBase
: public ExprEvaluatorBase<Derived> { ... }
```
```
/home/danny/llvm-project/clang/lib/AST/ExprConstant.cpp:8311:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
8311 | LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result, bool InvalidBaseOK)
| ^
```
```c++
template <typename Derived, typename KeyType, typename UnversionedDataType>
class VersionedTableInfo { ... }
```
```
.../llvm-project/clang/lib/APINotes/APINotesWriter.cpp:458:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
458 | class VersionedTableInfo {
| ^
```
```c++
template <typename Derived, typename UnversionedDataType>
class CommonTypeTableInfo
: public VersionedTableInfo<Derived, ContextTableKey, UnversionedDataType> { ... }
```
```
.../llvm-project/clang/lib/APINotes/APINotesWriter.cpp:1093:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
1093 | class CommonTypeTableInfo
| ^
```
```c++
template <typename Derived, typename UnversionedDataType>
class CommonTypeTableInfo
: public VersionedTableInfo<Derived, ContextTableKey, UnversionedDataType> { ... }
```
```
.../llvm-project/clang/lib/APINotes/APINotesWriter.cpp:1093:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
1093 | class CommonTypeTableInfo
| ^
```
```c++
/// A CRTP base class for emitting expressions of retainable object
/// pointer type in ARC.
template <typename Impl, typename Result> class ARCExprEmitter { ... }
```
```
.../llvm-project/clang/lib/CodeGen/CGObjC.cpp:3104:3: warning: protected contructor allows the CRTP to be inherited from as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
3104 | ARCExprEmitter(CodeGenFunction &CGF) : CGF(CGF) {}
| ^
```
```c++
template<typename Derived, typename ResultList, typename Result,
typename Subobject>
class DefaultedComparisonVisitor { ... }
```
```
.../llvm-project/clang/lib/Sema/SemaDeclCXX.cpp:7949:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
7949 | DefaultedComparisonVisitor(Sema &S, CXXRecordDecl *RD, FunctionDecl *FD,
| ^
```
```c++
/// CRTP base class for visiting operations performed by a special member
/// function (or inherited constructor).
template<typename Derived>
struct SpecialMemberVisitor { ... }
```
```
.../llvm-project/clang/lib/Sema/SemaDeclCXX.cpp:9269:3: warning: public contructor allows the CRTP to be constructed as a regular template class; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
9269 | SpecialMemberVisitor(Sema &S, CXXMethodDecl *MD, Sema::CXXSpecialMember CSM,
| ^
```
```
.../llvm-project/llvm/unittests/MIR/PassBuilderCallbacksTest.cpp:175:36: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
175 | template <typename DerivedT> class MockAnalysisHandleBase {
| ^
```
```
.../llvm-project/llvm/unittests/MIR/PassBuilderCallbacksTest.cpp:238:36: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
238 | template <typename DerivedT> class MockPassHandleBase {
| ^
```
```c++
template <typename Derived, typename KeyType, typename UnversionedDataType>
class VersionedTableInfo { ... }
```
```
.../llvm-project/clang/lib/APINotes/APINotesReader.cpp:49:7: warning: the implicit default constructor of the CRTP is publicly accessible; consider making it private and declaring the derived class as friend [bugprone-crtp-constructor-accessibility]
49 | class VersionedTableInfo {
| ^
```
TLDR, we have some false negatives, but the possitives look correct.
https://github.com/llvm/llvm-project/pull/82403
More information about the cfe-commits
mailing list