Add new warning -Wunsigned-arith-shift

scripts/diagnostics.txt

@@ -741,6 +741,7 @@ warning sign-conversion SignConversion "implicit conversion changes signedness f
 warning float-bool-conv FloatBoolConv "implicit conversion from floating point type {} to boolean value"
 warning int-bool-conv IntBoolConv "implicit conversion from {} to boolean value"
 warning useless-cast UselessCast "useless cast from {} to the same type"
+warning unsigned-arith-shift UnsignedArithShift "arithmetic right shift of unsigned type {} will always shift in zeros (use logical shift if that is intended)"
 
 subsystem Statements
 error ReturnNotInSubroutine "return statement is only valid inside task and function blocks"
@@ -1107,7 +1108,7 @@ group default = { real-underflow real-overflow vector-overflow int-overflow unco
 group extra = { empty-member empty-stmt dup-import pointless-void-cast case-gen-none case-gen-dup
                 unused-result format-real ignored-slice task-ignored width-trunc dup-attr event-const
                 ineffective-sign port-width-trunc constant-conversion float-bool-conv int-bool-conv
-                useless-cast }
+                useless-cast unsigned-arith-shift }
 
 group pedantic = { empty-pattern implicit-net-port nonstandard-escape-code nonstandard-generate format-multibit-strength
                    nonstandard-sys-func nonstandard-foreach nonstandard-dist }

scripts/warning_docs.txt

@@ -1188,3 +1188,13 @@ endconfig
 module top;
 endmodule
 ```
+
+-Wunsigned-arith-shift
+An arithmetic right shift of an unsigned integral type will always shift in zeros, which can be
+confusing since typically this operator is used to shift in the value of the most significant bit.
+If this is intended behavior use the logical right shift operator which does the same.
+```
+function bit[31:0] f1(int unsigned i);
+    return i >>> 5;
+endfunction
+```

source/ast/expressions/OperatorExpressions.cpp

@@ -933,6 +933,8 @@ bool BinaryExpression::propagateType(const ASTContext& context, const Type& newT
             // Only the left hand side gets propagated; the rhs is self determined.
             type = &newType;
             contextDetermined(context, left_, this, newType, opRange);
+            if (op == BinaryOperator::ArithmeticShiftRight && !type->isSigned())
+                context.addDiag(diag::UnsignedArithShift, left_->sourceRange) << *type;
             return true;
     }
     SLANG_UNREACHABLE;

tests/unittests/ast/WarningTests.cpp

@@ -818,7 +818,7 @@ endfunction
 
 function automatic int f2(int i);
     int unsigned j;
-    j += (i >>> 31);
+    j += (i + 31);
     return j;
 endfunction
 )");
@@ -827,8 +827,9 @@ endfunction
     compilation.addSyntaxTree(tree);
 
     auto& diags = compilation.getAllDiagnostics();
-    REQUIRE(diags.size() == 3);
+    REQUIRE(diags.size() == 4);
     CHECK(diags[0].code == diag::SignConversion);
-    CHECK(diags[1].code == diag::SignConversion);
+    CHECK(diags[1].code == diag::UnsignedArithShift);
     CHECK(diags[2].code == diag::SignConversion);
+    CHECK(diags[3].code == diag::SignConversion);
 }