Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ Improvements to Clang's diagnostics
an override of a virtual method.
- Fixed fix-it hint for fold expressions. Clang now correctly places the suggested right
parenthesis when diagnosing malformed fold expressions. (#GH151787)
- Added fix-it hint for when scoped enumerations require explicit conversions for binary operations. (#GH24265)

- Fixed an issue where emitted format-signedness diagnostics were not associated with an appropriate
diagnostic id. Besides being incorrect from an API standpoint, this was user visible, e.g.:
Expand Down
5 changes: 5 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -4404,6 +4404,11 @@ def warn_impcast_different_enum_types : Warning<
def warn_impcast_int_to_enum : Warning<
"implicit conversion from %0 to enumeration type %1 is invalid in C++">,
InGroup<ImplicitIntToEnumCast>, DefaultIgnore;

def note_no_implicit_conversion_for_scoped_enum
: Note<"no implicit conversion for scoped enum; consider casting to "
"underlying type">;

def warn_impcast_bool_to_null_pointer : Warning<
"initialization of pointer of type %0 to null from a constant boolean "
"expression">, InGroup<BoolConversion>;
Expand Down
6 changes: 3 additions & 3 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -8056,8 +8056,8 @@ class Sema final : public SemaBase {
ExprResult &RHS);

QualType CheckMultiplyDivideOperands( // C99 6.5.5
ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, bool IsCompAssign,
bool IsDivide);
ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,
BinaryOperatorKind Opc);
QualType CheckRemainderOperands( // C99 6.5.5
ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,
bool IsCompAssign = false);
Expand All @@ -8066,7 +8066,7 @@ class Sema final : public SemaBase {
BinaryOperatorKind Opc, QualType *CompLHSTy = nullptr);
QualType CheckSubtractionOperands( // C99 6.5.6
ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,
QualType *CompLHSTy = nullptr);
BinaryOperatorKind Opc, QualType *CompLHSTy = nullptr);
QualType CheckShiftOperands( // C99 6.5.7
ExprResult &LHS, ExprResult &RHS, SourceLocation Loc,
BinaryOperatorKind Opc, bool IsCompAssign = false);
Expand Down
119 changes: 93 additions & 26 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10750,9 +10750,50 @@ static void DiagnoseBadDivideOrRemainderValues(Sema& S, ExprResult &LHS,
<< IsDiv << RHS.get()->getSourceRange());
}

static void diagnoseScopedEnums(Sema &S, const SourceLocation Loc,
const ExprResult &LHS, const ExprResult &RHS,
BinaryOperatorKind Opc) {
if (!LHS.isUsable() || !RHS.isUsable())
return;
const Expr *LHSExpr = LHS.get();
const Expr *RHSExpr = RHS.get();
const QualType LHSType = LHSExpr->getType();
const QualType RHSType = RHSExpr->getType();
const bool LHSIsScoped = LHSType->isScopedEnumeralType();
const bool RHSIsScoped = RHSType->isScopedEnumeralType();
if (!LHSIsScoped && !RHSIsScoped)
return;
if (BinaryOperator::isAssignmentOp(Opc) && LHSIsScoped)
return;
if (!LHSIsScoped && !LHSType->isIntegralOrUnscopedEnumerationType())
return;
if (!RHSIsScoped && !RHSType->isIntegralOrUnscopedEnumerationType())
return;
auto DiagnosticHelper = [&S](const Expr *expr, const QualType type) {
SourceLocation BeginLoc = expr->getBeginLoc();
QualType IntType = type->castAs<EnumType>()
->getOriginalDecl()
->getDefinitionOrSelf()
->getIntegerType();
std::string InsertionString = "static_cast<" + IntType.getAsString() + ">(";
S.Diag(BeginLoc, diag::note_no_implicit_conversion_for_scoped_enum)
<< FixItHint::CreateInsertion(BeginLoc, InsertionString)
<< FixItHint::CreateInsertion(expr->getEndLoc(), ")");
};
if (LHSIsScoped) {
DiagnosticHelper(LHSExpr, LHSType);
}
if (RHSIsScoped) {
DiagnosticHelper(RHSExpr, RHSType);
}
}

