Skip to content

Commit

Permalink
Report Wint-bool-conv for values used in assertion expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
MikePopoloski committed Mar 9, 2025
1 parent e62d383 commit 83a09ab
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 49 deletions.
2 changes: 1 addition & 1 deletion include/slang/analysis/AbstractFlowAnalysis.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class SLANG_EXPORT FlowAnalysisBase {
/// See background on lattice flow analysis:
/// https://en.wikipedia.org/wiki/Data-flow_analysis
template<typename TDerived, typename TState>
class SLANG_EXPORT AbstractFlowAnalysis : public FlowAnalysisBase {
class AbstractFlowAnalysis : public FlowAnalysisBase {
#define DERIVED *static_cast<TDerived*>(this)
public:
/// Run the analysis.
Expand Down
25 changes: 17 additions & 8 deletions source/ast/expressions/AssertionExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,25 @@ using namespace parsing;
using namespace syntax;

static const Expression& bindExpr(const ExpressionSyntax& syntax, const ASTContext& context,
bool allowInstances = false) {
bool allowInstances = false, bool isBoolean = true) {
auto& expr = Expression::bind(syntax, context, ASTFlags::AssertionExpr);
if (expr.bad())
return expr;

if (allowInstances && (expr.type->isSequenceType() || expr.type->isPropertyType()))
return expr;

if (!expr.type->isValidForSequence() && expr.kind != ExpressionKind::Dist) {
auto& comp = context.getCompilation();
context.addDiag(diag::AssertionExprType, expr.sourceRange) << *expr.type;
return *comp.emplace<InvalidExpression>(&expr, comp.getErrorType());
if (expr.kind != ExpressionKind::Dist) {
if (!expr.type->isValidForSequence()) {
auto& comp = context.getCompilation();
context.addDiag(diag::AssertionExprType, expr.sourceRange) << *expr.type;
return *comp.emplace<InvalidExpression>(&expr, comp.getErrorType());
}

// This should always return true since we checked isValidForSequence above,
// but we call it to get bool conversion warnings issued.
if (isBoolean)
context.requireBooleanConvertible(expr);
}

return expr;
Expand Down Expand Up @@ -1473,7 +1480,7 @@ void ConditionalAssertionExpr::serializeTo(ASTSerializer& serializer) const {
AssertionExpr& CaseAssertionExpr::fromSyntax(const CasePropertyExprSyntax& syntax,
const ASTContext& context) {
auto& comp = context.getCompilation();
auto& expr = bindExpr(*syntax.expr, context);
auto& expr = bindExpr(*syntax.expr, context, /* allowInstances */ false, /* isBoolean */ false);

const AssertionExpr* defCase = nullptr;
SmallVector<ItemGroup, 4> items;
Expand All @@ -1483,8 +1490,10 @@ AssertionExpr& CaseAssertionExpr::fromSyntax(const CasePropertyExprSyntax& synta
auto& body = AssertionExpr::bind(*sci.expr, context);

SmallVector<const Expression*> exprs;
for (auto es : sci.expressions)
exprs.push_back(&bindExpr(*es, context));
for (auto es : sci.expressions) {
exprs.push_back(
&bindExpr(*es, context, /* allowInstances */ false, /* isBoolean */ false));
}

items.push_back(ItemGroup{exprs.copy(comp), &body});
}
Expand Down
69 changes: 35 additions & 34 deletions tests/unittests/ast/AssertionTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ TEST_CASE("Concurrent assertion expressions") {
auto tree = SyntaxTree::fromText(R"(
module m;
string a;
logic b;
int c,d,e;
logic b,c,d,e;
foo: assert property (a);
assert property (a ##1 b ##[+] c ##[*] d ##[1:5] e);
Expand Down Expand Up @@ -95,7 +94,7 @@ endfunction
module m;
int a[];
int b;
logic b;
chandle c;
C d = new;
logic o;
Expand Down Expand Up @@ -156,16 +155,16 @@ module m;
assert property (n.a(3));
assert property (b);
int c, d;
logic c, d;
sequence b;
##1 c ##1 d;
endsequence
endmodule
module n;
int c, d;
logic c, d;
property a(int i, foo = 1);
##1 c ##1 d ##1 i;
##1 c ##1 d ##1 i > 0;
endproperty
endmodule
)");
Expand Down Expand Up @@ -206,8 +205,8 @@ TEST_CASE("Assertion instance arg type checking") {
auto tree = SyntaxTree::fromText(R"(
module m;
int foo[];
assert property (a(1, 1 iff 2, foo, 1 and 2));
assert property (a(1 iff 2, 1, 1));
assert property (a(1, 1 iff 1, foo, 1 and 1));
assert property (a(1 iff 1, 1, 1));
assert property (a(1, 1, foo[*]));
int e;
Expand Down Expand Up @@ -241,19 +240,19 @@ module m;
assert property (s1($, 5));
assert property (s1(bar, 9.2));
assert property (s3(9));
assert property (s4(1 and 2));
assert property (s3(1));
assert property (s4(1 and 1));
sequence s1(a, int b);
s2(a) ##1 bar[b:0];
s2(a) ##1 bar[b:0] > 0;
endsequence
sequence s2(foo);
1 ##[0:foo] 2 ##1 foo;
1 ##[0:foo] 1 ##1 foo > 0;
endsequence
sequence s3(sequence a);
1 ##[0:a] 2;
1 ##[0:a] 1;
endsequence
sequence s4(foo);
Expand All @@ -278,7 +277,7 @@ TEST_CASE("More complex sequence arg expansion") {
auto tree = SyntaxTree::fromText(R"(
module m;
sequence s1;
int foo;
logic foo;
s2(s2(foo));
endsequence
Expand Down Expand Up @@ -347,13 +346,13 @@ module m;
int baz;
sequence s2;
int j, k = j;
first_match(j, !j, j + k, baz = 1, baz++);
first_match(j > 0, !j, j + k, baz = 1, baz++);
endsequence
typedef int Foo;
property p;
Foo u, v;
s(u) and v and s2;
s(u) and v > 0 and s2;
endproperty
assert property (p);
Expand Down Expand Up @@ -395,7 +394,7 @@ TEST_CASE("Sequence triggered method") {
module m;
wire clk, ready;
sequence e1;
@(posedge clk) $rose(ready) ##1 1 ##1 2;
@(posedge clk) $rose(ready) ##1 1 ##1 1;
endsequence
sequence rule;
Expand Down Expand Up @@ -468,13 +467,14 @@ module m;
endsequence
sequence s5(a, event b, sequence c);
a ##1 c ##1 @b 1;
a > 0 ##1 c ##1 @b 1;
endsequence
event e;
sequence s6;
int i;
s5(i + 1, e, i + 1).triggered ##1 s5(i, e, i).triggered;
logic j;
s5(i + 1, e, i + 1 > 0).triggered ##1 s5(i, e, j).triggered;
endsequence
endmodule
)");
Expand Down Expand Up @@ -848,7 +848,8 @@ endmodule
TEST_CASE("Local var formal arg") {
auto tree = SyntaxTree::fromText(R"(
module m;
int a,b,c;
logic a;
int b,c;
int data_in, data_out;
sequence sub_seq2(local inout int lv);
Expand All @@ -858,7 +859,7 @@ module m;
sequence seq2;
int v1;
(c, v1 = data_in)
(c > 0, v1 = data_in)
##1 sub_seq2(v1)
##1 (data_out == v1);
endsequence
Expand Down Expand Up @@ -925,7 +926,7 @@ endmodule
TEST_CASE("Match items + empty match") {
auto tree = SyntaxTree::fromText(R"(
module m;
int a,b,c;
logic a,b,c;
sequence s;
int x,e;
a ##1 (b[*0:1], x = e) ##1 c[*];
Expand Down Expand Up @@ -1208,7 +1209,7 @@ TEST_CASE("$past in $bits regress GH #509") {
module top;
logic clk, reset, a, b, c;
assert property(@(posedge clk) disable iff (reset)
a |-> {500-$bits($past(b)){1'b1}});
a |-> {500-$bits($past(b)){1'b1}} != 0);
endmodule
)");

Expand Down Expand Up @@ -1761,9 +1762,9 @@ package p;
checker d(r);
string a;
int b,c,d,e;
logic b,c,d,e;
assert property (a ##1 b ##[+] c ##[*] d ##[1:5] e + r);
assert property (a ##1 b ##[+] c ##[*] d ##[1:5] e & r);
endchecker
endpackage
Expand Down Expand Up @@ -1797,7 +1798,7 @@ source:4:14: note: expanded here
d d1(q);
^
source:11:62: note: expanded here
assert property (a ##1 b ##[+] c ##[*] d ##[1:5] e + r);
assert property (a ##1 b ##[+] c ##[*] d ##[1:5] e & r);
^
source:25:21: error: invalid operands to binary expression ('queue of int' and 'int')
int j = foo + r;
Expand Down Expand Up @@ -2380,7 +2381,7 @@ TEST_CASE("Sequence nondegeneracy tests 1") {
module top;
string a;
logic b;
int c, d, e;
logic c, d, e;
assert property ({});
endmodule
)",
Expand Down Expand Up @@ -2429,7 +2430,7 @@ module top(a, b, e);
input a;
input b;
input e;
int c, d;
logic c, d;
logic clk;
property p;
Expand Down Expand Up @@ -2458,7 +2459,7 @@ endmodule
};

test("!(2'b01 - 2'b01 + 3'b010 - 3'b010 + 4'b0010)", diag::SeqNoMatch);
test("2'b01 - 2'b01 + 3'b010 - 3'b010 + 4'b0010");
test("(2'b01 - 2'b01 + 3'b010 - 3'b010 + 4'b0010) > 0");
test("a[*0] |-> b", diag::SeqOnlyEmpty);
test("a[*1] |-> b");
test("1'b1 ##1 b");
Expand Down Expand Up @@ -2580,8 +2581,8 @@ endmodule
TEST_CASE("Sequence nondegeneracy tests 4") {
auto tree = SyntaxTree::fromText(R"(
module top;
int c, d;
property p(int i, foo = 1);
logic c, d;
property p(logic i, foo = 1);
##1 c ##1 d ##1 i; // may be legal or not - depends on value of `i`
endproperty
Expand All @@ -2603,13 +2604,13 @@ TEST_CASE("Sequence nondegeneracy tests 5") {
module top;
logic clk, reset, a, b;
assert property(@(posedge clk) disable iff (reset)
a |-> {500-$bits($past(b)){1'b0}}); // illegal
a |-> {500-$bits($past(b)){1'b0}} != 0); // illegal
assert property(@(posedge clk) disable iff (reset)
{3 - 2{3'b111}}); // legal
{3 - 2{3'b111}} != 0); // legal
assert property(@(posedge clk) disable iff (reset)
{500 - $bits(b){1'b1}}); // legal
{500 - $bits(b){1'b1}} != 0); // legal
endmodule
)");

Expand Down
2 changes: 1 addition & 1 deletion tests/unittests/ast/CoverTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ module m;
int b,c;
property p1;
int x;
@(posedge clk)(a, x = b) ##1 (c, cg1.sample(a, x));
@(posedge clk)(a, x = b) ##1 (c > 0, cg1.sample(a, x));
endproperty : p1
c1: cover property (p1);
Expand Down
4 changes: 2 additions & 2 deletions tests/unittests/ast/ParameterTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,14 @@ interface width_checker #(parameter min_cks = 1, parameter max_cks = 1)
if ($isunbounded(max_cks)) begin
property width;
@(posedge clk)
(reset_n && $rose(expr)) |-> (expr [*min_cks]);
(reset_n && $rose(expr)) |-> (expr[0] [*min_cks]);
endproperty
a2: assert property (width);
end
else begin
property width;
@(posedge clk)
(reset_n && $rose(expr)) |-> (expr[*min_cks:max_cks])
(reset_n && $rose(expr)) |-> (expr[0][*min_cks:max_cks])
##1 (!expr);
endproperty
a2: assert property (width);
Expand Down
2 changes: 1 addition & 1 deletion tests/unittests/ast/PortTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ interface A_Bus( input logic clk );
input gnt;
output req, addr;
inout data;
property p1; gnt ##[1:3] data; endproperty
property p1; gnt ##[1:3] data > 0; endproperty
endclocking
modport DUT ( input clk, req, addr,
Expand Down
2 changes: 1 addition & 1 deletion tests/unittests/ast/SystemFuncTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ module m;
sequence s;
int i;
$past(i);
$past(i) > 0;
endsequence
endmodule
Expand Down
2 changes: 1 addition & 1 deletion tests/unittests/ast/TypeTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1242,7 +1242,7 @@ interface A_Bus( input logic clk );
input gnt;
output req, addr;
inout data;
property p1; gnt ##[1:3] data; endproperty
property p1; gnt ##[1:3] data > 0; endproperty
endclocking
modport DUT ( input clk, req, addr,
Expand Down

0 comments on commit 83a09ab

Please sign in to comment.