Static casts should perform type propagation instead of treating thei…

…r operands as self determined
MikePopoloski · Jan 12, 2025 · dfed278 · dfed278
1 parent f04d7a0
commit dfed278
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 26 deletions.
diff --git a/include/slang/ast/Expression.h b/include/slang/ast/Expression.h
@@ -69,6 +69,17 @@ SLANG_ENUM(ExpressionKind, EXPRESSION)
 #define RANGE(x) x(Simple) x(IndexedUp) x(IndexedDown)
 SLANG_ENUM(RangeSelectionKind, RANGE)
 #undef RANGE
+
+// clang-format off
+#define CK(x) \
+    x(Implicit) \
+    x(Propagated) \
+    x(StreamingConcat) \
+    x(Explicit) \
+    x(BitstreamCast)
+// clang-format on
+SLANG_ENUM(ConversionKind, CK)
+#undef CK
 // clang-format on
 
 /// The base class for all expressions in SystemVerilog.
@@ -397,7 +408,8 @@ class SLANG_EXPORT Expression {
     // Perform type propagation and constant folding of a context-determined subexpression.
     static void contextDetermined(const ASTContext& context, Expression*& expr,
                                   const Expression* parentExpr, const Type& newType,
-                                  SourceRange operatorRange, bool isAssignment = false);
+                                  SourceRange operatorRange,
+                                  ConversionKind conversionKind = ConversionKind::Propagated);
 
     // Perform type propagation and constant folding of a self-determined subexpression.
     static void selfDetermined(const ASTContext& context, Expression*& expr);

