Merge branch 'master' into netlist

* master:
  Fix inference of parameters from large based integer literals
  Fix Wvector-overflow to work correctly with signed literals
  Fix cases where constant conversions were not being checked due to missing tryEval calls

include/slang/ast/SystemSubroutine.h

@@ -48,7 +48,7 @@ class SLANG_EXPORT SystemSubroutine {
     const Type& badArg(const ASTContext& context, const Expression& arg) const;
 
     bool notConst(EvalContext& context, SourceRange range) const;
-    bool noHierarchical(EvalContext& context, const Expression& expr) const;
+    [[nodiscard]] bool noHierarchical(EvalContext& context, const Expression& expr) const;
 
     bool checkArgCount(const ASTContext& context, bool isMethod, const Args& args,
                        SourceRange callRange, size_t min, size_t max) const;

scripts/diagnostics.txt

@@ -54,7 +54,7 @@ error ValueOutOfRange "{} is out of allowed range ({} to {})"
 error ExpectedVectorDigits "expected vector literal digits"
 warning real-underflow RealLiteralUnderflow "value of real literal is too small; minimum is {:.5e}"
 warning real-overflow RealLiteralOverflow "value of real literal is too large; maximum is {:.5e}"
-warning vector-overflow VectorLiteralOverflow "vector literal too large for the given number of bits"
+warning vector-overflow VectorLiteralOverflow "vector literal too large for the given number of bits ({} bits needed)"
 warning int-overflow SignedIntegerOverflow "signed integer literal overflows 32 bits, will be truncated to {}"
 
 subsystem Preprocessor

source/ast/Expression.cpp

@@ -388,7 +388,7 @@ std::tuple<const Expression*, const Type*> Expression::bindImplicitParam(
         if (lhsType->isIntegral()) {
             bitwidth_t bits = lhsType->getBitWidth();
             if (expr.isUnsizedInteger())
-                bits = 32;
+                bits = std::max(bits, 32u);
 
             // Keep the signed flag but force four state.
             auto flags = lhsType->getIntegralFlags();

source/ast/builtins/EnumMethods.cpp

@@ -29,8 +29,10 @@ class EnumFirstLastMethod : public SystemSubroutine {
 
     ConstantValue eval(EvalContext& context, const Args& args, SourceRange,
                        const CallExpression::SystemCallInfo&) const final {
+        if (!noHierarchical(context, *args[0]))
+            return nullptr;
+
         // Expression isn't actually evaluated here; we know the value to return at compile time.
-        noHierarchical(context, *args[0]);
         const EnumType& type = args[0]->type->getCanonicalType().as<EnumType>();
 
         auto range = type.values();
@@ -161,8 +163,10 @@ class EnumNumMethod : public SystemSubroutine {
 
     ConstantValue eval(EvalContext& context, const Args& args, SourceRange,
                        const CallExpression::SystemCallInfo&) const final {
+        if (!noHierarchical(context, *args[0]))
+            return nullptr;
+
         // Expression isn't actually evaluated here; we know the value to return at compile time.
-        noHierarchical(context, *args[0]);
         const EnumType& type = args[0]->type->getCanonicalType().as<EnumType>();
         return SVInt(32, (uint64_t)std::ranges::distance(type.values()), true);
     }

source/ast/builtins/QueryFuncs.cpp

@@ -47,7 +47,8 @@ class BitsFunction : public SystemSubroutine {
 
     ConstantValue eval(EvalContext& context, const Args& args, SourceRange,
                        const CallExpression::SystemCallInfo&) const final {
-        noHierarchical(context, *args[0]);
+        if (!noHierarchical(context, *args[0]))
+            return nullptr;
 
         size_t width;
         if (args[0]->type->isFixedSize()) {
@@ -90,7 +91,8 @@ class TypenameFunction : public SystemSubroutine {
 
     ConstantValue eval(EvalContext& context, const Args& args, SourceRange,
                        const CallExpression::SystemCallInfo&) const final {
-        noHierarchical(context, *args[0]);
+        if (!noHierarchical(context, *args[0]))
+            return nullptr;
 
         TypePrinter printer;
         printer.append(*args[0]->type);
@@ -120,7 +122,8 @@ class IsUnboundedFunction : public SystemSubroutine {
 
     ConstantValue eval(EvalContext& context, const Args& args, SourceRange range,
                        const CallExpression::SystemCallInfo&) const final {
-        noHierarchical(context, *args[0]);
+        if (!noHierarchical(context, *args[0]))
+            return nullptr;
 
         if (args[0]->type->isUnbounded())
             return SVInt(1, 1, false);
@@ -225,7 +228,8 @@ class ArrayQueryFunction : public SystemSubroutine {
     };
 
     DimResult getDim(EvalContext& context, const Args& args) const {
-        noHierarchical(context, *args[0]);
+        if (!noHierarchical(context, *args[0]))
+            return {};
 
         // If an index expression is provided, evaluate it. Otherwise default to 1.
         ConstantValue iv;
@@ -497,7 +501,8 @@ class ArrayDimensionFunction : public SystemSubroutine {
 
     ConstantValue eval(EvalContext& context, const Args& args, SourceRange,
                        const CallExpression::SystemCallInfo&) const final {
-        noHierarchical(context, *args[0]);
+        if (!noHierarchical(context, *args[0]))
+            return nullptr;
 
         // Count the number of dimensions by unwrapping arrays.
         uint64_t count = 0;

source/ast/expressions/AssignmentExpressions.cpp

@@ -112,8 +112,13 @@ void checkImplicitConversions(const ASTContext& context, const Type& sourceType,
     };
 
     auto expr = &op;
-    while (expr->kind == ExpressionKind::Conversion)
-        expr = &expr->as<ConversionExpression>().operand();
+    while (expr->kind == ExpressionKind::Conversion) {
+        auto& conv = expr->as<ConversionExpression>();
+        if (!conv.isImplicit())
+            break;
+
+        expr = &conv.operand();
+    }
 
     if (expr->kind == ExpressionKind::LValueReference)
         return;
@@ -140,14 +145,14 @@ void checkImplicitConversions(const ASTContext& context, const Type& sourceType,
             return;
         }
 
+        // Check to rule out false positives: try to eval as a constant.
+        // We'll ignore any constants, because they will get their own more
+        // fine grained warning during eval.
+        if (context.tryEval(op))
+            return;
+
         // Warn for sign conversions.
-        bool isKnownNotConst = false;
         if (lt.isSigned() != rt.isSigned()) {
-            if (context.tryEval(op))
-                return;
-
-            isKnownNotConst = true;
-
             // Comparisons get their own warning elsewhere.
             bool isComparison = false;
             if (parentExpr && parentExpr->kind == ExpressionKind::BinaryOp) {
@@ -201,20 +206,12 @@ void checkImplicitConversions(const ASTContext& context, const Type& sourceType,
         // effective and the actual width.
         SLANG_ASSERT(effective <= actualWidth);
         if (targetWidth < effective || targetWidth > actualWidth) {
-            // Final check to rule out false positives: try to eval as a constant.
-            // We'll ignore any constants, because as described above they
-            // will get their own more fine grained warning later during eval.
-            if (isKnownNotConst || !context.tryEval(op)) {
-                DiagCode code;
-                if (context.getInstance()) {
-                    code = targetWidth < effective ? diag::PortWidthTruncate
-                                                   : diag::PortWidthExpand;
-                }
-                else {
-                    code = targetWidth < effective ? diag::WidthTruncate : diag::WidthExpand;
-                }
-                context.addDiag(code, loc) << actualWidth << targetWidth << op.sourceRange;
-            }
+            DiagCode code;
+            if (context.getInstance())
+                code = targetWidth < effective ? diag::PortWidthTruncate : diag::PortWidthExpand;
+            else
+                code = targetWidth < effective ? diag::WidthTruncate : diag::WidthExpand;
+            context.addDiag(code, loc) << actualWidth << targetWidth << op.sourceRange;
         }
     }
 }

source/parsing/NumberParser.cpp

@@ -184,9 +184,14 @@ Token NumberParser::finishValue(Token firstToken, bool singleToken) {
         // Otherwise, optimize for this case by reusing the integer value already
         // computed by the token itself.
         if (!hasUnknown) {
+            bitwidth_t width = decimalValue.getBitWidth();
+            if (signFlag) {
+                width++;
+                decimalValue = decimalValue.resize(width);
+            }
+
             // If no size was specified, just return the value as-is. Otherwise,
             // resize it to match the desired size. Warn if that will truncate.
-            bitwidth_t width = decimalValue.getBitWidth();
             SVInt result;
             if (!sizeBits) {
                 // Unsized numbers are required to be at least 32 bits by the spec.
@@ -197,7 +202,7 @@ Token NumberParser::finishValue(Token firstToken, bool singleToken) {
             }
             else if (width != sizeBits) {
                 if (width > sizeBits)
-                    addDiag(diag::VectorLiteralOverflow, firstLocation);
+                    addDiag(diag::VectorLiteralOverflow, firstLocation) << width;
 
                 result = decimalValue.resize(sizeBits);
             }
@@ -240,6 +245,10 @@ Token NumberParser::finishValue(Token firstToken, bool singleToken) {
         if (!digits[0].isUnknown())
             bits += (bitwidth_t)std::bit_width(digits[0].value);
 
+        // Signed numbers need an extra bit for the sign.
+        if (signFlag)
+            bits++;
+
         if (bits > sizeBits) {
             if (sizeBits == 0) {
                 if (bits > SVInt::MAX_BITS) {
@@ -252,7 +261,7 @@ Token NumberParser::finishValue(Token firstToken, bool singleToken) {
             else {
                 // We should warn about overflow here, but the spec says it is valid and
                 // the literal gets truncated. Definitely a warning though.
-                addDiag(diag::VectorLiteralOverflow, firstLocation);
+                addDiag(diag::VectorLiteralOverflow, firstLocation) << bits;
             }
         }
     }

tests/unittests/ast/ExpressionTests.cpp

@@ -547,7 +547,7 @@ endmodule
 source:7:13: warning: implicit conversion from 'bit[34:0]' to 'int' changes value from 35'h22c6d1fba to 745349050 [-Wconstant-conversion]
     int i = 35'd123498234978234;
             ^~~~~~~~~~~~~~~~~~~
-source:7:17: warning: vector literal too large for the given number of bits [-Wvector-overflow]
+source:7:17: warning: vector literal too large for the given number of bits (47 bits needed) [-Wvector-overflow]
     int i = 35'd123498234978234;
                 ^
 source:8:13: error: size of vector literal cannot be zero
@@ -577,7 +577,7 @@ source:19:16: error: expected decimal digit
 source:20:16: error: expected hexadecimal digit
     int v = 'h g;
                ^
-source:21:17: warning: vector literal too large for the given number of bits [-Wvector-overflow]
+source:21:17: warning: vector literal too large for the given number of bits (4 bits needed) [-Wvector-overflow]
     int w = 3'h f;
                 ^
 source:22:15: error: expected vector literal digits
@@ -2956,3 +2956,18 @@ endclass
     compilation.addSyntaxTree(tree);
     NO_COMPILATION_ERRORS;
 }
+
+TEST_CASE("Type of unsized based literal") {
+    auto tree = SyntaxTree::fromText(R"(
+module m;
+    parameter p = 'd999999999999;
+endmodule
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+    NO_COMPILATION_ERRORS;
+
+    auto& p = compilation.getRoot().lookupName<ParameterSymbol>("m.p");
+    CHECK(p.getType().toString() == "logic[39:0]");
+}

tests/unittests/ast/WarningTests.cpp

@@ -613,13 +613,15 @@ TEST_CASE("Implicit conversions with constants") {
     auto tree = SyntaxTree::fromText(R"(
 module m;
     logic [9:0] a = 9000;
+    shortint b = -32769;
 endmodule
 )");
 
     Compilation compilation;
     compilation.addSyntaxTree(tree);
 
     auto& diags = compilation.getAllDiagnostics();
-    REQUIRE(diags.size() == 1);
+    REQUIRE(diags.size() == 2);
     CHECK(diags[0].code == diag::ConstantConversion);
+    CHECK(diags[1].code == diag::ConstantConversion);
 }

tests/unittests/parsing/ExpressionParsingTests.cpp

@@ -911,3 +911,19 @@ endmodule
     REQUIRE(diagnostics.size() == 1);
     CHECK(diagnostics[0].code == diag::AttributesNotAllowed);
 }
+
+TEST_CASE("Vector literal overflow parsing") {
+    auto& text = R"(
+shortint a = -16'sd32769;
+int b = -32'sd2147483649;
+int c = 32'd4294967297;
+int d = 9'so777;
+)";
+    parseCompilationUnit(text);
+
+    REQUIRE(diagnostics.size() == 4);
+    CHECK(diagnostics[0].code == diag::VectorLiteralOverflow);
+    CHECK(diagnostics[1].code == diag::VectorLiteralOverflow);
+    CHECK(diagnostics[2].code == diag::VectorLiteralOverflow);
+    CHECK(diagnostics[3].code == diag::VectorLiteralOverflow);
+}