================
@@ -131,19 +132,22 @@ void
ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr(
QualType WideExprTy;
// Get Ty of the same signedness as ExprTy, because we only want to suggest
// to widen the computation, but not change it's signedness domain.
- if (Ty->isSignedIntegerType() == ETy->isSignedIntegerType()) {
+ // However, if ETy is only signed because both operands were of an
+ // unsigned type narrower than int (and thus got integer-promoted to the
+ // signed type int), the multiplication was never really operating in a
+ // signed domain to begin with, so don't force a signed widened type in
+ // that case either.
+ const bool BothOperandsWereUnsigned =
+ LHS->IgnoreImpCasts()->getType()->isUnsignedIntegerType() &&
+ RHS->IgnoreImpCasts()->getType()->isUnsignedIntegerType();
+ const bool EffectiveETyIsSigned =
+ ETy->isSignedIntegerType() && !BothOperandsWereUnsigned;
+ if (Ty->isSignedIntegerType() == EffectiveETyIsSigned)
WideExprTy = Ty;
- } else if (Ty->isSignedIntegerType()) {
- assert(ETy->isUnsignedIntegerType() &&
- "Expected source type to be signed.");
+ else if (Ty->isSignedIntegerType())
WideExprTy = Context->getCorrespondingUnsignedType(Ty);
- } else {
- assert(Ty->isUnsignedIntegerType() &&
----------------
zeyi2 wrote:
These assertions still seem valid: reaching this branch means that the target
is unsigned and that `EffectiveETyIsSigned` is true, which in turn implies that
`ETy` is signed. Could we keep them instead of removing all the assertions?
Also the second assert has a typo (`unsigned` -> `signed`). While we're here,
could you please fix that as a drive-by?
https://github.com/llvm/llvm-project/pull/214501
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
