[clang-tidy] add modernize-use-constexpr check #146553
Open
+1,756
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-clang-tidy Author: Julian Schmidt (5chmidti) ChangesThis check finds all functions and variables that can be declared as Fixes #115622 Patch is 95.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146553.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index fd7affd22a463..4535df3451c95 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -19,6 +19,7 @@ set_target_properties(genconfusable PROPERTIES FOLDER "Clang Tools Extra/Sourceg
add_clang_library(clangTidyMiscModule STATIC
ConstCorrectnessCheck.cpp
+ ConstexprCheck.cpp
CoroutineHostileRAIICheck.cpp
DefinitionsInHeadersCheck.cpp
ConfusableIdentifierCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp
new file mode 100644
index 0000000000000..270ac1dd0fa48
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp
@@ -0,0 +1,936 @@
+//===--- ConstexprCheck.cpp - clang-tidy ----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ConstexprCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/OperationKinds.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/StmtCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Support/Casting.h"
+#include <cstddef>
+#include <functional>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+namespace {
+AST_MATCHER(FunctionDecl, locationPermitsConstexpr) {
+ const bool IsInMainFile =
+ Finder->getASTContext().getSourceManager().isInMainFile(
+ Node.getLocation());
+
+ if (IsInMainFile && Node.hasExternalFormalLinkage())
+ return false;
+ if (!IsInMainFile && !Node.isInlined())
+ return false;
+
+ return true;
+}
+
+AST_MATCHER(Expr, isCXX11ConstantExpr) {
+ return !Node.isValueDependent() &&
+ Node.isCXX11ConstantExpr(Finder->getASTContext());
+}
+
+AST_MATCHER(DeclaratorDecl, isInMacro) {
+ const SourceRange R =
+ SourceRange(Node.getInnerLocStart(), Node.getLocation());
+
+ return Node.getLocation().isMacroID() || Node.getEndLoc().isMacroID() ||
+ utils::rangeContainsMacroExpansion(
+ R, &Finder->getASTContext().getSourceManager()) ||
+ utils::rangeIsEntirelyWithinMacroArgument(
+ R, &Finder->getASTContext().getSourceManager());
+}
+
+AST_MATCHER(Decl, hasNoRedecl) {
+ // There is always the actual declaration
+ return !Node.redecls().empty() &&
+ std::next(Node.redecls_begin()) == Node.redecls_end();
+}
+
+AST_MATCHER(Decl, allRedeclsInSameFile) {
+ const SourceManager &SM = Finder->getASTContext().getSourceManager();
+ const SourceLocation L = Node.getLocation();
+ for (const Decl *ReDecl : Node.redecls()) {
+ if (!SM.isWrittenInSameFile(L, ReDecl->getLocation()))
+ return false;
+ }
+ return true;
+}
+
+AST_MATCHER(FunctionDecl, isConstexprSpecified) {
+ return Node.isConstexprSpecified();
+}
+
+bool satisfiesConstructorPropertiesUntil20(const CXXConstructorDecl *Ctor,
+ ASTContext &Ctx) {
+ const CXXRecordDecl *Rec = Ctor->getParent();
+ llvm::SmallPtrSet<const RecordDecl *, 8> Bases{};
+ for (const CXXBaseSpecifier Base : Rec->bases()) {
+ Bases.insert(Base.getType()->getAsRecordDecl());
+ }
+ llvm::SmallPtrSet<const FieldDecl *, 8> Fields{Rec->field_begin(),
+ Rec->field_end()};
+ llvm::SmallPtrSet<const FieldDecl *, 4> Indirects{};
+
+ for (const CXXCtorInitializer *const Init : Ctor->inits()) {
+ const Type *InitType = Init->getBaseClass();
+ if (InitType && InitType->isRecordType()) {
+ const auto *ConstructingInit =
+ llvm::dyn_cast<CXXConstructExpr>(Init->getInit());
+ if (ConstructingInit &&
+ !ConstructingInit->getConstructor()->isConstexprSpecified())
+ return false;
+ }
+
+ if (Init->isBaseInitializer()) {
+ Bases.erase(Init->getBaseClass()->getAsRecordDecl());
+ continue;
+ }
+
+ if (Init->isMemberInitializer()) {
+ const FieldDecl *Field = Init->getMember();
+
+ if (Field->isAnonymousStructOrUnion())
+ Indirects.insert(Field);
+
+ Fields.erase(Field);
+ continue;
+ }
+ }
+
+ for (const auto &Match :
+ match(cxxRecordDecl(forEach(indirectFieldDecl().bind("indirect"))), *Rec,
+ Ctx)) {
+ const auto *IField = Match.getNodeAs<IndirectFieldDecl>("indirect");
+
+ size_t NumInitializations = false;
+ for (const NamedDecl *ND : IField->chain())
+ NumInitializations += Indirects.erase(llvm::dyn_cast<FieldDecl>(ND));
+
+ if (NumInitializations != 1)
+ return false;
+
+ for (const NamedDecl *ND : IField->chain())
+ Fields.erase(llvm::dyn_cast<FieldDecl>(ND));
+ }
+
+ if (!Fields.empty())
+ return false;
+
+ return true;
+}
+
+const Type *unwrapPointee(const Type *T) {
+ if (!T->isPointerOrReferenceType())
+ return T;
+
+ while (T && T->isPointerOrReferenceType()) {
+ if (T->isReferenceType()) {
+ const QualType QType = T->getPointeeType();
+ if (!QType.isNull())
+ T = QType.getTypePtr();
+ } else
+ T = T->getPointeeOrArrayElementType();
+ }
+
+ return T;
+}
+
+bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ const bool ConservativeLiteralType);
+
+bool isLiteralType(const Type *T, const ASTContext &Ctx,
+ const bool ConservativeLiteralType) {
+ if (!T)
+ return false;
+
+ if (!T->isLiteralType(Ctx))
+ return false;
+
+ if (!ConservativeLiteralType)
+ return T->isLiteralType(Ctx) && !T->isVoidType();
+
+ if (T->isIncompleteType() || T->isIncompleteArrayType())
+ return false;
+
+ T = unwrapPointee(T);
+ if (!T)
+ return false;
+
+ assert(!T->isPointerOrReferenceType());
+
+ if (T->isIncompleteType() || T->isIncompleteArrayType())
+ return false;
+
+ if (T->isLiteralType(Ctx))
+ return true;
+
+ if (const auto *Rec = T->getAsCXXRecordDecl()) {
+ if (llvm::any_of(Rec->ctors(), [](const CXXConstructorDecl *Ctor) {
+ return !Ctor->isCopyOrMoveConstructor() &&
+ Ctor->isConstexprSpecified();
+ }))
+ return false;
+
+ for (const CXXBaseSpecifier Base : Rec->bases()) {
+ if (!isLiteralType(Base.getType(), Ctx, ConservativeLiteralType))
+ return false;
+ }
+ }
+
+ if (const Type *ArrayElementType = T->getArrayElementTypeNoTypeQual())
+ return isLiteralType(ArrayElementType, Ctx, ConservativeLiteralType);
+
+ return false;
+}
+
+bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ const bool ConservativeLiteralType) {
+ return isLiteralType(QT.getTypePtr(), Ctx, ConservativeLiteralType);
+}
+
+bool satisfiesProperties11(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified()) {
+ return true;
+ }
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (Method &&
+ (Method->isVirtual() ||
+ !match(cxxMethodDecl(hasBody(cxxTryStmt())), *Method, Ctx).empty()))
+ return false;
+
+ if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(FDecl);
+ Ctor && (!satisfiesConstructorPropertiesUntil20(Ctor, Ctx) ||
+ llvm::any_of(Ctor->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) {
+ return Base.isVirtual();
+ })))
+ return false;
+
+ if (const auto *Dtor = llvm::dyn_cast<CXXDestructorDecl>(FDecl);
+ Dtor && !Dtor->isTrivial())
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor11 : public clang::RecursiveASTVisitor<Visitor11> {
+ public:
+ using Base = clang::RecursiveASTVisitor<Visitor11>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor11(ASTContext &Ctx, bool ConservativeLiteralType)
+ : Ctx(Ctx), ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool WalkUpFromNullStmt(NullStmt *) {
+ Possible = false;
+ return true;
+ }
+ bool WalkUpFromDeclStmt(DeclStmt *DS) {
+ for (const Decl *D : DS->decls())
+ if (!llvm::isa<StaticAssertDecl, TypedefNameDecl, UsingDecl,
+ UsingDirectiveDecl>(D)) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromExpr(Expr *) { return true; }
+ bool WalkUpFromCompoundStmt(CompoundStmt *S) {
+ for (const DynTypedNode &Node : Ctx.getParents(*S))
+ if (Node.get<FunctionDecl>() != nullptr)
+ return true;
+
+ Possible = false;
+ return false;
+ }
+ bool WalkUpFromStmt(Stmt *) {
+ Possible = false;
+ return false;
+ }
+
+ bool WalkUpFromReturnStmt(ReturnStmt *) {
+ ++NumReturns;
+ if (NumReturns != 1U) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromCastExpr(CastExpr *CE) {
+ if (llvm::is_contained(
+ {
+ CK_LValueBitCast,
+ CK_IntegralToPointer,
+ CK_PointerToIntegral,
+ },
+ CE->getCastKind())) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXDynamicCastExpr(CXXDynamicCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseCXXReinterpretCastExpr(CXXReinterpretCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseType(QualType QT) {
+ if (QT.isNull())
+ return true;
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType)) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseType(QT);
+ }
+
+ bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) {
+ if (const auto *Ctor = CE->getConstructor();
+ Ctor && !Ctor->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+
+ return true;
+ }
+ bool WalkUpFromCallExpr(CallExpr *CE) {
+ if (const auto *FDecl =
+ llvm::dyn_cast_if_present<FunctionDecl>(CE->getCalleeDecl());
+ FDecl && !FDecl->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ bool Possible = true;
+ size_t NumReturns = 0;
+ };
+
+ Visitor11 V{Ctx, ConservativeLiteralType};
+ V.TraverseDecl(const_cast<FunctionDecl *>(FDecl));
+ if (!V.Possible)
+ return false;
+
+ return true;
+}
+
+// The only difference between C++14 and C++17 is that `constexpr` lambdas
+// can be used in C++17.
+bool satisfiesProperties1417(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified())
+ return true;
+
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (Method && Method->isVirtual())
+ return false;
+
+ if (llvm::isa<CXXConstructorDecl>(FDecl) &&
+ llvm::any_of(
+ Method->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) { return Base.isVirtual(); }))
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor14 : public clang::RecursiveASTVisitor<Visitor14> {
+ public:
+ using Base = clang::RecursiveASTVisitor<Visitor14>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor14(bool CXX17, ASTContext &Ctx, bool ConservativeLiteralType,
+ bool AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ : CXX17(CXX17), Ctx(Ctx),
+ ConservativeLiteralType(ConservativeLiteralType),
+ AddConstexprToMethodOfClassWithoutConstexprConstructor(
+ AddConstexprToMethodOfClassWithoutConstexprConstructor) {}
+
+ bool TraverseGotoStmt(GotoStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseLabelStmt(LabelStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXTryStmt(CXXTryStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseGCCAsmStmt(GCCAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseMSAsmStmt(MSAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseDecompositionDecl(DecompositionDecl * /*DD*/) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseVarDecl(VarDecl *VD) {
+ const auto StorageDur = VD->getStorageDuration();
+ Possible = VD->hasInit() &&
+ isLiteralType(VD->getType(), VD->getASTContext(),
+ ConservativeLiteralType) &&
+ (StorageDur != StorageDuration::SD_Static &&
+ StorageDur != StorageDuration::SD_Thread);
+ return Possible && Base::TraverseVarDecl(VD);
+ }
+ bool TraverseLambdaExpr(LambdaExpr *LE) {
+ if (CXX17) {
+ Possible = satisfiesProperties1417(
+ LE->getCallOperator(), Ctx, ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor);
+ return Possible;
+ }
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseDeclRefExpr(DeclRefExpr *DRef) {
+ if (const auto *D = llvm::dyn_cast_if_present<VarDecl>(DRef->getDecl());
+ D && !D->isLocalVarDeclOrParm() && D->hasGlobalStorage()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromCastExpr(CastExpr *CE) {
+ if (llvm::is_contained(
+ {
+ CK_LValueBitCast,
+ CK_IntegralToPointer,
+ CK_PointerToIntegral,
+ },
+ CE->getCastKind())) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXDynamicCastExpr(CXXDynamicCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseCXXReinterpretCastExpr(CXXReinterpretCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ const bool CXX17;
+ bool Possible = true;
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor;
+ };
+
+ Visitor14 V{Ctx.getLangOpts().CPlusPlus17 != 0, Ctx, ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor};
+ V.TraverseDecl(const_cast<FunctionDecl *>(FDecl));
+ if (!V.Possible)
+ return false;
+
+ if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(FDecl);
+ Ctor && !satisfiesConstructorPropertiesUntil20(Ctor, Ctx))
+ return false;
+
+ if (const auto *Dtor = llvm::dyn_cast<CXXDestructorDecl>(FDecl);
+ Dtor && !Dtor->isTrivial())
+ return false;
+
+ class BodyVisitor : public clang::RecursiveASTVisitor<BodyVisitor> {
+ public:
+ using Base = clang::RecursiveASTVisitor<BodyVisitor>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ explicit BodyVisitor(ASTContext &Ctx, bool ConservativeLiteralType)
+ : Ctx(Ctx), ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool TraverseType(QualType QT) {
+ if (QT.isNull())
+ return true;
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType)) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseType(QT);
+ }
+
+ bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) {
+ if (const auto *Ctor = CE->getConstructor();
+ Ctor && !Ctor->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+
+ return true;
+ }
+ bool WalkUpFromCallExpr(CallExpr *CE) {
+ if (const auto *FDecl =
+ llvm::dyn_cast_if_present<FunctionDecl>(CE->getCalleeDecl());
+ FDecl && !FDecl->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ bool Possible = true;
+ };
+
+ if (FDecl->hasBody() && ConservativeLiteralType) {
+ BodyVisitor Visitor(Ctx, ConservativeLiteralType);
+ Visitor.TraverseStmt(FDecl->getBody());
+ if (!Visitor.Possible)
+ return false;
+ }
+ return true;
+}
+
+bool satisfiesProperties20(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified()) {
+ return true;
+ }
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (FDecl->hasBody() && llvm::isa<CoroutineBodyStmt>(FDecl->getBody()))
+ return false;
+
+ if ((llvm::isa<CXXConstructorDecl>(FDecl) ||
+ llvm::isa<CXXDestructorDecl>(FDecl)) &&
+ llvm::any_of(
+ Method->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) { return Base.isVirtual(); }))
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor20 : public clang::RecursiveASTVisitor<Visitor20> {
+ public:
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor20(bool ConservativeLiteralType)
+ : ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool TraverseGotoStmt(GotoStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseLabelStmt(LabelStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXTryStmt(CXXTryStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseGCCAsmStmt(GCCAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseMSAsmStmt(MSAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseDecompositionDecl(DecompositionDecl * /*DD*/) {
+ Possible = f...
[truncated]
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Julian Schmidt (5chmidti) ChangesThis check finds all functions and variables that can be declared as Fixes #115622 Patch is 95.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146553.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index fd7affd22a463..4535df3451c95 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -19,6 +19,7 @@ set_target_properties(genconfusable PROPERTIES FOLDER "Clang Tools Extra/Sourceg
add_clang_library(clangTidyMiscModule STATIC
ConstCorrectnessCheck.cpp
+ ConstexprCheck.cpp
CoroutineHostileRAIICheck.cpp
DefinitionsInHeadersCheck.cpp
ConfusableIdentifierCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp
new file mode 100644
index 0000000000000..270ac1dd0fa48
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/ConstexprCheck.cpp
@@ -0,0 +1,936 @@
+//===--- ConstexprCheck.cpp - clang-tidy ----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "ConstexprCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/OperationKinds.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/StmtCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Support/Casting.h"
+#include <cstddef>
+#include <functional>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+namespace {
+AST_MATCHER(FunctionDecl, locationPermitsConstexpr) {
+ const bool IsInMainFile =
+ Finder->getASTContext().getSourceManager().isInMainFile(
+ Node.getLocation());
+
+ if (IsInMainFile && Node.hasExternalFormalLinkage())
+ return false;
+ if (!IsInMainFile && !Node.isInlined())
+ return false;
+
+ return true;
+}
+
+AST_MATCHER(Expr, isCXX11ConstantExpr) {
+ return !Node.isValueDependent() &&
+ Node.isCXX11ConstantExpr(Finder->getASTContext());
+}
+
+AST_MATCHER(DeclaratorDecl, isInMacro) {
+ const SourceRange R =
+ SourceRange(Node.getInnerLocStart(), Node.getLocation());
+
+ return Node.getLocation().isMacroID() || Node.getEndLoc().isMacroID() ||
+ utils::rangeContainsMacroExpansion(
+ R, &Finder->getASTContext().getSourceManager()) ||
+ utils::rangeIsEntirelyWithinMacroArgument(
+ R, &Finder->getASTContext().getSourceManager());
+}
+
+AST_MATCHER(Decl, hasNoRedecl) {
+ // There is always the actual declaration
+ return !Node.redecls().empty() &&
+ std::next(Node.redecls_begin()) == Node.redecls_end();
+}
+
+AST_MATCHER(Decl, allRedeclsInSameFile) {
+ const SourceManager &SM = Finder->getASTContext().getSourceManager();
+ const SourceLocation L = Node.getLocation();
+ for (const Decl *ReDecl : Node.redecls()) {
+ if (!SM.isWrittenInSameFile(L, ReDecl->getLocation()))
+ return false;
+ }
+ return true;
+}
+
+AST_MATCHER(FunctionDecl, isConstexprSpecified) {
+ return Node.isConstexprSpecified();
+}
+
+bool satisfiesConstructorPropertiesUntil20(const CXXConstructorDecl *Ctor,
+ ASTContext &Ctx) {
+ const CXXRecordDecl *Rec = Ctor->getParent();
+ llvm::SmallPtrSet<const RecordDecl *, 8> Bases{};
+ for (const CXXBaseSpecifier Base : Rec->bases()) {
+ Bases.insert(Base.getType()->getAsRecordDecl());
+ }
+ llvm::SmallPtrSet<const FieldDecl *, 8> Fields{Rec->field_begin(),
+ Rec->field_end()};
+ llvm::SmallPtrSet<const FieldDecl *, 4> Indirects{};
+
+ for (const CXXCtorInitializer *const Init : Ctor->inits()) {
+ const Type *InitType = Init->getBaseClass();
+ if (InitType && InitType->isRecordType()) {
+ const auto *ConstructingInit =
+ llvm::dyn_cast<CXXConstructExpr>(Init->getInit());
+ if (ConstructingInit &&
+ !ConstructingInit->getConstructor()->isConstexprSpecified())
+ return false;
+ }
+
+ if (Init->isBaseInitializer()) {
+ Bases.erase(Init->getBaseClass()->getAsRecordDecl());
+ continue;
+ }
+
+ if (Init->isMemberInitializer()) {
+ const FieldDecl *Field = Init->getMember();
+
+ if (Field->isAnonymousStructOrUnion())
+ Indirects.insert(Field);
+
+ Fields.erase(Field);
+ continue;
+ }
+ }
+
+ for (const auto &Match :
+ match(cxxRecordDecl(forEach(indirectFieldDecl().bind("indirect"))), *Rec,
+ Ctx)) {
+ const auto *IField = Match.getNodeAs<IndirectFieldDecl>("indirect");
+
+ size_t NumInitializations = false;
+ for (const NamedDecl *ND : IField->chain())
+ NumInitializations += Indirects.erase(llvm::dyn_cast<FieldDecl>(ND));
+
+ if (NumInitializations != 1)
+ return false;
+
+ for (const NamedDecl *ND : IField->chain())
+ Fields.erase(llvm::dyn_cast<FieldDecl>(ND));
+ }
+
+ if (!Fields.empty())
+ return false;
+
+ return true;
+}
+
+const Type *unwrapPointee(const Type *T) {
+ if (!T->isPointerOrReferenceType())
+ return T;
+
+ while (T && T->isPointerOrReferenceType()) {
+ if (T->isReferenceType()) {
+ const QualType QType = T->getPointeeType();
+ if (!QType.isNull())
+ T = QType.getTypePtr();
+ } else
+ T = T->getPointeeOrArrayElementType();
+ }
+
+ return T;
+}
+
+bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ const bool ConservativeLiteralType);
+
+bool isLiteralType(const Type *T, const ASTContext &Ctx,
+ const bool ConservativeLiteralType) {
+ if (!T)
+ return false;
+
+ if (!T->isLiteralType(Ctx))
+ return false;
+
+ if (!ConservativeLiteralType)
+ return T->isLiteralType(Ctx) && !T->isVoidType();
+
+ if (T->isIncompleteType() || T->isIncompleteArrayType())
+ return false;
+
+ T = unwrapPointee(T);
+ if (!T)
+ return false;
+
+ assert(!T->isPointerOrReferenceType());
+
+ if (T->isIncompleteType() || T->isIncompleteArrayType())
+ return false;
+
+ if (T->isLiteralType(Ctx))
+ return true;
+
+ if (const auto *Rec = T->getAsCXXRecordDecl()) {
+ if (llvm::any_of(Rec->ctors(), [](const CXXConstructorDecl *Ctor) {
+ return !Ctor->isCopyOrMoveConstructor() &&
+ Ctor->isConstexprSpecified();
+ }))
+ return false;
+
+ for (const CXXBaseSpecifier Base : Rec->bases()) {
+ if (!isLiteralType(Base.getType(), Ctx, ConservativeLiteralType))
+ return false;
+ }
+ }
+
+ if (const Type *ArrayElementType = T->getArrayElementTypeNoTypeQual())
+ return isLiteralType(ArrayElementType, Ctx, ConservativeLiteralType);
+
+ return false;
+}
+
+bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ const bool ConservativeLiteralType) {
+ return isLiteralType(QT.getTypePtr(), Ctx, ConservativeLiteralType);
+}
+
+bool satisfiesProperties11(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified()) {
+ return true;
+ }
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (Method &&
+ (Method->isVirtual() ||
+ !match(cxxMethodDecl(hasBody(cxxTryStmt())), *Method, Ctx).empty()))
+ return false;
+
+ if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(FDecl);
+ Ctor && (!satisfiesConstructorPropertiesUntil20(Ctor, Ctx) ||
+ llvm::any_of(Ctor->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) {
+ return Base.isVirtual();
+ })))
+ return false;
+
+ if (const auto *Dtor = llvm::dyn_cast<CXXDestructorDecl>(FDecl);
+ Dtor && !Dtor->isTrivial())
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor11 : public clang::RecursiveASTVisitor<Visitor11> {
+ public:
+ using Base = clang::RecursiveASTVisitor<Visitor11>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor11(ASTContext &Ctx, bool ConservativeLiteralType)
+ : Ctx(Ctx), ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool WalkUpFromNullStmt(NullStmt *) {
+ Possible = false;
+ return true;
+ }
+ bool WalkUpFromDeclStmt(DeclStmt *DS) {
+ for (const Decl *D : DS->decls())
+ if (!llvm::isa<StaticAssertDecl, TypedefNameDecl, UsingDecl,
+ UsingDirectiveDecl>(D)) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromExpr(Expr *) { return true; }
+ bool WalkUpFromCompoundStmt(CompoundStmt *S) {
+ for (const DynTypedNode &Node : Ctx.getParents(*S))
+ if (Node.get<FunctionDecl>() != nullptr)
+ return true;
+
+ Possible = false;
+ return false;
+ }
+ bool WalkUpFromStmt(Stmt *) {
+ Possible = false;
+ return false;
+ }
+
+ bool WalkUpFromReturnStmt(ReturnStmt *) {
+ ++NumReturns;
+ if (NumReturns != 1U) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromCastExpr(CastExpr *CE) {
+ if (llvm::is_contained(
+ {
+ CK_LValueBitCast,
+ CK_IntegralToPointer,
+ CK_PointerToIntegral,
+ },
+ CE->getCastKind())) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXDynamicCastExpr(CXXDynamicCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseCXXReinterpretCastExpr(CXXReinterpretCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseType(QualType QT) {
+ if (QT.isNull())
+ return true;
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType)) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseType(QT);
+ }
+
+ bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) {
+ if (const auto *Ctor = CE->getConstructor();
+ Ctor && !Ctor->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+
+ return true;
+ }
+ bool WalkUpFromCallExpr(CallExpr *CE) {
+ if (const auto *FDecl =
+ llvm::dyn_cast_if_present<FunctionDecl>(CE->getCalleeDecl());
+ FDecl && !FDecl->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ bool Possible = true;
+ size_t NumReturns = 0;
+ };
+
+ Visitor11 V{Ctx, ConservativeLiteralType};
+ V.TraverseDecl(const_cast<FunctionDecl *>(FDecl));
+ if (!V.Possible)
+ return false;
+
+ return true;
+}
+
+// The only difference between C++14 and C++17 is that `constexpr` lambdas
+// can be used in C++17.
+bool satisfiesProperties1417(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified())
+ return true;
+
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (Method && Method->isVirtual())
+ return false;
+
+ if (llvm::isa<CXXConstructorDecl>(FDecl) &&
+ llvm::any_of(
+ Method->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) { return Base.isVirtual(); }))
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor14 : public clang::RecursiveASTVisitor<Visitor14> {
+ public:
+ using Base = clang::RecursiveASTVisitor<Visitor14>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor14(bool CXX17, ASTContext &Ctx, bool ConservativeLiteralType,
+ bool AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ : CXX17(CXX17), Ctx(Ctx),
+ ConservativeLiteralType(ConservativeLiteralType),
+ AddConstexprToMethodOfClassWithoutConstexprConstructor(
+ AddConstexprToMethodOfClassWithoutConstexprConstructor) {}
+
+ bool TraverseGotoStmt(GotoStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseLabelStmt(LabelStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXTryStmt(CXXTryStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseGCCAsmStmt(GCCAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseMSAsmStmt(MSAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseDecompositionDecl(DecompositionDecl * /*DD*/) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseVarDecl(VarDecl *VD) {
+ const auto StorageDur = VD->getStorageDuration();
+ Possible = VD->hasInit() &&
+ isLiteralType(VD->getType(), VD->getASTContext(),
+ ConservativeLiteralType) &&
+ (StorageDur != StorageDuration::SD_Static &&
+ StorageDur != StorageDuration::SD_Thread);
+ return Possible && Base::TraverseVarDecl(VD);
+ }
+ bool TraverseLambdaExpr(LambdaExpr *LE) {
+ if (CXX17) {
+ Possible = satisfiesProperties1417(
+ LE->getCallOperator(), Ctx, ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor);
+ return Possible;
+ }
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseDeclRefExpr(DeclRefExpr *DRef) {
+ if (const auto *D = llvm::dyn_cast_if_present<VarDecl>(DRef->getDecl());
+ D && !D->isLocalVarDeclOrParm() && D->hasGlobalStorage()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool WalkUpFromCastExpr(CastExpr *CE) {
+ if (llvm::is_contained(
+ {
+ CK_LValueBitCast,
+ CK_IntegralToPointer,
+ CK_PointerToIntegral,
+ },
+ CE->getCastKind())) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXDynamicCastExpr(CXXDynamicCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ bool TraverseCXXReinterpretCastExpr(CXXReinterpretCastExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ const bool CXX17;
+ bool Possible = true;
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor;
+ };
+
+ Visitor14 V{Ctx.getLangOpts().CPlusPlus17 != 0, Ctx, ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor};
+ V.TraverseDecl(const_cast<FunctionDecl *>(FDecl));
+ if (!V.Possible)
+ return false;
+
+ if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(FDecl);
+ Ctor && !satisfiesConstructorPropertiesUntil20(Ctor, Ctx))
+ return false;
+
+ if (const auto *Dtor = llvm::dyn_cast<CXXDestructorDecl>(FDecl);
+ Dtor && !Dtor->isTrivial())
+ return false;
+
+ class BodyVisitor : public clang::RecursiveASTVisitor<BodyVisitor> {
+ public:
+ using Base = clang::RecursiveASTVisitor<BodyVisitor>;
+ bool shouldVisitImplicitCode() const { return true; }
+
+ explicit BodyVisitor(ASTContext &Ctx, bool ConservativeLiteralType)
+ : Ctx(Ctx), ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool TraverseType(QualType QT) {
+ if (QT.isNull())
+ return true;
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType)) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseType(QT);
+ }
+
+ bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) {
+ if (const auto *Ctor = CE->getConstructor();
+ Ctor && !Ctor->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+
+ return true;
+ }
+ bool WalkUpFromCallExpr(CallExpr *CE) {
+ if (const auto *FDecl =
+ llvm::dyn_cast_if_present<FunctionDecl>(CE->getCalleeDecl());
+ FDecl && !FDecl->isConstexprSpecified()) {
+ Possible = false;
+ return false;
+ }
+ return true;
+ }
+
+ bool TraverseCXXNewExpr(CXXNewExpr *) {
+ Possible = false;
+ return false;
+ }
+
+ ASTContext &Ctx;
+ const bool ConservativeLiteralType;
+ bool Possible = true;
+ };
+
+ if (FDecl->hasBody() && ConservativeLiteralType) {
+ BodyVisitor Visitor(Ctx, ConservativeLiteralType);
+ Visitor.TraverseStmt(FDecl->getBody());
+ if (!Visitor.Possible)
+ return false;
+ }
+ return true;
+}
+
+bool satisfiesProperties20(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified()) {
+ return true;
+ }
+ const LangOptions LO = Ctx.getLangOpts();
+ const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl);
+ if (Method && !Method->isStatic() &&
+ !Method->getParent()->hasConstexprNonCopyMoveConstructor() &&
+ !AddConstexprToMethodOfClassWithoutConstexprConstructor)
+ return false;
+
+ if (FDecl->hasBody() && llvm::isa<CoroutineBodyStmt>(FDecl->getBody()))
+ return false;
+
+ if ((llvm::isa<CXXConstructorDecl>(FDecl) ||
+ llvm::isa<CXXDestructorDecl>(FDecl)) &&
+ llvm::any_of(
+ Method->getParent()->bases(),
+ [](const CXXBaseSpecifier &Base) { return Base.isVirtual(); }))
+ return false;
+
+ if (!isLiteralType(FDecl->getReturnType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ for (const ParmVarDecl *Param : FDecl->parameters())
+ if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType))
+ return false;
+
+ class Visitor20 : public clang::RecursiveASTVisitor<Visitor20> {
+ public:
+ bool shouldVisitImplicitCode() const { return true; }
+
+ Visitor20(bool ConservativeLiteralType)
+ : ConservativeLiteralType(ConservativeLiteralType) {}
+
+ bool TraverseGotoStmt(GotoStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseLabelStmt(LabelStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseCXXTryStmt(CXXTryStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseGCCAsmStmt(GCCAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseMSAsmStmt(MSAsmStmt *) {
+ Possible = false;
+ return false;
+ }
+ bool TraverseDecompositionDecl(DecompositionDecl * /*DD*/) {
+ Possible = f...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
5chmidti
force-pushed
the
clang_tidy_constexpr
branch
from
35a081f to
2a50c3e
Compare
|
misc-use-constexpr? :) |
|
I ran this check on all of Some perf info from running the checks enabled in LLVM + this check: Running on: Running on |
modernize-use-constexpr? |
5chmidti
force-pushed
the
clang_tidy_constexpr
branch
from
2a50c3e to
8d35424
Compare
This check finds all functions and variables that can be declared as `constexpr`, using the specified standard version to check if the requirements are met. Fixes llvm#115622
5chmidti
force-pushed
the
clang_tidy_constexpr
branch
from
8d35424 to
16d5a3f
Compare
Hm. It is indeed also a modernization... +- on which to pick. Let's see what others think.
while the misc category has constexpr int div(int a, int b) { return a / b; }
int main() {
constexpr auto res = div(10,0);
return 0;
}, or potential performance improvements. |
vbvictor
reviewed
Jul 1, 2025
|
|
denzor200
reviewed
Jul 1, 2025
| // CHECK-MESSAGES-11: :[[@LINE-1]]:13: warning: function 'g1' can be declared 'constexpr' [misc-use-constexpr] | ||
| // CHECK-FIXES-11: constexpr int g1() { return 0; } | ||
| static constexpr int A1 = 0; | ||
| static int B1 = 0; |
There was a problem hiding this comment.
Just asking - this check will never change it to static constexpr int B1 = 0;, is that intended?
There was a problem hiding this comment.
I've based the check for variables on them being const as a prerequisite. Otherwise, I'd have to pull in the const-correctness logic on top of this check and that would essentially duplicate the runtime cost of the const-correctness check.
There was a problem hiding this comment.
As I understand, this is a job for "misc-const-correctness", and then this check should be runned.
Thus we should write in the doc a recomendation to run this check only after "misc-const-correctness" have done.
There was a problem hiding this comment.
I will extend the check docs to explain the dependency on const for variables.
|
I'm +1 on the |
vbvictor
reviewed
Jul 1, 2025
|
I'm ok with modernize-use-constexpr as well! |
5chmidti
changed the title
|
I tried with ITK and the resulting transformation did not compile. Many changes were of this form: - const double Max = 1.0 - Min;
+ constexpr double Max = 1.0 - Min;Which is great, though notice the double space after Other changes were like this: - const auto check = [](const auto & ptr) { EXPECT_THROW(itk::Deref(ptr), itk::DerefError); };
+ constexpr const auto check = [](auto & ptr) { EXPECT_THROW(itk::Deref(ptr), itk::DerefError); };I'm no C++ expert, but is it right to have both Also, I was surprised to see the 2nd const removed. And this removal generates one of the many compiler errors. Still, this is looking like it'll be great! |
|
I got some fixits that do not compile, e.g.: is changed into |
The reason is that when inserting, I add a
Eh... yeah. The range I passed to the
Thanks, and thank you very much for trying it out. My current setup is limited in what I can test outside the LLVM codebase. |
|
Thanks for testing this on your codebase so fast as well.
I've tried to reproduce this but was unable to, could you please provide a small reproducer? #include <vector>
int main() {
std::vector<int> Numbers = {
0, 1, 2, 3, 4, 5, 6, 7,
};
const auto count = std::erase_if(Numbers, [](int N) { return N % 2 == 0; });
} |
|
Sorry for the silly mistakes, btw. I've updated the docs, added replacement string options, and fixed the |
With 127d7f0 I can reporoduce the wrong fixit like this: test.cpp #include <vector>
template <typename T> void func() {
std::vector<int> Numbers = {0, 1};
const auto count =
std::erase_if(Numbers, [](int N) { return N % 2 == 0; });
}commands clang-tidy -checks=-*,*constexpr* -export-fixes fixes.yaml test.cpp -- --std=c++23
clang-apply-replacements .result #include <vector>
template <typename T> void func() {
std::vector<int> Numbers = {0, 1};
const auto count =
std::erase_if(Numbers, [](int Nconstexpr ) { return N % 2 == 0; });
} |
| if (T->isLiteralType(Ctx)) | ||
| return true; | ||
|
|
||
| if (const auto *Rec = T->getAsCXXRecordDecl()) { |
There was a problem hiding this comment.
auto should not be used - type is not spelled explicitly.
| } | ||
|
|
||
| bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) { | ||
| if (const auto *Ctor = CE->getConstructor(); |
| return true; | ||
|
|
||
| const LangOptions LO = Ctx.getLangOpts(); | ||
| const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl); |
There was a problem hiding this comment.
Suggested change
| const CXXMethodDecl *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl); | |
| const auto *Method = llvm::dyn_cast<CXXMethodDecl>(FDecl); |
| return false; | ||
| } | ||
| bool TraverseVarDecl(VarDecl *VD) { | ||
| const auto StorageDur = VD->getStorageDuration(); |
There was a problem hiding this comment.
auto should not be used.
| } | ||
|
|
||
| bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) { | ||
| if (const auto *Ctor = CE->getConstructor(); |
Comment on lines
+145
to
+147
| - New :doc:`modernize-use-constexpr | ||
| <clang-tidy/checks/modernize/use-constexpr>` check that finds functions and | ||
| variables that can be declared `constexpr`. |
There was a problem hiding this comment.
Suggested change
| - New :doc:`modernize-use-constexpr | |
| <clang-tidy/checks/modernize/use-constexpr>` check that finds functions and | |
| variables that can be declared `constexpr`. | |
| - New :doc:`modernize-use-constexpr | |
| <clang-tidy/checks/modernize/use-constexpr>` check. | |
| Finds functions and variables that can be declared ``constexpr``. |
| modernize-use-constexpr | ||
| ======================= | ||
|
|
||
| Finds functions and variables that can be declared 'constexpr'. |
There was a problem hiding this comment.
Suggested change
| Finds functions and variables that can be declared 'constexpr'. | |
| Finds functions and variables that can be declared '`constexpr'`. |
| While a function of a class or struct could be declared ``constexpr``, when | ||
| the class itself can never be constructed at compile-time, then adding | ||
| ``constexpr`` to a member function is superfluous. This option controls if | ||
| ``constexpr`` should be added anyways. Default is ``false``. |
There was a problem hiding this comment.
Suggested change
| ``constexpr`` should be added anyways. Default is ``false``. | |
| ``constexpr`` should be added anyways. Default is `false`. |
| .. option:: ConstexprString | ||
|
|
||
| The string to use to specify a variable or function as ``constexpr``, for | ||
| example, a macro. Default is ``constexpr``. |
There was a problem hiding this comment.
Suggested change
| example, a macro. Default is ``constexpr``. | |
| example, a macro. Default is `constexpr`. |
| .. option:: ConstexprString | ||
|
|
||
| The string to use with C++23 to specify a function-local variable as | ||
| ``static constexpr``, for example, a macro. Default is ``static constexpr`` |
There was a problem hiding this comment.
Suggested change
| ``static constexpr``, for example, a macro. Default is ``static constexpr`` | |
| ``static constexpr``, for example, a macro. Default is `static constexpr` |
|
Another compiler error after transformation: It made this constexpr: PUGI_IMPL_FN constexpr bool is_nan(double value)
{
#if defined(PUGI_IMPL_MSVC_CRT_VERSION) || defined(__BORLANDC__)
return !!_isnan(value);
#elif defined(fpclassify) && defined(FP_NAN)
return fpclassify(value) == FP_NAN;
#else
// fallback
const volatile double v = value;
return v != v;
#endif
}But then: |
|
Another interesting case, it seems somehow multiple constexprs got added, ex: template <typename CellType>
constexpr constexpr constexpr constexpr constexpr constexpr constexpr constexpr int numberOfSidesOfDimension(int dimension);This actually compiles, though with a |
I'll take a look |
I failed to reproduce this with template <typename T>
static T forwardDeclared();
template <typename T>
static T forwardDeclared() { return T{}; }
static void useForwardDeclared() {
forwardDeclared<int>() + forwardDeclared<double>() + forwardDeclared<char>();
} template <typename T>
-static T forwardDeclared();
+constexpr static T forwardDeclared();
template <typename T>
-static T forwardDeclared() { return T{}; }
+constexpr static T forwardDeclared() { return T{}; }
static void useForwardDeclared() {
forwardDeclared<int>() + forwardDeclared<double>() + forwardDeclared<char>(); I had seen this one as well, but it was already fixed in the initial commit, at least I thought. |
I don't have a minimal repro case, I was building VTK. It's pretty simple to build it yourself if you're familiar with CMake. I could give you exact command line steps if you need... |
|
I've tried another project: opencv and the result also fails to compile. One case I think is because two variables are declared together. i.e.: instead of: Concretely: const char * const borderMap[] = { "BORDER_CONSTANT", "BORDER_REPLICATE", "BORDER_REFLECT", "BORDER_WRAP", "BORDER_REFLECT_101" },
* const btype = borderMap[borderType & ~BORDER_ISOLATED];Got changed to: which is fine for Resulting in: |
|
Another interesting case: static int testToWindowsExtendedPath()
{
#ifdef _WIN32
// lots of real, non constant work
return ret;
#else
return 0;
#endif
}For some platforms this can be constexpr; for other platforms (Windows), it cannot. It was transformed to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This check finds all functions and variables that can be declared as
constexpr, using the specified standard version to check if therequirements are met.
Fixes #115622