QualType Sema::CheckMultiplyDivideOperands(ExprResult &LHS, ExprResult &RHS,
SourceLocation Loc,
bool IsCompAssign, bool IsDiv) {
BinaryOperatorKind Opc) {
bool IsCompAssign = Opc == BO_MulAssign || Opc == BO_DivAssign;
bool IsDiv = Opc == BO_Div || Opc == BO_DivAssign;

checkArithmeticNull(*this, LHS, RHS, Loc, /*IsCompare=*/false);

QualType LHSTy = LHS.get()->getType();
Expand Down Expand Up @@ -10780,9 +10821,11 @@ QualType Sema::CheckMultiplyDivideOperands(ExprResult &LHS, ExprResult &RHS,
if (LHS.isInvalid() || RHS.isInvalid())
return QualType();


if (compType.isNull() || !compType->isArithmeticType())
return InvalidOperands(Loc, LHS, RHS);
if (compType.isNull() || !compType->isArithmeticType()) {
QualType ResultTy = InvalidOperands(Loc, LHS, RHS);
diagnoseScopedEnums(*this, Loc, LHS, RHS, Opc);
return ResultTy;
}
if (IsDiv) {
DetectPrecisionLossInComplexDivision(*this, RHS.get()->getType(), Loc);
DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, IsDiv);
Expand Down Expand Up @@ -10845,8 +10888,12 @@ QualType Sema::CheckRemainderOperands(

if (compType.isNull() ||
(!compType->isIntegerType() &&
!(getLangOpts().HLSL && compType->isFloatingType())))
return InvalidOperands(Loc, LHS, RHS);
!(getLangOpts().HLSL && compType->isFloatingType()))) {
QualType ResultTy = InvalidOperands(Loc, LHS, RHS);
diagnoseScopedEnums(*this, Loc, LHS, RHS,
IsCompAssign ? BO_RemAssign : BO_Rem);
return ResultTy;
}
DiagnoseBadDivideOrRemainderValues(*this, LHS, RHS, Loc, false /* IsDiv */);
return compType;
}
Expand Down Expand Up @@ -11202,7 +11249,9 @@ QualType Sema::CheckAdditionOperands(ExprResult &LHS, ExprResult &RHS,
} else if (PExp->getType()->isObjCObjectPointerType()) {
isObjCPointer = true;
} else {
return InvalidOperands(Loc, LHS, RHS);
QualType ResultTy = InvalidOperands(Loc, LHS, RHS);
diagnoseScopedEnums(*this, Loc, LHS, RHS, Opc);
return ResultTy;
}
}
assert(PExp->getType()->isAnyPointerType());
Expand Down Expand Up @@ -11259,7 +11308,8 @@ QualType Sema::CheckAdditionOperands(ExprResult &LHS, ExprResult &RHS,
// C99 6.5.6
QualType Sema::CheckSubtractionOperands(ExprResult &LHS, ExprResult &RHS,
SourceLocation Loc,
QualType* CompLHSTy) {
BinaryOperatorKind Opc,
QualType *CompLHSTy) {
checkArithmeticNull(*this, LHS, RHS, Loc, /*IsCompare=*/false);

if (LHS.get()->getType()->isVectorType() ||
Expand Down Expand Up @@ -11404,7 +11454,9 @@ QualType Sema::CheckSubtractionOperands(ExprResult &LHS, ExprResult &RHS,
}
}

return InvalidOperands(Loc, LHS, RHS);
QualType ResultTy = InvalidOperands(Loc, LHS, RHS);
diagnoseScopedEnums(*this, Loc, LHS, RHS, Opc);
return ResultTy;
}

static bool isScopedEnumerationType(QualType T) {
Expand Down Expand Up @@ -11752,8 +11804,11 @@ QualType Sema::CheckShiftOperands(ExprResult &LHS, ExprResult &RHS,
// Embedded-C 4.1.6.2.2: The LHS may also be fixed-point.
if ((!LHSType->isFixedPointOrIntegerType() &&
!LHSType->hasIntegerRepresentation()) ||
!RHSType->hasIntegerRepresentation())
return InvalidOperands(Loc, LHS, RHS);
!RHSType->hasIntegerRepresentation()) {
QualType ResultTy = InvalidOperands(Loc, LHS, RHS);
diagnoseScopedEnums(*this, Loc, LHS, RHS, Opc);
return ResultTy;
}

// C++0x: Don't allow scoped enums. FIXME: Use something better than
// hasIntegerRepresentation() above instead of this.
Expand Down Expand Up @@ -12321,8 +12376,11 @@ static QualType checkArithmeticOrEnumeralThreeWayCompare(Sema &S,
S.UsualArithmeticConversions(LHS, RHS, Loc, ArithConvKind::Comparison);
if (LHS.isInvalid() || RHS.isInvalid())
return QualType();
if (Type.isNull())
return S.InvalidOperands(Loc, LHS, RHS);
if (Type.isNull()) {
QualType ResultTy = S.InvalidOperands(Loc, LHS, RHS);
diagnoseScopedEnums(S, Loc, LHS, RHS, BO_Cmp);
return ResultTy;
}

std::optional<ComparisonCategoryType> CCT =
getComparisonCategoryForBuiltinCmp(Type);
Expand Down Expand Up @@ -12354,8 +12412,11 @@ static QualType checkArithmeticOrEnumeralCompare(Sema &S, ExprResult &LHS,
S.UsualArithmeticConversions(LHS, RHS, Loc, ArithConvKind::Comparison);
if (LHS.isInvalid() || RHS.isInvalid())
return QualType();
if (Type.isNull())
return S.InvalidOperands(Loc, LHS, RHS);
if (Type.isNull()) {
QualType ResultTy = S.InvalidOperands(Loc, LHS, RHS);
diagnoseScopedEnums(S, Loc, LHS, RHS, Opc);
return ResultTy;
}
assert(Type->isArithmeticType() || Type->isEnumeralType());

if (Type->isAnyComplexType() && BinaryOperator::isRelationalOp(Opc))
Expand Down Expand Up @@ -13365,7 +13426,9 @@ inline QualType Sema::CheckBitwiseOperands(ExprResult &LHS, ExprResult &RHS,

if (!compType.isNull() && compType->isIntegralOrUnscopedEnumerationType())
return compType;
return InvalidOperands(Loc, LHS, RHS);
QualType ResultTy = InvalidOperands(Loc, LHS, RHS);
diagnoseScopedEnums(*this, Loc, LHS, RHS, Opc);
return ResultTy;
}

// C99 6.5.[13,14]
Expand Down Expand Up @@ -13467,13 +13530,19 @@ inline QualType Sema::CheckLogicalOperands(ExprResult &LHS, ExprResult &RHS,
// C++ [expr.log.or]p1
// The operands are both contextually converted to type bool.
ExprResult LHSRes = PerformContextuallyConvertToBool(LHS.get());
if (LHSRes.isInvalid())
return InvalidOperands(Loc, LHS, RHS);
if (LHSRes.isInvalid()) {
QualType ResultTy = InvalidOperands(Loc, LHS, RHS);
diagnoseScopedEnums(*this, Loc, LHS, RHS, Opc);
return ResultTy;
}
LHS = LHSRes;

ExprResult RHSRes = PerformContextuallyConvertToBool(RHS.get());
if (RHSRes.isInvalid())
return InvalidOperands(Loc, LHS, RHS);
if (RHSRes.isInvalid()) {
QualType ResultTy = InvalidOperands(Loc, LHS, RHS);
diagnoseScopedEnums(*this, Loc, LHS, RHS, Opc);
return ResultTy;
}
RHS = RHSRes;

// C++ [expr.log.and]p2
Expand Down Expand Up @@ -15069,8 +15138,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
case BO_Mul:
case BO_Div:
ConvertHalfVec = true;
ResultTy = CheckMultiplyDivideOperands(LHS, RHS, OpLoc, false,
Opc == BO_Div);
ResultTy = CheckMultiplyDivideOperands(LHS, RHS, OpLoc, Opc);
break;
case BO_Rem:
ResultTy = CheckRemainderOperands(LHS, RHS, OpLoc);
Expand All @@ -15081,7 +15149,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
break;
case BO_Sub:
ConvertHalfVec = true;
ResultTy = CheckSubtractionOperands(LHS, RHS, OpLoc);
ResultTy = CheckSubtractionOperands(LHS, RHS, OpLoc, Opc);
break;
case BO_Shl:
case BO_Shr:
Expand Down Expand Up @@ -15125,8 +15193,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
case BO_MulAssign:
case BO_DivAssign:
ConvertHalfVec = true;
CompResultTy = CheckMultiplyDivideOperands(LHS, RHS, OpLoc, true,
Opc == BO_DivAssign);
CompResultTy = CheckMultiplyDivideOperands(LHS, RHS, OpLoc, Opc);
CompLHSTy = CompResultTy;
if (!CompResultTy.isNull() && !LHS.isInvalid() && !RHS.isInvalid())
ResultTy =
Expand All @@ -15148,7 +15215,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
break;
case BO_SubAssign:
ConvertHalfVec = true;
CompResultTy = CheckSubtractionOperands(LHS, RHS, OpLoc, &CompLHSTy);
CompResultTy = CheckSubtractionOperands(LHS, RHS, OpLoc, Opc, &CompLHSTy);
if (!CompResultTy.isNull() && !LHS.isInvalid() && !RHS.isInvalid())
ResultTy =
CheckAssignmentOperands(LHS.get(), RHS, OpLoc, CompResultTy, Opc);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ enum class E { e };

template<typename T> int f(T t) { return ~t; } // expected-error {{invalid argument type}}
template<typename T, typename U> int f(T t, U u) { return t % u; } // expected-error {{invalid operands to}}
// expected-note@-1 {{no implicit conversion for scoped enum}}

int b1 = ~E::e; // expected-error {{invalid argument type}}
int b2 = f(E::e); // expected-note {{in instantiation of}}
Expand Down
95 changes: 95 additions & 0 deletions clang/test/FixIt/fixit-enum-scoped.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits -std=c++20 -triple x86_64-apple-darwin %s 2>&1 | FileCheck %s

namespace GH24265 {
enum class E_int { e };
enum class E_long : long { e };

void f() {
E_int::e + E_long::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:5-[[@LINE]]:5}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:12-[[@LINE-1]]:12}:")"
// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"static_cast<long>("
// CHECK: fix-it:{{.*}}:{[[@LINE-3]]:24-[[@LINE-3]]:24}:")"
E_int::e + 0; // CHECK: fix-it:{{.*}}:{[[@LINE]]:5-[[@LINE]]:5}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:12-[[@LINE-1]]:12}:")"

0 * E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:9-[[@LINE]]:9}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:16}:")"
0 / E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:9-[[@LINE]]:9}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:16}:")"
0 % E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:9-[[@LINE]]:9}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:16}:")"
0 + E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:9-[[@LINE]]:9}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:16}:")"
0 - E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:9-[[@LINE]]:9}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:16}:")"
0 << E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
0 >> E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
0 <=> E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:11-[[@LINE]]:11}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:18-[[@LINE-1]]:18}:")"
0 < E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:9-[[@LINE]]:9}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:16}:")"
0 > E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:9-[[@LINE]]:9}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:16}:")"
0 <= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
0 >= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
0 == E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
0 != E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
0 & E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:9-[[@LINE]]:9}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:16}:")"
0 ^ E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:9-[[@LINE]]:9}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:16}:")"
0 | E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:9-[[@LINE]]:9}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:16-[[@LINE-1]]:16}:")"
0 && E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
0 || E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"

int a;
a *= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
a /= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
a %= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
a += E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
a -= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
a <<= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:11-[[@LINE]]:11}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:18-[[@LINE-1]]:18}:")"
a >>= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:11-[[@LINE]]:11}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:18-[[@LINE-1]]:18}:")"
a &= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
a ^= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"
a |= E_int::e; // CHECK: fix-it:{{.*}}:{[[@LINE]]:10-[[@LINE]]:10}:"static_cast<int>("
// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:17-[[@LINE-1]]:17}:")"

// TODO: These do not have the diagnostic yet
E_int b;
b *= 0;
b /= 0;
b %= 0;
b += 0;
b -= 0;
b <<= 0;
b >>= 0;
b &= 0;
b ^= 0;
b |= 0;

a = E_int::e;
b = 0;

E_int c = 0;
int d = E_int::e;
}
}
Loading