diff --git a/include/slang/ast/expressions/LiteralExpressions.h b/include/slang/ast/expressions/LiteralExpressions.h index 5c293f40d..dd1ea7fde 100644 --- a/include/slang/ast/expressions/LiteralExpressions.h +++ b/include/slang/ast/expressions/LiteralExpressions.h @@ -100,7 +100,8 @@ class SLANG_EXPORT UnbasedUnsizedIntegerLiteral : public Expression { SVInt getValue() const; ConstantValue evalImpl(EvalContext& context) const; - bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange); + bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange, + ConversionKind conversionKind); std::optional getEffectiveWidthImpl() const; EffectiveSign getEffectiveSignImpl(bool isForConversion) const; diff --git a/include/slang/ast/expressions/MiscExpressions.h b/include/slang/ast/expressions/MiscExpressions.h index ac4cd5f9b..cccc619cd 100644 --- a/include/slang/ast/expressions/MiscExpressions.h +++ b/include/slang/ast/expressions/MiscExpressions.h @@ -306,7 +306,8 @@ class SLANG_EXPORT MinTypMaxExpression : public Expression { Expression& selected() { return *selected_; } ConstantValue evalImpl(EvalContext& context) const; - bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange); + bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange, + ConversionKind conversionKind); std::optional getEffectiveWidthImpl() const; EffectiveSign getEffectiveSignImpl(bool isForConversion) const; diff --git a/include/slang/ast/expressions/OperatorExpressions.h b/include/slang/ast/expressions/OperatorExpressions.h index b549cce9e..87585d1c1 100644 --- a/include/slang/ast/expressions/OperatorExpressions.h +++ b/include/slang/ast/expressions/OperatorExpressions.h @@ -37,7 +37,8 @@ class SLANG_EXPORT UnaryExpression : public Expression { Expression& operand() { return *operand_; } ConstantValue evalImpl(EvalContext& context) const; - bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange); + bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange, + ConversionKind conversionKind); std::optional getEffectiveWidthImpl() const; EffectiveSign getEffectiveSignImpl(bool isForConversion) const; @@ -93,7 +94,8 @@ class SLANG_EXPORT BinaryExpression : public Expression { Expression& right() { return *right_; } ConstantValue evalImpl(EvalContext& context) const; - bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange); + bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange, + ConversionKind conversionKind); std::optional getEffectiveWidthImpl() const; EffectiveSign getEffectiveSignImpl(bool isForConversion) const; @@ -158,7 +160,8 @@ class SLANG_EXPORT ConditionalExpression : public Expression { Expression& right() { return *right_; } ConstantValue evalImpl(EvalContext& context) const; - bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange); + bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange, + ConversionKind conversionKind); std::optional getEffectiveWidthImpl() const; EffectiveSign getEffectiveSignImpl(bool isForConversion) const; @@ -394,7 +397,8 @@ class SLANG_EXPORT ValueRangeExpression : public Expression { Expression& right() { return *right_; } ConstantValue evalImpl(EvalContext& context) const; - bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange); + bool propagateType(const ASTContext& context, const Type& newType, SourceRange opRange, + ConversionKind conversionKind); ConstantValue checkInside(EvalContext& context, const ConstantValue& val) const; diff --git a/source/ast/Expression.cpp b/source/ast/Expression.cpp index e781bb724..517db7e1e 100644 --- a/source/ast/Expression.cpp +++ b/source/ast/Expression.cpp @@ -160,7 +160,7 @@ struct Expression::PropagationVisitor { // check if the conversion should be pushed further down the tree. Otherwise we // should insert the implicit conversion here. bool needConversion = !newType.isEquivalent(*expr.type); - if constexpr (requires { expr.propagateType(context, newType, opRange); }) { + if constexpr (requires { expr.propagateType(context, newType, opRange, conversionKind); }) { if ((newType.isFloating() && expr.type->isFloating()) || (newType.isIntegral() && expr.type->isIntegral()) || newType.isString() || expr.kind == ExpressionKind::ValueRange) { @@ -174,7 +174,7 @@ struct Expression::PropagationVisitor { updateRange(expr); } - if (expr.propagateType(context, newType, opRange)) { + if (expr.propagateType(context, newType, opRange, conversionKind)) { // We propagated the type successfully so we don't need a conversion. // We should however clear out any constant value that may have been // stored here, since it may no longer be valid given the new type. diff --git a/source/ast/expressions/ConversionExpression.cpp b/source/ast/expressions/ConversionExpression.cpp index 7cfd5db73..79275df7f 100644 --- a/source/ast/expressions/ConversionExpression.cpp +++ b/source/ast/expressions/ConversionExpression.cpp @@ -303,6 +303,8 @@ static bool actuallyNeededCast(const Type& type, const Expression& operand) { return actuallyNeededCast(type, operand.as().selected()); case ExpressionKind::ConditionalOp: { auto& cond = operand.as(); + if (!type.isEquivalent(*cond.left().type) || !type.isEquivalent(*cond.right().type)) + return true; return actuallyNeededCast(type, cond.left()) || actuallyNeededCast(type, cond.right()); } default: @@ -403,7 +405,8 @@ Expression& ConversionExpression::fromSyntax(Compilation& comp, const CastExpres }; if (type->isMatching(*operand->type) && !isGenvar() && !isDifferentTypedef() && - ((assignmentTarget && assignmentTarget->isMatching(*type)) || + ((assignmentTarget && assignmentTarget->isMatching(*type) && + operand->kind != ExpressionKind::ConditionalOp) || !actuallyNeededCast(*type, *operand))) { context.addDiag(diag::UselessCast, syntax.apostrophe.location()) << *operand->type << targetExpr.sourceRange << operand->sourceRange; diff --git a/source/ast/expressions/LiteralExpressions.cpp b/source/ast/expressions/LiteralExpressions.cpp index 033b57e55..86b12b916 100644 --- a/source/ast/expressions/LiteralExpressions.cpp +++ b/source/ast/expressions/LiteralExpressions.cpp @@ -166,7 +166,7 @@ Expression& UnbasedUnsizedIntegerLiteral::fromSyntax(Compilation& compilation, } bool UnbasedUnsizedIntegerLiteral::propagateType(const ASTContext&, const Type& newType, - SourceRange) { + SourceRange, ConversionKind) { SLANG_ASSERT(newType.isIntegral()); SLANG_ASSERT(newType.getBitWidth()); diff --git a/source/ast/expressions/MiscExpressions.cpp b/source/ast/expressions/MiscExpressions.cpp index 90d6a9af2..e135af786 100644 --- a/source/ast/expressions/MiscExpressions.cpp +++ b/source/ast/expressions/MiscExpressions.cpp @@ -1340,7 +1340,7 @@ Expression& MinTypMaxExpression::fromSyntax(Compilation& compilation, } bool MinTypMaxExpression::propagateType(const ASTContext& context, const Type& newType, - SourceRange opRange) { + SourceRange opRange, ConversionKind) { // Only the selected expression gets a propagated type. type = &newType; contextDetermined(context, selected_, this, newType, opRange); diff --git a/source/ast/expressions/OperatorExpressions.cpp b/source/ast/expressions/OperatorExpressions.cpp index 09f86f30c..7d76a777d 100644 --- a/source/ast/expressions/OperatorExpressions.cpp +++ b/source/ast/expressions/OperatorExpressions.cpp @@ -329,7 +329,7 @@ Expression& UnaryExpression::fromSyntax(Compilation& compilation, } bool UnaryExpression::propagateType(const ASTContext& context, const Type& newType, - SourceRange propRange) { + SourceRange propRange, ConversionKind) { switch (op) { case UnaryOperator::Plus: case UnaryOperator::Minus: @@ -1113,7 +1113,7 @@ void BinaryExpression::analyzeOpTypes(const Type& clt, const Type& crt, const Ty } bool BinaryExpression::propagateType(const ASTContext& context, const Type& newType, - SourceRange propRange) { + SourceRange propRange, ConversionKind) { switch (op) { case BinaryOperator::Add: case BinaryOperator::Subtract: @@ -1460,7 +1460,7 @@ Expression& ConditionalExpression::fromSyntax(Compilation& comp, } bool ConditionalExpression::propagateType(const ASTContext& context, const Type& newType, - SourceRange opRange) { + SourceRange opRange, ConversionKind conversionKind) { const bool parentTypeEquiv = type->isEquivalent(newType); type = &newType; @@ -1477,7 +1477,8 @@ bool ConditionalExpression::propagateType(const ASTContext& context, const Type& std::optional otherEffectiveWidth) { // This is a propagated conversion but we'd like to see width-expand // warnings anyway so we'll manually do the check conversion check here. - if (!flags.has(ASTFlags::UnevaluatedBranch)) { + if (!flags.has(ASTFlags::UnevaluatedBranch) && + conversionKind <= ConversionKind::Propagated) { // If the parent type was already equivalent to what's being propagated, // then the only conversions we might be doing are "self induced", in the // sense that one branch is propagating its type to the other side. @@ -2423,7 +2424,7 @@ Expression& ValueRangeExpression::fromSyntax(Compilation& comp, } bool ValueRangeExpression::propagateType(const ASTContext& context, const Type& newType, - SourceRange opRange) { + SourceRange opRange, ConversionKind) { contextDetermined(context, left_, this, newType, opRange); if (rangeKind == ValueRangeKind::Simple) contextDetermined(context, right_, this, newType, opRange); diff --git a/tests/unittests/ast/WarningTests.cpp b/tests/unittests/ast/WarningTests.cpp index fea4cae77..c5b1572a8 100644 --- a/tests/unittests/ast/WarningTests.cpp +++ b/tests/unittests/ast/WarningTests.cpp @@ -1358,6 +1358,21 @@ module test2; c = cond ? d : 0; end endmodule + +module test3; + localparam int unsigned SIZE = 16; + + logic a; + logic [7:0] b, c; + logic [SIZE-1:0] d, e, f, g; + + always_comb begin + d = SIZE'( a ? b : c); + e = SIZE'(b); + f = a ? SIZE'(b) : SIZE'(c); + g = SIZE'( (a == 0) ? f : c); + end +endmodule )"); Compilation compilation;