Skip to content

Commit

Permalink
Don't take the expression range into account when looking for widening
Browse files Browse the repository at this point in the history
of a unary - expression.

This fixes an issue where we'd produce bogus diagnostics, and also
should recover ~0.3% compile time.
  • Loading branch information
zygoloid committed Sep 2, 2020
1 parent 369f916 commit 0ffbbce
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 12 deletions.
23 changes: 11 additions & 12 deletions clang/lib/Sema/SemaChecking.cpp
Expand Up @@ -12064,13 +12064,13 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
<< E->getType());
}

IntRange MinSourceRange =
IntRange SourceTypeRange =
IntRange::forTargetOfCanonicalType(S.Context, Source);
IntRange LikelySourceRange =
GetExprRange(S.Context, E, S.isConstantEvaluated(), /*Approximate*/ true);
IntRange MaxSourceRange =
GetExprRange(S.Context, E, S.isConstantEvaluated(), /*Approximate*/ false);
IntRange TargetRange = IntRange::forTargetOfCanonicalType(S.Context, Target);

if (MinSourceRange.Width > TargetRange.Width) {
if (LikelySourceRange.Width > TargetRange.Width) {
// If the source is a constant, use a default-on diagnostic.
// TODO: this should happen for bitfield stores, too.
Expr::EvalResult Result;
Expand Down Expand Up @@ -12103,9 +12103,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_integer_precision);
}

// Only warn here if E can't possibly reach the target range, not only if
// it's not likely to be in that range.
if (TargetRange.Width > MaxSourceRange.Width) {
if (TargetRange.Width > SourceTypeRange.Width) {
if (auto *UO = dyn_cast<UnaryOperator>(E))
if (UO->getOpcode() == UO_Minus)
if (Source->isUnsignedIntegerType()) {
Expand All @@ -12118,8 +12116,9 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
}
}

if (TargetRange.Width == MinSourceRange.Width && !TargetRange.NonNegative &&
MinSourceRange.NonNegative && Source->isSignedIntegerType()) {
if (TargetRange.Width == LikelySourceRange.Width &&
!TargetRange.NonNegative && LikelySourceRange.NonNegative &&
Source->isSignedIntegerType()) {
// Warn when doing a signed to signed conversion, warn if the positive
// source value is exactly the width of the target type, which will
// cause a negative value to be stored.
Expand All @@ -12144,9 +12143,9 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
// Fall through for non-constants to give a sign conversion warning.
}

if ((TargetRange.NonNegative && !MinSourceRange.NonNegative) ||
(!TargetRange.NonNegative && MinSourceRange.NonNegative &&
MinSourceRange.Width == TargetRange.Width)) {
if ((TargetRange.NonNegative && !LikelySourceRange.NonNegative) ||
(!TargetRange.NonNegative && LikelySourceRange.NonNegative &&
LikelySourceRange.Width == TargetRange.Width)) {
if (S.SourceMgr.isInSystemMacro(CC))
return;

Expand Down
3 changes: 3 additions & 0 deletions clang/test/Sema/unary-minus-integer-impcast.c
Expand Up @@ -17,4 +17,7 @@ void test(void) {
#else
// expected-warning@-4 {{implicit conversion changes signedness: 'unsigned int' to 'long'}}
#endif

unsigned m;
int n = -(m & 0xff); // no warning
}

0 comments on commit 0ffbbce

Please sign in to comment.