diff --git a/include/slang/ast/expressions/ConversionExpression.h b/include/slang/ast/expressions/ConversionExpression.h
@@ -11,17 +11,6 @@
 
 namespace slang::ast {
 
-// clang-format off
-#define CK(x) \
-    x(Implicit) \
-    x(Propagated) \
-    x(StreamingConcat) \
-    x(Explicit) \
-    x(BitstreamCast)
-// clang-format on
-SLANG_ENUM(ConversionKind, CK)
-#undef CK
-
 /// Represents a type conversion expression (implicit or explicit).
 class SLANG_EXPORT ConversionExpression : public Expression {
 public:

diff --git a/source/ast/Expression.cpp b/source/ast/Expression.cpp
@@ -138,12 +138,12 @@ struct Expression::PropagationVisitor {
     const Type& newType;
     const Expression* parentExpr;
     SourceRange opRange;
-    bool isAssignment;
+    ConversionKind conversionKind;
 
     PropagationVisitor(const ASTContext& context, const Type& newType, const Expression* parentExpr,
-                       SourceRange opRange, bool isAssignment) :
+                       SourceRange opRange, ConversionKind conversionKind) :
         context(context), newType(newType), parentExpr(parentExpr), opRange(opRange),
-        isAssignment(isAssignment) {}
+        conversionKind(conversionKind) {}
 
     template<typename T>
     Expression& visit(T& expr) {
@@ -173,7 +173,8 @@ struct Expression::PropagationVisitor {
                     // most immediate parent expression instead.
                     updateRange(expr);
                 }
-                else if (expr.kind == ExpressionKind::ConditionalOp && isAssignment) {
+                else if (expr.kind == ExpressionKind::ConditionalOp &&
+                         conversionKind == ConversionKind::Implicit) {
                     // This is a special case to make sure we get a width expansion
                     // warning for assignments from a conditional operator. The type
                     // conversion here is a propagation so no implicit conversion
@@ -190,8 +191,6 @@ struct Expression::PropagationVisitor {
 
         Expression* result = &expr;
         if (needConversion) {
-            auto conversionKind = isAssignment ? ConversionKind::Implicit
-                                               : ConversionKind::Propagated;
             result = &ConversionExpression::makeImplicit(context, newType, conversionKind, expr,
                                                          parentExpr, opRange);
         }
@@ -1549,14 +1548,14 @@ void Expression::findPotentiallyImplicitNets(
 
 void Expression::contextDetermined(const ASTContext& context, Expression*& expr,
                                    const Expression* parentExpr, const Type& newType,
-                                   SourceRange opRange, bool isAssignment) {
-    PropagationVisitor visitor(context, newType, parentExpr, opRange, isAssignment);
+                                   SourceRange opRange, ConversionKind conversionKind) {
+    PropagationVisitor visitor(context, newType, parentExpr, opRange, conversionKind);
     expr = &expr->visit(visitor);
 }
 
 void Expression::selfDetermined(const ASTContext& context, Expression*& expr) {
     SLANG_ASSERT(expr->type);
-    PropagationVisitor visitor(context, *expr->type, nullptr, {}, false);
+    PropagationVisitor visitor(context, *expr->type, nullptr, {}, ConversionKind::Propagated);
     expr = &expr->visit(visitor);
 }
 

diff --git a/source/ast/expressions/ConversionExpression.cpp b/source/ast/expressions/ConversionExpression.cpp
@@ -147,7 +147,7 @@ Expression& Expression::convertAssignment(const ASTContext& context, const Type&
     const Type* rt = expr.type;
 
     auto finalizeType = [&](const Type& t) {
-        contextDetermined(context, result, nullptr, t, assignmentRange, /* isAssignment */ true);
+        contextDetermined(context, result, nullptr, t, assignmentRange, ConversionKind::Implicit);
     };
 
     if (type.isEquivalent(*rt)) {
@@ -328,8 +328,6 @@ Expression& ConversionExpression::fromSyntax(Compilation& comp, const CastExpres
         }
 
         operand = &create(comp, *syntax.right, context, ASTFlags::StreamingAllowed, type);
-        selfDetermined(context, operand);
-
         if (operand->bad())
             return badExpr(comp, nullptr);
     }
@@ -342,7 +340,7 @@ Expression& ConversionExpression::fromSyntax(Compilation& comp, const CastExpres
         if (!context.requireValidBitWidth(width, targetExpr.sourceRange))
             return badExpr(comp, nullptr);
 
-        operand = &selfDetermined(comp, *syntax.right, context, ASTFlags::StreamingAllowed);
+        operand = &create(comp, *syntax.right, context, ASTFlags::StreamingAllowed);
         if (operand->bad())
             return badExpr(comp, nullptr);
 
@@ -411,6 +409,29 @@ Expression& ConversionExpression::fromSyntax(Compilation& comp, const CastExpres
             << *operand->type << targetExpr.sourceRange << operand->sourceRange;
     }
 
+    if (type->isAssignmentCompatible(*operand->type)) {
+        // The type we propagate should use the sign of the operand, just like it
+        // would if we were doing an implicit conversion via an assignment expression.
+        auto propagatedType = type;
+        if (type->isIntegral() && operand->type->isIntegral() &&
+            type->getIntegralFlags() != operand->type->getIntegralFlags()) {
+            propagatedType = &comp.getType(type->getBitWidth(), operand->type->getIntegralFlags());
+        }
+
+        contextDetermined(context, operand, nullptr, *propagatedType, syntax.apostrophe.range(),
+                          ConversionKind::Explicit);
+
+        if (operand->kind == ExpressionKind::Conversion &&
+            !operand->as<ConversionExpression>().isImplicit()) {
+            operand->sourceRange = syntax.sourceRange();
+            operand->type = type;
+            return *operand;
+        }
+    }
+    else {
+        selfDetermined(context, operand);
+    }
+
     return *result();
 }
 

diff --git a/tests/unittests/ast/EvalTests.cpp b/tests/unittests/ast/EvalTests.cpp
@@ -2610,7 +2610,7 @@ endmodule
 }
 
 TEST_CASE("Eval truthiness of strings") {
-    ScriptSession session(optionsFor(LanguageVersion::v1800_2023));
+    ScriptSession session;
     session.eval(R"(
 function automatic bit b;
     string s = "SDF";
@@ -2624,3 +2624,11 @@ endfunction
 
     NO_SESSION_ERRORS;
 }
+
+TEST_CASE("Eval static cast type propagation") {
+    ScriptSession session;
+    CHECK(session.eval("(3)'(1'b1 << 2)").integer() == 4);
+    CHECK(session.eval("int'(1'b1 + (1'b1 << 2))").integer() == 5);
+
+    NO_SESSION_ERRORS;
+}