diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 1dd97bcc6736..b1bc087322c7 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 bf7017436f38..604bb56d2825 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, 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, DefaultError; + def warn_enum_constant_in_bool_context : Warning< "converting the enum constant to a boolean">, InGroup, DefaultIgnore; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 003583f84cf9..14f9304b9903 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -7215,13 +7215,15 @@ public: 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 b6867077d6ff..0cd86dc54b0a 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(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(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 3bcbe0f2133a..7743ce0a62e1 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -2967,10 +2967,11 @@ public: /// /// By default, performs semantic analysis to build the new expression. /// Subclasses may override this routine to provide different 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::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::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(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 ac2711169773..4a329646b799 100644 --- a/clang/test/Parser/cxx1z-fold-expressions.cpp +++ b/clang/test/Parser/cxx1z-fold-expressions.cpp @@ -52,9 +52,11 @@ template void as_operand_of_cast(int a, T ...t) { // fold-operator can be '>' or '>>'. template constexpr bool greaterThan() { return (N > ...); } +// expected-error@-1 {{comparison in fold expression}} + template 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 constexpr auto Identity = V; diff --git a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp index 47a252eb335f..1ac821002f95 100644 --- a/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp +++ b/clang/test/SemaTemplate/cxx1z-fold-expressions.cpp @@ -132,3 +132,30 @@ bool f(); template void g(bool = (f() || ...)); } + + +namespace comparison_warning { + struct S { + bool operator<(const S&) const; + bool operator<(int) const; + bool operator==(const S&) const; + }; + + template + 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); + } + +};