[clang-tidy] add modernize-use-constexpr check #162741
Open
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
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
5chmidti
requested review from
EugeneZelenko,
HerrCai0907,
PiotrZSL,
carlosgalvezp and
vbvictor
|
@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]
|
|
@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]
|
|
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
Co-authored-by: EugeneZelenko <[email protected]>
5chmidti
changed the title
zwuis
reviewed
Oct 10, 2025
Comment on lines
+27
to
+32
| if (IsInMainFile && Node.hasExternalFormalLinkage()) | ||
| return false; | ||
| if (!IsInMainFile && !Node.isInlined()) | ||
| return false; | ||
|
|
||
| return true; |
There was a problem hiding this comment.
if (IsInMainFile)
return !Node.hasExternalFormalLinkage();
// not in main file
return Node.isInlined();I think it's more clear.
Comment on lines
+60
to
+64
| for (const Decl *ReDecl : Node.redecls()) { | ||
| if (!SM.isWrittenInSameFile(L, ReDecl->getLocation())) | ||
| return false; | ||
| } | ||
| return true; |
Comment on lines
+81
to
+82
| const Type *InitType = Init->getBaseClass(); | ||
| if (InitType && InitType->isRecordType()) { |
There was a problem hiding this comment.
if (const Type *InitType = ...;
InitType && ...) {
// ...
}| Indirects.insert(Field); | ||
|
|
||
| Fields.erase(Field); | ||
| continue; |
Comment on lines
+371
to
+376
| const bool IsDeclaredInsideConstexprFunction = std::invoke([&Node]() { | ||
| const auto *Func = llvm::dyn_cast<FunctionDecl>(Node.getDeclContext()); | ||
| if (!Func) | ||
| return false; | ||
| return Func->isConstexpr(); | ||
| }); |
There was a problem hiding this comment.
I think calling lambda immediately is better than using std::invoke. It is already clear that lambda is a function object.
Comment on lines
+373
to
+375
| if (!Func) | ||
| return false; | ||
| return Func->isConstexpr(); |
There was a problem hiding this comment.
return Func && Func->isConstexpr();
Comment on lines
+389
to
+390
| if (RDecl && (!RDecl->hasDefinition() || !RDecl->hasConstexprDestructor())) | ||
| return false; |
There was a problem hiding this comment.
if (RDecl)
return RDecl->hasDefinition() && RDecl->hasConstexprDestructor();
Comment on lines
+400
to
+403
| unless(anyOf(isConstexpr(), isImplicit(), hasExternalFormalLinkage(), | ||
| isInMacro(), isMain(), isInStdNamespace(), | ||
| isExpansionInSystemHeader(), isExternC(), | ||
| cxxMethodDecl(ofClass(cxxRecordDecl(isLambda()))))), |
There was a problem hiding this comment.
Should we add isDeleted to unless? It seems meaningless to suggest adding constexpr to functions with = delete.
| FunctionReplacement); | ||
| } | ||
|
|
||
| const auto MaybeRemoveConst = [&, this](DiagnosticBuilder &Diag, |
There was a problem hiding this comment.
*this is already implicitly captured by reference if the capture-default is &.
| }; | ||
|
|
||
| This type is a literal type, but can not be constructed at compile-time. | ||
| With `ConservativeLiteralType` equal to `true`, variables or funtions |
There was a problem hiding this comment.
Suggested change
| With `ConservativeLiteralType` equal to `true`, variables or funtions | |
| With :option:`ConservativeLiteralType` equal to `true`, variables or functions |
| .. option:: ConservativeLiteralType | ||
|
|
||
| With this option enabled, only literal types that can be constructed at | ||
| compile-time are considered to supoprt ``constexpr``. |
There was a problem hiding this comment.
Suggested change
| compile-time are considered to supoprt ``constexpr``. | |
| compile-time are considered to support ``constexpr``. |
HerrCai0907
reviewed
Oct 11, 2025
Comment on lines
+45
to
+49
| struct NonLiteral{ | ||
| NonLiteral(); | ||
| ~NonLiteral(); | ||
| int &ref; | ||
| }; |
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
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.
| satisfiesConstructorPropertiesUntil20(const CXXConstructorDecl *Ctor, | ||
| ASTContext &Ctx) { | ||
| const CXXRecordDecl *Rec = Ctor->getParent(); | ||
| llvm::SmallPtrSet<const RecordDecl *, 8> Bases{}; |
|
|
||
| if (Method && | ||
| (Method->isVirtual() || | ||
| !match(cxxMethodDecl(hasBody(cxxTryStmt())), *Method, Ctx).empty())) |
There was a problem hiding this comment.
Can we remove this? It seems that function try blocks are correctly handled by Visitor11.
Comment on lines
+203
to
+206
| llvm::any_of(Ctor->getParent()->bases(), | ||
| [](const CXXBaseSpecifier &Base) { | ||
| return Base.isVirtual(); | ||
| }))) |
There was a problem hiding this comment.
Is this case handled correctly
struct A {};
struct B : virtual A {};
struct C : B {
/* constexpr? */ C() {}
};? Perhaps using CXXRecordDecl::getNumVBases is better.
Comment on lines
+216
to
+218
| for (const ParmVarDecl *Param : FDecl->parameters()) | ||
| if (!isLiteralType(Param->getType(), Ctx, ConservativeLiteralType)) | ||
| return false; |
HerrCai0907
reviewed
Oct 11, 2025
Comment on lines
+381
to
+382
| if (!Ctx.getLangOpts().CPlusPlus20) | ||
| return true; |
There was a problem hiding this comment.
C++20 should support more constexpr cases, Is this condition reversed?
| } | ||
| } | ||
|
|
||
| void UseConstexprCheck::onEndOfTranslationUnit() { |
There was a problem hiding this comment.
Can we put diag in UseConstexprCheck::check?
I think that |
shafik
reviewed
Oct 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 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