Skip to content

Commit

Permalink
Suppress vector literal overflow warning for negated minimum integer
Browse files Browse the repository at this point in the history
  • Loading branch information
MikePopoloski committed Apr 6, 2024
1 parent 5412051 commit 0836f13
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 35 deletions.
6 changes: 4 additions & 2 deletions include/slang/parsing/NumberParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class NumberParser {

template<typename TStream, bool RequireSameLine = false>
IntResult parseInteger(TStream& stream) {
const bool isNegated = stream.getLastConsumed().kind == TokenKind::Minus;

Token sizeToken;
Token baseToken;

Expand Down Expand Up @@ -115,7 +117,7 @@ class NumberParser {
next = stream.peek();
} while (syntax::SyntaxFacts::isPossibleVectorDigit(next.kind) && next.trivia().empty());

return IntResult::vector(sizeToken, baseToken, finishValue(first, count == 1));
return IntResult::vector(sizeToken, baseToken, finishValue(first, count == 1, isNegated));
}

template<typename TStream>
Expand Down Expand Up @@ -172,7 +174,7 @@ class NumberParser {

void startVector(Token baseToken, Token sizeToken);
int append(Token token, bool isFirst);
Token finishValue(Token firstToken, bool singleToken);
Token finishValue(Token firstToken, bool singleToken, bool isNegated);
void addDigit(logic_t digit, int maxValue);
Diagnostic& addDiag(DiagCode code, SourceLocation location);
IntResult reportMissingDigits(Token sizeToken, Token baseToken, Token first);
Expand Down
1 change: 1 addition & 0 deletions include/slang/parsing/Preprocessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ class SLANG_EXPORT Preprocessor {
bool applyMacroOps(std::span<Token const> tokens, SmallVectorBase<Token>& dest);
void createBuiltInMacro(std::string_view name, int value, std::string_view valueStr = {});
void splitTokens(Token sourceToken, size_t offset, SmallVectorBase<Token>& results);
Token getLastConsumed() const { return lastConsumed; }

static bool isSameMacro(const syntax::DefineDirectiveSyntax& left,
const syntax::DefineDirectiveSyntax& right);
Expand Down
75 changes: 42 additions & 33 deletions source/parsing/NumberParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,50 +174,58 @@ int NumberParser::append(Token token, bool isFirst) {
return -1;
}

Token NumberParser::finishValue(Token firstToken, bool singleToken) {
Token NumberParser::finishValue(Token firstToken, bool singleToken, bool isNegated) {
auto createResult = [&](auto&& val) {
return Token(alloc, TokenKind::IntegerLiteral, firstToken.trivia(),
singleToken ? firstToken.rawText() : toStringView(text.copy(alloc)),
firstLocation, std::forward<decltype(val)>(val));
};

auto checkOverflow = [&](bitwidth_t computedWidth, const SVInt& value) {
// Special case to avoid the warning when we have a minus operator preceeding
// a min value signed negative integer, with no unknowns.
if (isNegated && signFlag && !hasUnknown && computedWidth == sizeBits + 1 &&
value.isNegative() && value.countOnes() == 1) {
return;
}

addDiag(diag::VectorLiteralOverflow, firstLocation) << computedWidth;
};

if (!valid)
return createResult(0);

if (literalBase == LiteralBase::Decimal) {
// If we added an x or z, fall through to the general handler below.
if (literalBase == LiteralBase::Decimal && !hasUnknown) {
// If we added an x or z, we will fall through to the general handler below.
// 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.
SVInt result;
if (!sizeBits) {
// Unsized numbers are required to be at least 32 bits by the spec.
if (width < 32)
result = decimalValue.resize(32);
else
result = std::move(decimalValue);
}
else if (width != sizeBits) {
if (width > sizeBits)
addDiag(diag::VectorLiteralOverflow, firstLocation) << width;
bitwidth_t width = decimalValue.getBitWidth();
if (signFlag) {
width++;
decimalValue = decimalValue.resize(width);
}

result = decimalValue.resize(sizeBits);
}
else {
// If no size was specified, just return the value as-is. Otherwise,
// resize it to match the desired size. Warn if that will truncate.
SVInt result;
if (!sizeBits) {
// Unsized numbers are required to be at least 32 bits by the spec.
if (width < 32)
result = decimalValue.resize(32);
else
result = std::move(decimalValue);
}

result.setSigned(signFlag);
return createResult(result);
}
else if (width != sizeBits) {
result = decimalValue.resize(sizeBits);
if (width > sizeBits)
checkOverflow(width, result);
}
else {
result = std::move(decimalValue);
}

result.setSigned(signFlag);
return createResult(std::move(result));
}

if (digits.empty()) {
Expand Down Expand Up @@ -264,9 +272,10 @@ Token NumberParser::finishValue(Token firstToken, bool singleToken) {
sizeBits = std::max(32u, bits);
}
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) << bits;
auto result = SVInt::fromDigits(sizeBits, literalBase, signFlag, hasUnknown,
digits);
checkOverflow(bits, result);
return createResult(std::move(result));
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions tests/unittests/parsing/ExpressionParsingTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -952,3 +952,12 @@ real s = 3e+_3;
CHECK(diagnostics[2].code == diag::MissingExponentDigits);
CHECK(diagnostics[3].code == diag::DigitsLeadingUnderscore);
}

TEST_CASE("Special case for literal overflow warning at int min") {
auto& text = R"(
wire [15:0] x = -16'sd32768;
wire [15:0] x = -16'sh8000;
)";
parseCompilationUnit(text);
CHECK_DIAGNOSTICS_EMPTY;
}

0 comments on commit 0836f13

Please sign in to comment.