Author: cor3ntin Date: 2025-05-01T17:00:03+02:00 New Revision: 7ffaaf47528ff42d4c037e1b003d940e51347db5
URL: https://github.com/llvm/llvm-project/commit/7ffaaf47528ff42d4c037e1b003d940e51347db5 DIFF: https://github.com/llvm/llvm-project/commit/7ffaaf47528ff42d4c037e1b003d940e51347db5.diff LOG: [Clang] Warning as error for fold expressions over comparison operators (#136836) We made chained comparisons an error. Fold-expressions over a comparison operator produce chained comparisons, so we should be consistent there too. We only emit the warning when instantiating the fold expression so as not to warn on types with user-defined comparisons. Partially addresses #129570 Added: Modified: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaExpr.cpp clang/lib/Sema/TreeTransform.h clang/test/Parser/cxx1z-fold-expressions.cpp clang/test/SemaTemplate/cxx1z-fold-expressions.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 1dd97bcc67364..b1bc087322c72 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -389,6 +389,7 @@ Improvements to Clang's diagnostics under the subgroup ``-Wunsafe-buffer-usage-in-libc-call``. - Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with ``-Wno-error=parentheses``. +- Similarly, fold expressions over a comparison operator are now an error by default. - Clang now better preserves the sugared types of pointers to member. - Clang now better preserves the presence of the template keyword with dependent prefixes. diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index bf7017436f38c..604bb56d28252 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7204,6 +7204,11 @@ def warn_consecutive_comparison : Warning< "chained comparison 'X %0 Y %1 Z' does not behave the same as a mathematical expression">, InGroup<Parentheses>, DefaultError; +def warn_comparison_in_fold_expression : Warning< + "comparison in fold expression would evaluate to '(X %0 Y) %0 Z' " + "which does not behave the same as a mathematical expression">, + InGroup<Parentheses>, DefaultError; + def warn_enum_constant_in_bool_context : Warning< "converting the enum constant to a boolean">, InGroup<IntInBoolContext>, DefaultIgnore; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 003583f84cf97..14f9304b99030 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -7215,13 +7215,15 @@ class Sema final : public SemaBase { ExprResult ActOnBinOp(Scope *S, SourceLocation TokLoc, tok::TokenKind Kind, Expr *LHSExpr, Expr *RHSExpr); ExprResult BuildBinOp(Scope *S, SourceLocation OpLoc, BinaryOperatorKind Opc, - Expr *LHSExpr, Expr *RHSExpr); + Expr *LHSExpr, Expr *RHSExpr, + bool ForFoldExpression = false); /// CreateBuiltinBinOp - Creates a new built-in binary operation with /// operator @p Opc at location @c TokLoc. This routine only supports /// built-in operations; ActOnBinOp handles overloaded operators. ExprResult CreateBuiltinBinOp(SourceLocation OpLoc, BinaryOperatorKind Opc, - Expr *LHSExpr, Expr *RHSExpr); + Expr *LHSExpr, Expr *RHSExpr, + bool ForFoldExpression = false); void LookupBinOp(Scope *S, SourceLocation OpLoc, BinaryOperatorKind Opc, UnresolvedSetImpl &Functions); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index b6867077d6ff8..0cd86dc54b0ab 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -14947,8 +14947,8 @@ static bool needsConversionOfHalfVec(bool OpRequiresConversion, ASTContext &Ctx, } ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, - BinaryOperatorKind Opc, - Expr *LHSExpr, Expr *RHSExpr) { + BinaryOperatorKind Opc, Expr *LHSExpr, + Expr *RHSExpr, bool ForFoldExpression) { if (getLangOpts().CPlusPlus11 && isa<InitListExpr>(RHSExpr)) { // The syntax only allows initializer lists on the RHS of assignment, // so we don't need to worry about accepting invalid code for @@ -15079,7 +15079,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc); if (const auto *BI = dyn_cast<BinaryOperator>(LHSExpr); - BI && BI->isComparisonOp()) + !ForFoldExpression && BI && BI->isComparisonOp()) Diag(OpLoc, diag::warn_consecutive_comparison) << BI->getOpcodeStr() << BinaryOperator::getOpcodeStr(Opc); @@ -15490,8 +15490,8 @@ static ExprResult BuildOverloadedBinOp(Sema &S, Scope *Sc, SourceLocation OpLoc, } ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc, - BinaryOperatorKind Opc, - Expr *LHSExpr, Expr *RHSExpr) { + BinaryOperatorKind Opc, Expr *LHSExpr, + Expr *RHSExpr, bool ForFoldExpression) { ExprResult LHS, RHS; std::tie(LHS, RHS) = CorrectDelayedTyposInBinOp(*this, Opc, LHSExpr, RHSExpr); if (!LHS.isUsable() || !RHS.isUsable()) @@ -15565,7 +15565,8 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc, LHSExpr->getType()->isOverloadableType())) return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr); - return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr); + return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr, + ForFoldExpression); } // Don't resolve overloads if the other type is overloadable. @@ -15629,7 +15630,7 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc, } // Build a built-in binary operation. - return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr); + return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr, ForFoldExpression); } static bool isOverflowingIntegerType(ASTContext &Ctx, QualType T) { diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 3bcbe0f2133a2..7743ce0a62e1f 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -2967,10 +2967,11 @@ class TreeTransform { /// /// By default, performs semantic analysis to build the new expression. /// Subclasses may override this routine to provide diff erent behavior. - ExprResult RebuildBinaryOperator(SourceLocation OpLoc, - BinaryOperatorKind Opc, - Expr *LHS, Expr *RHS) { - return getSema().BuildBinOp(/*Scope=*/nullptr, OpLoc, Opc, LHS, RHS); + ExprResult RebuildBinaryOperator(SourceLocation OpLoc, BinaryOperatorKind Opc, + Expr *LHS, Expr *RHS, + bool ForFoldExpression = false) { + return getSema().BuildBinOp(/*Scope=*/nullptr, OpLoc, Opc, LHS, RHS, + ForFoldExpression); } /// Build a new rewritten operator expression. @@ -16408,6 +16409,7 @@ TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) { return true; } + bool WarnedOnComparison = false; for (unsigned I = 0; I != *NumExpansions; ++I) { Sema::ArgPackSubstIndexRAII SubstIndex( getSema(), LeftFold ? I : *NumExpansions - I - 1); @@ -16435,7 +16437,17 @@ TreeTransform<Derived>::TransformCXXFoldExpr(CXXFoldExpr *E) { Functions, LHS, RHS); } else { Result = getDerived().RebuildBinaryOperator(E->getEllipsisLoc(), - E->getOperator(), LHS, RHS); + E->getOperator(), LHS, RHS, + /*ForFoldExpresion=*/true); + if (!WarnedOnComparison && Result.isUsable()) { + if (auto *BO = dyn_cast<BinaryOperator>(Result.get()); + BO && BO->isComparisonOp()) { + WarnedOnComparison = true; + SemaRef.Diag(BO->getBeginLoc(), + diag::warn_comparison_in_fold_expression) + << BO->getOpcodeStr(); + } + } } } else Result = Out; diff --git a/clang/test/Parser/cxx1z-fold-expressions.cpp b/clang/test/Parser/cxx1z-fold-expressions.cpp index ac27111697737..4a329646b799f 100644 --- a/clang/test/Parser/cxx1z-fold-expressions.cpp +++ b/clang/test/Parser/cxx1z-fold-expressions.cpp @@ -52,9 +52,11 @@ template<typename ...T> void as_operand_of_cast(int a, T ...t) { // fold-operator can be '>' or '>>'. template <int... N> constexpr bool greaterThan() { return (N > ...); } +// expected-error@-1 {{comparison in fold expression}} + template <int... N> constexpr int rightShift() { return (N >> ...); } -static_assert(greaterThan<2, 1>()); +static_assert(greaterThan<2, 1>()); // expected-note {{in instantiation}} static_assert(rightShift<10, 1>() == 5); template <auto V> constexpr auto Identity = V; diff --git a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp index 47a252eb335f6..1ac821002f95c 100644 --- a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp +++ b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp @@ -132,3 +132,30 @@ bool f(); template <typename... T> void g(bool = (f<T>() || ...)); } + + +namespace comparison_warning { + struct S { + bool operator<(const S&) const; + bool operator<(int) const; + bool operator==(const S&) const; + }; + + template <typename...T> + void f(T... ts) { + (void)(ts == ...); + // expected-error@-1 2{{comparison in fold expression would evaluate to '(X == Y) == Z'}} + (void)(ts < ...); + // expected-error@-1 2{{comparison in fold expression would evaluate to '(X < Y) < Z'}} + (void)(... < ts); + // expected-error@-1 2{{comparison in fold expression would evaluate to '(X < Y) < Z'}} + } + + void test() { + f(0, 1, 2); // expected-note{{in instantiation}} + f(0, 1); // expected-note{{in instantiation}} + f(S{}, S{}); + f(0); + } + +}; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
