diff --git a/include/slang/ast/SystemSubroutine.h b/include/slang/ast/SystemSubroutine.h index 4c13f1657..d9e7fec8a 100644 --- a/include/slang/ast/SystemSubroutine.h +++ b/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; diff --git a/scripts/diagnostics.txt b/scripts/diagnostics.txt index a1cb7665e..868d1e9e4 100644 --- a/scripts/diagnostics.txt +++ b/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 diff --git a/source/ast/Expression.cpp b/source/ast/Expression.cpp index f076a386d..64d2f4d9b 100644 --- a/source/ast/Expression.cpp +++ b/source/ast/Expression.cpp @@ -388,7 +388,7 @@ std::tuple 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(); diff --git a/source/ast/builtins/EnumMethods.cpp b/source/ast/builtins/EnumMethods.cpp index 3c7b959b7..05af36e48 100644 --- a/source/ast/builtins/EnumMethods.cpp +++ b/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(); 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(); return SVInt(32, (uint64_t)std::ranges::distance(type.values()), true); } diff --git a/source/ast/builtins/QueryFuncs.cpp b/source/ast/builtins/QueryFuncs.cpp index 07b83416c..856f875e5 100644 --- a/source/ast/builtins/QueryFuncs.cpp +++ b/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; diff --git a/source/ast/expressions/AssignmentExpressions.cpp b/source/ast/expressions/AssignmentExpressions.cpp index a2d68b890..72b8e9e34 100644 --- a/source/ast/expressions/AssignmentExpressions.cpp +++ b/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().operand(); + while (expr->kind == ExpressionKind::Conversion) { + auto& conv = expr->as(); + 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; } } } diff --git a/source/parsing/NumberParser.cpp b/source/parsing/NumberParser.cpp index 6e4ed6334..7e7dad1a9 100644 --- a/source/parsing/NumberParser.cpp +++ b/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; } } } diff --git a/tests/unittests/ast/ExpressionTests.cpp b/tests/unittests/ast/ExpressionTests.cpp index 63a0a279d..32e48a563 100644 --- a/tests/unittests/ast/ExpressionTests.cpp +++ b/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("m.p"); + CHECK(p.getType().toString() == "logic[39:0]"); +} diff --git a/tests/unittests/ast/WarningTests.cpp b/tests/unittests/ast/WarningTests.cpp index 2ec480850..afde66339 100644 --- a/tests/unittests/ast/WarningTests.cpp +++ b/tests/unittests/ast/WarningTests.cpp @@ -613,6 +613,7 @@ TEST_CASE("Implicit conversions with constants") { auto tree = SyntaxTree::fromText(R"( module m; logic [9:0] a = 9000; + shortint b = -32769; endmodule )"); @@ -620,6 +621,7 @@ endmodule 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); } diff --git a/tests/unittests/parsing/ExpressionParsingTests.cpp b/tests/unittests/parsing/ExpressionParsingTests.cpp index 8f376fa62..b2c3e5db8 100644 --- a/tests/unittests/parsing/ExpressionParsingTests.cpp +++ b/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); +}