[clang-tidy] add modernize-use-constexpr check #162741
Open
+1,184
−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
5chmidti
requested review from
EugeneZelenko,
HerrCai0907,
PiotrZSL,
carlosgalvezp and
vbvictor
Member
|
@llvm/pr-subscribers-clang-tidy Author: Julian Schmidt (5chmidti) ChangesThis check finds all functions and variables that can be declared as This is part 1/N, adding only the C++11 rule-set. Fixes #115622 Patch is 51.72 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/162741.diff 13 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index 882f2dc9fb4d8..f7dbc1d7312ee 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -32,6 +32,7 @@ add_clang_library(clangTidyModernizeModule STATIC
UnaryStaticAssertCheck.cpp
UseAutoCheck.cpp
UseBoolLiteralsCheck.cpp
+ UseConstexprCheck.cpp
UseConstraintsCheck.cpp
UseDefaultMemberInitCheck.cpp
UseDesignatedInitializersCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index 360e2b8434d0c..1b54adeef9281 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -33,6 +33,7 @@
#include "UnaryStaticAssertCheck.h"
#include "UseAutoCheck.h"
#include "UseBoolLiteralsCheck.h"
+#include "UseConstexprCheck.h"
#include "UseConstraintsCheck.h"
#include "UseDefaultMemberInitCheck.h"
#include "UseDesignatedInitializersCheck.h"
@@ -82,6 +83,7 @@ class ModernizeModule : public ClangTidyModule {
CheckFactories.registerCheck<MinMaxUseInitializerListCheck>(
"modernize-min-max-use-initializer-list");
CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
+ CheckFactories.registerCheck<UseConstexprCheck>("modernize-use-constexpr");
CheckFactories.registerCheck<UseDesignatedInitializersCheck>(
"modernize-use-designated-initializers");
CheckFactories.registerCheck<UseIntegerSignComparisonCheck>(
diff --git a/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.cpp
new file mode 100644
index 0000000000000..9238605b3cb4a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.cpp
@@ -0,0 +1,517 @@
+//===--- UseConstexprCheck.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 "UseConstexprCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+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.getTypeSpecStartLoc(), 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;
+}
+} // namespace
+
+static 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;
+}
+
+static bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ bool ConservativeLiteralType);
+
+static 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 = utils::unwrapPointee(T);
+ if (!T)
+ return false;
+
+ assert(!T->isPointerOrReferenceType());
+
+ if (T->isIncompleteType() || T->isIncompleteArrayType())
+ return false;
+
+ if (T->isLiteralType(Ctx))
+ return true;
+
+ if (const CXXRecordDecl *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;
+}
+
+static bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ const bool ConservativeLiteralType) {
+ return !QT.isVolatileQualified() &&
+ isLiteralType(QT.getTypePtr(), Ctx, ConservativeLiteralType);
+}
+
+static bool satisfiesProperties11(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified())
+ return true;
+
+ const LangOptions LO = Ctx.getLangOpts();
+ const auto *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 false;
+ }
+ 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, const bool TraverseQualifier = true) {
+ if (QT.isNull())
+ return true;
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType)) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseType(QT, TraverseQualifier);
+ }
+
+ bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) {
+ if (const CXXConstructorDecl *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;
+ }
+
+ bool TraverseDeclRefExpr(DeclRefExpr *DRef) {
+ if (DRef->getType().isVolatileQualified()) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseDeclRefExpr(DRef);
+ }
+
+ 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;
+}
+
+namespace {
+AST_MATCHER_P2(FunctionDecl, satisfiesProperties, bool, ConservativeLiteralType,
+ bool, AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ ASTContext &Ctx = Finder->getASTContext();
+ const LangOptions LO = Ctx.getLangOpts();
+
+ if (LO.CPlusPlus11)
+ return satisfiesProperties11(
+ &Node, Ctx, ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor);
+
+ return false;
+}
+
+AST_MATCHER_P(VarDecl, satisfiesVariableProperties, bool,
+ ConservativeLiteralType) {
+ ASTContext &Ctx = Finder->getASTContext();
+
+ const QualType QT = Node.getType();
+ const Type *T = QT.getTypePtr();
+ if (!T)
+ return false;
+
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType))
+ return false;
+
+ const bool IsDeclaredInsideConstexprFunction = std::invoke([&Node]() {
+ const auto *Func = llvm::dyn_cast<FunctionDecl>(Node.getDeclContext());
+ if (!Func)
+ return false;
+ return Func->isConstexpr();
+ });
+
+ if (Node.isStaticLocal() && IsDeclaredInsideConstexprFunction)
+ return false;
+
+ if (!Ctx.getLangOpts().CPlusPlus20)
+ return true;
+
+ const CXXRecordDecl *RDecl = T->getAsCXXRecordDecl();
+ const Type *const ArrayOrPtrElement = T->getPointeeOrArrayElementType();
+ if (ArrayOrPtrElement)
+ RDecl = ArrayOrPtrElement->getAsCXXRecordDecl();
+
+ if (RDecl && (!RDecl->hasDefinition() || !RDecl->hasConstexprDestructor()))
+ return false;
+
+ return true;
+}
+} // namespace
+
+void UseConstexprCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ functionDecl(
+ isDefinition(),
+ unless(anyOf(isConstexpr(), isImplicit(), hasExternalFormalLinkage(),
+ isInMacro(), isMain(), isInStdNamespace(),
+ isExpansionInSystemHeader(), isExternC(),
+ cxxMethodDecl(ofClass(cxxRecordDecl(isLambda()))))),
+ locationPermitsConstexpr(), allRedeclsInSameFile(),
+ satisfiesProperties(
+ ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor))
+ .bind("func"),
+ this);
+
+ const auto CallToNonConstexprFunction =
+ callExpr(callee(functionDecl(unless(isConstexpr()))));
+
+ const auto VarSupportingConstexpr =
+ varDecl(
+ unless(anyOf(parmVarDecl(), isImplicit(), isInStdNamespace(),
+ isExpansionInSystemHeader(), isConstexpr(), isExternC(),
+ hasExternalFormalLinkage(), isInMacro())),
+ hasNoRedecl(), hasType(qualType(isConstQualified())),
+ satisfiesVariableProperties(ConservativeLiteralType),
+ hasInitializer(
+ expr(isCXX11ConstantExpr(), unless(CallToNonConstexprFunction),
+ unless(hasDescendant(CallToNonConstexprFunction)))))
+ .bind("var");
+ Finder->addMatcher(mapAnyOf(translationUnitDecl, namespaceDecl)
+ .with(forEach(VarSupportingConstexpr)),
+ this);
+ Finder->addMatcher(declStmt(hasSingleDecl(VarSupportingConstexpr)), this);
+}
+
+static const FunctionDecl *
+maybeResolveToTemplateDecl(const FunctionDecl *Func) {
+ if (Func && Func->isTemplateInstantiation())
+ Func = Func->getTemplateInstantiationPattern();
+ return Func;
+}
+
+void UseConstexprCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func")) {
+ Func = maybeResolveToTemplateDecl(Func);
+ if (Func)
+ Functions.insert(Func);
+ return;
+ }
+
+ if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var")) {
+ if (const VarDecl *VarTemplate = Var->getTemplateInstantiationPattern())
+ Var = VarTemplate;
+
+ Variables.insert(Var);
+ return;
+ }
+}
+
+void UseConstexprCheck::onEndOfTranslationUnit() {
+ const std::string FunctionReplacement = ConstexprString + " ";
+
+ for (const FunctionDecl *Func : Functions) {
+ const SourceRange R =
+ SourceRange(Func->getTypeSpecStartLoc(), Func->getLocation());
+ auto Diag = diag(Func->getLocation(), "declare function %0 as 'constexpr'")
+ << Func << R;
+
+ for (const Decl *D : Func->redecls())
+ if (const auto *FDecl = llvm::dyn_cast<FunctionDecl>(D))
+ Diag << FixItHint::CreateInsertion(FDecl->getTypeSpecStartLoc(),
+ FunctionReplacement);
+ }
+
+ const auto MaybeRemoveConst = [&, this](DiagnosticBuilder &Diag,
+ const VarDecl *Var) {
+ // Since either of the locs can be in a macro, use `makeFileCharRange` to be
+ // sure that we have a consistent `CharSourceRange`, located entirely in the
+ // source file.
+ const CharSourceRange FileRange = Lexer::makeFileCharRange(
+ CharSourceRange::getCharRange(Var->getInnerLocStart(),
+ Var->getLocation()),
+ Var->getASTContext().getSourceManager(), getLangOpts());
+ if (const std::optional<Token> ConstToken =
+ utils::lexer::getQualifyingToken(
+ tok::TokenKind::kw_const, FileRange, Var->getASTContext(),
+ Var->getASTContext().getSourceManager())) {
+ Diag << FixItHint::CreateRemoval(ConstToken->getLocation());
+ }
+ };
+
+ for (const auto *Var : Variables) {
+ const SourceRange R =
+ SourceRange(Var->getTypeSpecStartLoc(), Var->getLocation());
+ auto Diag = diag(Var->getLocation(), "declare variable %0 as 'constexpr'")
+ << Var << R
+ << FixItHint::CreateInsertion(Var->getTypeSpecStartLoc(),
+ FunctionReplacement);
+ MaybeRemoveConst(Diag, Var);
+ }
+
+ Functions.clear();
+ Variables.clear();
+}
+
+UseConstexprCheck::UseConstexprCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ ConservativeLiteralType(Options.get("ConservativeLiteralType", true)),
+ AddConstexprToMethodOfClassWithoutConstexprConstructor(Options.get(
+ "AddConstexprToMethodOfClassWithoutConstexprConstructor", false)),
+ ConstexprString(Options.get("ConstexprString", "constexpr")),
+ StaticConstexprString(
+ Options.get("StaticConstexprString", "static " + ConstexprString)) {}
+
+void UseConstexprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "ConservativeLiteralType", ConservativeLiteralType);
+ Options.store(Opts, "AddConstexprToMethodOfClassWithoutConstexprConstructor",
+ AddConstexprToMethodOfClassWithoutConstexprConstructor);
+ Options.store(Opts, "ConstexprString", ConstexprString);
+ Options.store(Opts, "StaticConstexprString", StaticConstexprString);
+}
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.h b/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.h
new file mode 100644
index 0000000000000..bfb206d40ba47
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.h
@@ -0,0 +1,45 @@
+//===--- UseUseConstexprCheck.h - clang-tidy --------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USECONSTEXPRCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USECONSTEXPRCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/AST/Decl.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
+namespace clang::tidy::modernize {
+
+/// Finds functions and variables that can be declared 'constexpr'.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-...
[truncated]
|
Member
|
@llvm/pr-subscribers-clang-tools-extra Author: Julian Schmidt (5chmidti) ChangesThis check finds all functions and variables that can be declared as This is part 1/N, adding only the C++11 rule-set. Fixes #115622 Patch is 51.72 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/162741.diff 13 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index 882f2dc9fb4d8..f7dbc1d7312ee 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -32,6 +32,7 @@ add_clang_library(clangTidyModernizeModule STATIC
UnaryStaticAssertCheck.cpp
UseAutoCheck.cpp
UseBoolLiteralsCheck.cpp
+ UseConstexprCheck.cpp
UseConstraintsCheck.cpp
UseDefaultMemberInitCheck.cpp
UseDesignatedInitializersCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index 360e2b8434d0c..1b54adeef9281 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -33,6 +33,7 @@
#include "UnaryStaticAssertCheck.h"
#include "UseAutoCheck.h"
#include "UseBoolLiteralsCheck.h"
+#include "UseConstexprCheck.h"
#include "UseConstraintsCheck.h"
#include "UseDefaultMemberInitCheck.h"
#include "UseDesignatedInitializersCheck.h"
@@ -82,6 +83,7 @@ class ModernizeModule : public ClangTidyModule {
CheckFactories.registerCheck<MinMaxUseInitializerListCheck>(
"modernize-min-max-use-initializer-list");
CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
+ CheckFactories.registerCheck<UseConstexprCheck>("modernize-use-constexpr");
CheckFactories.registerCheck<UseDesignatedInitializersCheck>(
"modernize-use-designated-initializers");
CheckFactories.registerCheck<UseIntegerSignComparisonCheck>(
diff --git a/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.cpp
new file mode 100644
index 0000000000000..9238605b3cb4a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.cpp
@@ -0,0 +1,517 @@
+//===--- UseConstexprCheck.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 "UseConstexprCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+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.getTypeSpecStartLoc(), 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;
+}
+} // namespace
+
+static 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;
+}
+
+static bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ bool ConservativeLiteralType);
+
+static 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 = utils::unwrapPointee(T);
+ if (!T)
+ return false;
+
+ assert(!T->isPointerOrReferenceType());
+
+ if (T->isIncompleteType() || T->isIncompleteArrayType())
+ return false;
+
+ if (T->isLiteralType(Ctx))
+ return true;
+
+ if (const CXXRecordDecl *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;
+}
+
+static bool isLiteralType(QualType QT, const ASTContext &Ctx,
+ const bool ConservativeLiteralType) {
+ return !QT.isVolatileQualified() &&
+ isLiteralType(QT.getTypePtr(), Ctx, ConservativeLiteralType);
+}
+
+static bool satisfiesProperties11(
+ const FunctionDecl *FDecl, ASTContext &Ctx,
+ const bool ConservativeLiteralType,
+ const bool AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ if (FDecl->isConstexprSpecified())
+ return true;
+
+ const LangOptions LO = Ctx.getLangOpts();
+ const auto *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 false;
+ }
+ 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, const bool TraverseQualifier = true) {
+ if (QT.isNull())
+ return true;
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType)) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseType(QT, TraverseQualifier);
+ }
+
+ bool WalkUpFromCXXConstructExpr(CXXConstructExpr *CE) {
+ if (const CXXConstructorDecl *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;
+ }
+
+ bool TraverseDeclRefExpr(DeclRefExpr *DRef) {
+ if (DRef->getType().isVolatileQualified()) {
+ Possible = false;
+ return false;
+ }
+ return Base::TraverseDeclRefExpr(DRef);
+ }
+
+ 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;
+}
+
+namespace {
+AST_MATCHER_P2(FunctionDecl, satisfiesProperties, bool, ConservativeLiteralType,
+ bool, AddConstexprToMethodOfClassWithoutConstexprConstructor) {
+ ASTContext &Ctx = Finder->getASTContext();
+ const LangOptions LO = Ctx.getLangOpts();
+
+ if (LO.CPlusPlus11)
+ return satisfiesProperties11(
+ &Node, Ctx, ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor);
+
+ return false;
+}
+
+AST_MATCHER_P(VarDecl, satisfiesVariableProperties, bool,
+ ConservativeLiteralType) {
+ ASTContext &Ctx = Finder->getASTContext();
+
+ const QualType QT = Node.getType();
+ const Type *T = QT.getTypePtr();
+ if (!T)
+ return false;
+
+ if (!isLiteralType(QT, Ctx, ConservativeLiteralType))
+ return false;
+
+ const bool IsDeclaredInsideConstexprFunction = std::invoke([&Node]() {
+ const auto *Func = llvm::dyn_cast<FunctionDecl>(Node.getDeclContext());
+ if (!Func)
+ return false;
+ return Func->isConstexpr();
+ });
+
+ if (Node.isStaticLocal() && IsDeclaredInsideConstexprFunction)
+ return false;
+
+ if (!Ctx.getLangOpts().CPlusPlus20)
+ return true;
+
+ const CXXRecordDecl *RDecl = T->getAsCXXRecordDecl();
+ const Type *const ArrayOrPtrElement = T->getPointeeOrArrayElementType();
+ if (ArrayOrPtrElement)
+ RDecl = ArrayOrPtrElement->getAsCXXRecordDecl();
+
+ if (RDecl && (!RDecl->hasDefinition() || !RDecl->hasConstexprDestructor()))
+ return false;
+
+ return true;
+}
+} // namespace
+
+void UseConstexprCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ functionDecl(
+ isDefinition(),
+ unless(anyOf(isConstexpr(), isImplicit(), hasExternalFormalLinkage(),
+ isInMacro(), isMain(), isInStdNamespace(),
+ isExpansionInSystemHeader(), isExternC(),
+ cxxMethodDecl(ofClass(cxxRecordDecl(isLambda()))))),
+ locationPermitsConstexpr(), allRedeclsInSameFile(),
+ satisfiesProperties(
+ ConservativeLiteralType,
+ AddConstexprToMethodOfClassWithoutConstexprConstructor))
+ .bind("func"),
+ this);
+
+ const auto CallToNonConstexprFunction =
+ callExpr(callee(functionDecl(unless(isConstexpr()))));
+
+ const auto VarSupportingConstexpr =
+ varDecl(
+ unless(anyOf(parmVarDecl(), isImplicit(), isInStdNamespace(),
+ isExpansionInSystemHeader(), isConstexpr(), isExternC(),
+ hasExternalFormalLinkage(), isInMacro())),
+ hasNoRedecl(), hasType(qualType(isConstQualified())),
+ satisfiesVariableProperties(ConservativeLiteralType),
+ hasInitializer(
+ expr(isCXX11ConstantExpr(), unless(CallToNonConstexprFunction),
+ unless(hasDescendant(CallToNonConstexprFunction)))))
+ .bind("var");
+ Finder->addMatcher(mapAnyOf(translationUnitDecl, namespaceDecl)
+ .with(forEach(VarSupportingConstexpr)),
+ this);
+ Finder->addMatcher(declStmt(hasSingleDecl(VarSupportingConstexpr)), this);
+}
+
+static const FunctionDecl *
+maybeResolveToTemplateDecl(const FunctionDecl *Func) {
+ if (Func && Func->isTemplateInstantiation())
+ Func = Func->getTemplateInstantiationPattern();
+ return Func;
+}
+
+void UseConstexprCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func")) {
+ Func = maybeResolveToTemplateDecl(Func);
+ if (Func)
+ Functions.insert(Func);
+ return;
+ }
+
+ if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var")) {
+ if (const VarDecl *VarTemplate = Var->getTemplateInstantiationPattern())
+ Var = VarTemplate;
+
+ Variables.insert(Var);
+ return;
+ }
+}
+
+void UseConstexprCheck::onEndOfTranslationUnit() {
+ const std::string FunctionReplacement = ConstexprString + " ";
+
+ for (const FunctionDecl *Func : Functions) {
+ const SourceRange R =
+ SourceRange(Func->getTypeSpecStartLoc(), Func->getLocation());
+ auto Diag = diag(Func->getLocation(), "declare function %0 as 'constexpr'")
+ << Func << R;
+
+ for (const Decl *D : Func->redecls())
+ if (const auto *FDecl = llvm::dyn_cast<FunctionDecl>(D))
+ Diag << FixItHint::CreateInsertion(FDecl->getTypeSpecStartLoc(),
+ FunctionReplacement);
+ }
+
+ const auto MaybeRemoveConst = [&, this](DiagnosticBuilder &Diag,
+ const VarDecl *Var) {
+ // Since either of the locs can be in a macro, use `makeFileCharRange` to be
+ // sure that we have a consistent `CharSourceRange`, located entirely in the
+ // source file.
+ const CharSourceRange FileRange = Lexer::makeFileCharRange(
+ CharSourceRange::getCharRange(Var->getInnerLocStart(),
+ Var->getLocation()),
+ Var->getASTContext().getSourceManager(), getLangOpts());
+ if (const std::optional<Token> ConstToken =
+ utils::lexer::getQualifyingToken(
+ tok::TokenKind::kw_const, FileRange, Var->getASTContext(),
+ Var->getASTContext().getSourceManager())) {
+ Diag << FixItHint::CreateRemoval(ConstToken->getLocation());
+ }
+ };
+
+ for (const auto *Var : Variables) {
+ const SourceRange R =
+ SourceRange(Var->getTypeSpecStartLoc(), Var->getLocation());
+ auto Diag = diag(Var->getLocation(), "declare variable %0 as 'constexpr'")
+ << Var << R
+ << FixItHint::CreateInsertion(Var->getTypeSpecStartLoc(),
+ FunctionReplacement);
+ MaybeRemoveConst(Diag, Var);
+ }
+
+ Functions.clear();
+ Variables.clear();
+}
+
+UseConstexprCheck::UseConstexprCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ ConservativeLiteralType(Options.get("ConservativeLiteralType", true)),
+ AddConstexprToMethodOfClassWithoutConstexprConstructor(Options.get(
+ "AddConstexprToMethodOfClassWithoutConstexprConstructor", false)),
+ ConstexprString(Options.get("ConstexprString", "constexpr")),
+ StaticConstexprString(
+ Options.get("StaticConstexprString", "static " + ConstexprString)) {}
+
+void UseConstexprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "ConservativeLiteralType", ConservativeLiteralType);
+ Options.store(Opts, "AddConstexprToMethodOfClassWithoutConstexprConstructor",
+ AddConstexprToMethodOfClassWithoutConstexprConstructor);
+ Options.store(Opts, "ConstexprString", ConstexprString);
+ Options.store(Opts, "StaticConstexprString", StaticConstexprString);
+}
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.h b/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.h
new file mode 100644
index 0000000000000..bfb206d40ba47
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.h
@@ -0,0 +1,45 @@
+//===--- UseUseConstexprCheck.h - clang-tidy --------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USECONSTEXPRCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USECONSTEXPRCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/AST/Decl.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
+namespace clang::tidy::modernize {
+
+/// Finds functions and variables that can be declared 'constexpr'.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-...
[truncated]
|
Contributor
Author
|
One issue that I have found when I ran this on VTK: template <typename T>
- int foo();
+ constexpr constexpr int foo();
template <>
- int foo<int>() { return 10; }
+ constexpr int foo<int>() { return 10; }
template <>
- int foo<unsigned int>() { return 0; }
+ constexpr int foo<unsigned int>() { return 0; }
|
clang-tools-extra/docs/clang-tidy/checks/modernize/use-constexpr.rst
Outdated
Show resolved
Hide resolved
5chmidti
changed the title
zwuis
reviewed
Oct 10, 2025
Contributor
zwuis
left a comment
zwuis
left a comment
There was a problem hiding this comment.
Thank you for your hard work!
clang-tools-extra/docs/clang-tidy/checks/modernize/use-constexpr.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/modernize/use-constexpr.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/modernize/use-constexpr.rst
Outdated
Show resolved
Hide resolved
HerrCai0907
reviewed
Oct 11, 2025
Comment on lines
+45
to
+49
| struct NonLiteral{ | ||
| NonLiteral(); | ||
| ~NonLiteral(); | ||
| int &ref; | ||
| }; |
Contributor
There was a problem hiding this comment.
Only struct with constexpr constructor can be marked as constexpr. even ConservativeLiteralType is false, NonLiteral cannot be marked as constexpr IMO.
This code block will confuse me how to make this NonLiteral constexpr.
zwuis
reviewed
Oct 11, 2025
Contributor
zwuis
left a comment
zwuis
left a comment
There was a problem hiding this comment.
Can you add more test cases with anonymous union members and constructors?
I realise that suggesting llvm::any_of or llvm::all_of on short for loops is my personal preference. It is not listed in llvm coding standards. Please ignore these suggestions if you don't like them.
HerrCai0907
reviewed
Oct 11, 2025
Comment on lines
381
to
382
| if (!Ctx.getLangOpts().CPlusPlus20) | ||
| return true; |
Contributor
There was a problem hiding this comment.
C++20 should support more constexpr cases, Is this condition reversed?
Contributor
Author
There was a problem hiding this comment.
Good catch. I'm not sure why, but the things below should have been checked in the isLiteralType function. I've folded them into the function in this commit: 5c78e2a
| } | ||
| } | ||
|
|
||
| void UseConstexprCheck::onEndOfTranslationUnit() { |
Contributor
There was a problem hiding this comment.
Can we put diag in UseConstexprCheck::check?
Contributor
Author
There was a problem hiding this comment.
As things are right now, you are right, but this way of doing things becomes necessary once the standard allows declaring variables in C++14+. The reason I have to process the functions and variables together like this, is because for each variable, I have to know if we are proposing to add constexpr to its enclosing function. Pre C++23 we can not declare a static variable constexpr if it is declared inside a constexpr function. You can see the logic to guard against this in action in my old PR, here are the lines of code: https://github.com/5chmidti/llvm-project/blob/dd9039ca95064b5e90075bc0d8405832b000bbf6/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.cpp#L1003-L1006
Contributor
I think that |
shafik
reviewed
Oct 14, 2025
Collaborator
shafik
left a comment
shafik
left a comment
There was a problem hiding this comment.
Will this recommend constexpr for arrays? What about globals?
|
@5chmidti, any chance to finally get the review finished? Would be cool to finally have it available. Is it possible to ping the missing reviewers? |
Contributor
Hi, I don't think there are currently any blocking reviews missing. |
Thanks @vbvictor, then I just hope he finds the time soon. I would be happy to test. |
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. This is part 1/N, adding only the C++11 rule-set. Fixes llvm#115622
Co-authored-by: EugeneZelenko <[email protected]>
5chmidti
force-pushed
the
clang_tidy_constexpr_11
branch
from
52d9889 to
5c78e2a
Compare
5chmidti
commented
Dec 3, 2025
Contributor
Author
5chmidti
left a comment
5chmidti
left a comment
There was a problem hiding this comment.
I'm back on it :)
| } | ||
| } | ||
|
|
||
| void UseConstexprCheck::onEndOfTranslationUnit() { |
Contributor
Author
There was a problem hiding this comment.
As things are right now, you are right, but this way of doing things becomes necessary once the standard allows declaring variables in C++14+. The reason I have to process the functions and variables together like this, is because for each variable, I have to know if we are proposing to add constexpr to its enclosing function. Pre C++23 we can not declare a static variable constexpr if it is declared inside a constexpr function. You can see the logic to guard against this in action in my old PR, here are the lines of code: https://github.com/5chmidti/llvm-project/blob/dd9039ca95064b5e90075bc0d8405832b000bbf6/clang-tools-extra/clang-tidy/modernize/UseConstexprCheck.cpp#L1003-L1006
Comment on lines
381
to
382
| if (!Ctx.getLangOpts().CPlusPlus20) | ||
| return true; |
Contributor
Author
There was a problem hiding this comment.
Good catch. I'm not sure why, but the things below should have been checked in the isLiteralType function. I've folded them into the function in this commit: 5c78e2a
Contributor
Author
@shafik Yes, this does support globals and arrays. Check out this part of tests for some globals and arrays. For arrays, I am checking if the array type itself supports being used in a constexpr context, and each element in the initializing expression is also checked. E.g., in these tests there are functions called in the initializer list that are not constexpr and therefore the array can not be constexpr. |
|
✅ With the latest revision this PR passed the C/C++ code linter. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
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 the requirements are met.This is part 1/N, adding only the C++11 rule-set. The implementation for C++11-C++23 is done. I'll start looking into C++26 and C23 next.
Fixes #115622