misc: reduce max-line-length ruff setting to 130 [NFC] (#3869)

I don't know if we'll ever get to 88, but it feels like it would be good to get the line lengths a little more under control. In this PR, I update the line length, ignore all test files, ignore the line length limit on all assembly format strings, reflow some doc strings, and split up some inline strings in the code. After this change, there are 840 errors when I set the line length to 88, and 313 if I set it to 100. I think without some sort of AI "fix everything" button it's a waste of time to go shorter, but our current setting of 300 definitely seems to big. This PR contains about 60 whitespace changes.
xdslproject · Feb 10, 2025 · 317bd94 · 317bd94
1 parent 23d2a0e
commit 317bd94
Show file tree

Hide file tree

Showing 51 changed files with 243 additions and 141 deletions.
diff --git a/docs/Toy/Toy_Ch0.ipynb b/docs/Toy/Toy_Ch0.ipynb
@@ -79,8 +79,11 @@
    "metadata": {},
    "outputs": [],
    "source": [
-    "# Uncomment the following line to execute\n",
-    "# !python -m toy examples/interpret.toy --emit=scf --ir | xdsl-opt -p printf-to-llvm | mlir-opt --test-lower-to-llvm | mlir-cpu-runner --entry-point-result=void"
+    "# Uncomment the following lines to execute\n",
+    "# !python -m toy examples/interpret.toy --emit=scf --ir \\\n",
+    "# | xdsl-opt -p printf-to-llvm \\\n",
+    "# | mlir-opt --test-lower-to-llvm \\\n",
+    "# | mlir-cpu-runner --entry-point-result=void"
    ]
   }
  ],

diff --git a/pyproject.toml b/pyproject.toml
@@ -105,7 +105,7 @@ ignore = [
 # TODO: remove this setting and go to default line length of 88
 # Removing this line length results in a large number of errors for doc strings, ideally
 # these should be ignored separately.
-max-line-length = 300
+max-line-length = 130
 
 [tool.ruff.lint.flake8-tidy-imports.banned-api]
 "xdsl.dialects.utils.fast_math".msg = "Use xdsl.dialects.utils instead"
@@ -128,8 +128,7 @@ max-line-length = 300
 [tool.ruff.lint.per-file-ignores]
 "versioneer.py" = ["ALL"]
 "tests/test_declarative_assembly_format.py" = ["F811"]
-"tests/filecheck/frontend/programs/invalid.py" = ["F811", "F841"]
-"tests/filecheck/frontend/dialects/invalid.py" = ["F811"]
+"**/filecheck/*" = ["F811", "F841", "E501"]
 "docs/xdsl-introduction.ipynb" = ["ALL"]
 "docs/tutorial.ipynb" = ["ALL"]
 "docs/Toy/Toy_Ch3.ipynb" = ["E402"]
@@ -142,6 +141,7 @@ max-line-length = 300
 "**/__marimo__/*" = ["ALL"]
 "_version.py" = ["ALL"]
 "__init__.py" = ["F403"]
+"tests/*" = ["E501"]
 
 [tool.ruff.lint.mccabe]
 max-complexity = 10

diff --git a/tests/irdl/test_operation_definition.py b/tests/irdl/test_operation_definition.py
@@ -852,7 +852,9 @@ class OptionlessMultipleVarOp(IRDLOperation):
 def test_no_multiple_var_option():
     with pytest.raises(
         PyRDLOpDefinitionError,
-        match="Operation test.multiple_var_op defines more than two variadic operands, but do not define any of SameVariadicOperandSize or AttrSizedOperandSegments PyRDL options.",
+        match="Operation test.multiple_var_op defines more than two variadic operands, "
+        "but do not define any of SameVariadicOperandSize or AttrSizedOperandSegments "
+        "PyRDL options.",
     ):
         irdl_op_definition(OptionlessMultipleVarOp)
 

diff --git a/tests/test_pass_to_arg_and_types_str.py b/tests/test_pass_to_arg_and_types_str.py
@@ -56,7 +56,9 @@ def apply(self, ctx: MLContext, op: builtin.ModuleOp) -> None:
     "str_arg, pass_arg",
     [
         (
-            """number=int|float single_number=int int_list=tuple[int, ...] non_init_thing=int str_thing=str nullable_str=str|None literal=no optional_bool=false""",
+            "number=int|float single_number=int int_list=tuple[int, ...] "
+            "non_init_thing=int str_thing=str nullable_str=str|None literal=no "
+            "optional_bool=false",
             CustomPass,
         ),
         ("", EmptyPass),

diff --git a/tests/test_pyrdl.py b/tests/test_pyrdl.py
@@ -225,12 +225,12 @@ def test_allof_verify_multiple_failures():
     """
     constraint = AllOf((LessThan(5), GreaterThan(8)))
 
-    with pytest.raises(VerifyException) as e:
+    with pytest.raises(
+        VerifyException,
+        match=f"The following constraints were not satisfied:\n{IntData(7)} should "
+        f"hold a value less than 5\n{IntData(7)} should hold a value greater than 8",
+    ):
         constraint.verify(IntData(7), ConstraintContext())
-    assert (
-        e.value.args[0]
-        == f"The following constraints were not satisfied:\n{IntData(7)} should hold a value less than 5\n{IntData(7)} should hold a value greater than 8"
-    )
 
 
 def test_param_attr_verify():

diff --git a/xdsl/backend/csl/print_csl.py b/xdsl/backend/csl/print_csl.py
@@ -453,7 +453,7 @@ def attribute_value_to_str(self, attr: Attribute) -> str:
             case StringAttr() as s:
                 return f'"{s.data}"'
             case DenseIntOrFPElementsAttr(type=typ):
-                return f"{self.mlir_type_to_csl_type(typ)} {{ {', '.join(self.attribute_value_to_str(a) for a in attr.iter_attrs())} }}"
+                return f"{self.mlir_type_to_csl_type(typ)} {{ {', '.join(self.attribute_value_to_str(a) for a in attr.iter_attrs())} }}"  # noqa: E501
             case _:
                 return f"<!unknown value {attr}>"
 
@@ -809,21 +809,21 @@ def print_block(self, body: Block):
                     op=input_dsd, base_addr=base_addr, result=result
                 ):
                     self.print(
-                        f"{self._var_use(result)} = @set_dsd_base_addr({self._get_variable_name_for(input_dsd)}, {self._get_variable_name_for(base_addr)});"
+                        f"{self._var_use(result)} = @set_dsd_base_addr({self._get_variable_name_for(input_dsd)}, {self._get_variable_name_for(base_addr)});"  # noqa: E501
                     )
                 case csl.IncrementDsdOffsetOp(
                     op=input_dsd, offset=offset, elem_type=elem_type, result=result
                 ):
                     self.print(
-                        f"{self._var_use(result)} = @increment_dsd_offset({self._get_variable_name_for(input_dsd)}, {self._get_variable_name_for(offset)}, {self.mlir_type_to_csl_type(elem_type)});"
+                        f"{self._var_use(result)} = @increment_dsd_offset({self._get_variable_name_for(input_dsd)}, {self._get_variable_name_for(offset)}, {self.mlir_type_to_csl_type(elem_type)});"  # noqa: E501
                     )
                 case csl.SetDsdLengthOp(op=input_dsd, length=length, result=result):
                     self.print(
-                        f"{self._var_use(result)} = @set_dsd_length({self._get_variable_name_for(input_dsd)}, {self._get_variable_name_for(length)});"
+                        f"{self._var_use(result)} = @set_dsd_length({self._get_variable_name_for(input_dsd)}, {self._get_variable_name_for(length)});"  # noqa: E501
                     )
                 case csl.SetDsdStrideOp(op=input_dsd, stride=stride, result=result):
                     self.print(
-                        f"{self._var_use(result)} = @set_dsd_stride({self._get_variable_name_for(input_dsd)}, {self._get_variable_name_for(stride)});"
+                        f"{self._var_use(result)} = @set_dsd_stride({self._get_variable_name_for(input_dsd)}, {self._get_variable_name_for(stride)});"  # noqa: E501
                     )
                 case csl.BuiltinDsdOp(ops=ops):
                     self.print(

diff --git a/xdsl/dialects/affine.py b/xdsl/dialects/affine.py
@@ -65,7 +65,10 @@ def __init__(self, map_operands: Sequence[SSAValue], affine_map: AffineMapAttr):
     def verify_(self) -> None:
         if len(self.mapOperands) != self.map.data.num_dims + self.map.data.num_symbols:
             raise VerifyException(
-                f"{self.name} expects {self.map.data.num_dims + self.map.data.num_symbols} operands, but got {len(self.mapOperands)}. The number of map operands must match the sum of the dimensions and symbols of its map."
+                f"{self.name} expects "
+                f"{self.map.data.num_dims + self.map.data.num_symbols} operands, but "
+                f"got {len(self.mapOperands)}. The number of map operands must match "
+                "the sum of the dimensions and symbols of its map."
             )
         if len(self.map.data.results) != 1:
             raise VerifyException("affine.apply expects a unidimensional map.")
@@ -346,7 +349,10 @@ class MinOp(IRDLOperation):
     def verify_(self) -> None:
         if len(self.operands) != self.map.data.num_dims + self.map.data.num_symbols:
             raise VerifyException(
-                f"{self.name} expects {self.map.data.num_dims + self.map.data.num_symbols} operands, but got {len(self.operands)}. The number of map operands must match the sum of the dimensions and symbols of its map."
+                f"{self.name} expects "
+                f"{self.map.data.num_dims + self.map.data.num_symbols} "
+                "operands, but got {len(self.operands)}. The number of map operands "
+                "must match the sum of the dimensions and symbols of its map."
             )
 
 

diff --git a/xdsl/dialects/arith.py b/xdsl/dialects/arith.py
@@ -1241,7 +1241,8 @@ def verify_(self) -> None:
         # exactly one of input or result must be of IndexType, no more, no less.
         if not isinstance(self.input.type, it) ^ isinstance(self.result.type, it):
             raise VerifyException(
-                f"'arith.index_cast' op operand type '{self.input.type}' and result type '{self.input.type}' are cast incompatible"
+                f"'arith.index_cast' op operand type '{self.input.type}' and result "
+                f"type '{self.input.type}' are cast incompatible"
             )
 
 

diff --git a/xdsl/dialects/bufferization.py b/xdsl/dialects/bufferization.py
@@ -120,7 +120,7 @@ class AllocTensorOp(IRDLOperation):
 
     irdl_options = [AttrSizedOperandSegments(as_property=True)]
 
-    assembly_format = "`(` $dynamic_sizes `)` ( `copy` `(` $copy^ `)`)? (`size_hint` `=` $size_hint^)? attr-dict `:` type($tensor)"
+    assembly_format = "`(` $dynamic_sizes `)` ( `copy` `(` $copy^ `)`)? (`size_hint` `=` $size_hint^)? attr-dict `:` type($tensor)"  # noqa E501
 
     def __init__(
         self,
@@ -216,7 +216,7 @@ class MaterializeInDestinationOp(IRDLOperation):
     restrict = opt_prop_def(UnitAttr)
     writable = opt_prop_def(UnitAttr)
 
-    assembly_format = "$source `in` (`restrict` $restrict^)? (`writable` $writable^)? $dest attr-dict `:` functional-type(operands, results)"
+    assembly_format = "$source `in` (`restrict` $restrict^)? (`writable` $writable^)? $dest attr-dict `:` functional-type(operands, results)"  # noqa: E501
 
 
 Bufferization = Dialect(

diff --git a/xdsl/dialects/csl/csl_stencil.py b/xdsl/dialects/csl/csl_stencil.py
@@ -357,7 +357,8 @@ def verify_(self) -> None:
         for operand, argument in zip(self.operands, op_args):
             if operand.type != argument.type:
                 raise VerifyException(
-                    f"Expected argument type of {type(self)} to match operand type, got {argument.type} != {operand.type} at index {argument.index}"
+                    f"Expected argument type of {type(self)} to match operand type, "
+                    f"got {argument.type} != {operand.type} at index {argument.index}"
                 )
 
         # typecheck required (only) block arguments
@@ -569,21 +570,26 @@ def verify_(self) -> None:
             if isa(self.op.type, memref.MemRefType[Attribute]):
                 if not self.result.type == self.op.type:
                     raise VerifyException(
-                        f"{type(self)} access to own data requires{self.op.type} but found {self.result.type}"
+                        f"{type(self)} access to own data requires{self.op.type} but "
+                        f"found {self.result.type}"
                     )
             elif isattr(self.op.type, stencil.StencilTypeConstr):
                 if not self.result.type == self.op.type.get_element_type():
                     raise VerifyException(
-                        f"{type(self)} access to own data requires{self.op.type.get_element_type()} but found {self.result.type}"
+                        f"{type(self)} access to own data requires "
+                        f"{self.op.type.get_element_type()} but found "
+                        f"{self.result.type}"
                     )
             else:
                 raise VerifyException(
-                    f"{type(self)} access to own data requires type stencil.StencilType or memref.MemRefType but found {self.op.type}"
+                    f"{type(self)} access to own data requires type "
+                    f"stencil.StencilType or memref.MemRefType but found {self.op.type}"
                 )
         else:
             if not isattr(self.op.type, AnyTensorTypeConstr | MemRefType.constr()):
                 raise VerifyException(
-                    f"{type(self)} access to neighbor data requires type memref.MemRefType or TensorType but found {self.op.type}"
+                    f"{type(self)} access to neighbor data requires type "
+                    f"memref.MemRefType or TensorType but found {self.op.type}"
                 )
 
         # As promised by HasAncestor(ApplyOp)

diff --git a/xdsl/dialects/experimental/air.py b/xdsl/dialects/experimental/air.py
@@ -138,7 +138,7 @@ def __init__(
             ],
         )
 
-    assembly_format = "(`async` `[` $async_dependencies^ `]`)? $chan_name `[` $indices `]` `(` $dst `[` $dst_offsets `]``[` $dst_sizes `]``[` $dst_strides `]` `)` attr-dict `:` `(` type($dst) `)`"
+    assembly_format = "(`async` `[` $async_dependencies^ `]`)? $chan_name `[` $indices `]` `(` $dst `[` $dst_offsets `]``[` $dst_sizes `]``[` $dst_strides `]` `)` attr-dict `:` `(` type($dst) `)`"  # noqa: E501
 
 
 @irdl_op_definition
@@ -181,7 +181,7 @@ def __init__(
             result_types=[AsyncTokenAttr()],
         )
 
-    assembly_format = "(`async` `[` $async_dependencies^ `]`)? $chan_name `[` $indices `]` `(` $src `[` $src_offsets `]``[` $src_sizes `]``[` $src_strides `]` `)` attr-dict `:` `(` type($src) `)`"
+    assembly_format = "(`async` `[` $async_dependencies^ `]`)? $chan_name `[` $indices `]` `(` $src `[` $src_offsets `]``[` $src_sizes `]``[` $src_strides `]` `)` attr-dict `:` `(` type($src) `)`"  # noqa: E501
 
 
 @irdl_op_definition
@@ -284,7 +284,7 @@ def __init__(
             result_types=[AsyncTokenAttr()],
         )
 
-    assembly_format = "(`async` $async_dependencies^)? `(` $dst `[` $dst_offsets `]``[` $dst_sizes `]``[` $dst_strides `]` `,` $src `[` $src_offsets `]``[` $src_sizes `]``[` $src_strides `]` `)`  attr-dict `:` `(` type($dst) `,` type($src) `)`"
+    assembly_format = "(`async` $async_dependencies^)? `(` $dst `[` $dst_offsets `]``[` $dst_sizes `]``[` $dst_strides `]` `,` $src `[` $src_offsets `]``[` $src_sizes `]``[` $src_strides `]` `)`  attr-dict `:` `(` type($dst) `,` type($src) `)`"  # noqa: E501
 
 
 @irdl_op_definition

diff --git a/xdsl/dialects/experimental/hlfir.py b/xdsl/dialects/experimental/hlfir.py
@@ -142,7 +142,7 @@ class DeclareOp(IRDLOperation):
       %3 = hfir.declare %arg0(%2) typeparams %1 {uniq_name = "c"} (fir.ref<!fir.array<?x?x!fir.char<1,?>>>, fir.shapeshift<2>, index) -> (fir.box<!fir.array<?x?x!fir.char<1,?>>>, fir.ref<!fir.array<?x?x!fir.char<1,?>>>)
       // ... uses %3#0 as "c"
     }
-    """
+    """  # noqa: E501
 
     name = "hlfir.declare"
     memref = operand_def()
@@ -1027,7 +1027,7 @@ class CharExtremumOp(IRDLOperation):
     minimum or maximum number of characters. Example:
 
     %0 = hlfir.char_extremum min, %arg0, %arg1 : (!fir.ref<!fir.char<1,10>>, !fir.ref<!fir.char<1,20>>) -> !hlfir.expr<!fir.char<1,10>>
-    """
+    """  # noqa E501
 
     name = "hlfir.char_extremum"
     predicate = operand_def()

diff --git a/xdsl/dialects/func.py b/xdsl/dialects/func.py
@@ -86,15 +86,17 @@ def verify(self, op: Operation) -> None:
         ):
             if found_operand != operand:
                 raise VerifyException(
-                    f"operand type mismatch: expected operand type {found_operand}, but provided {operand} for operand number {idx}"
+                    f"operand type mismatch: expected operand type {found_operand}, "
+                    f"but provided {operand} for operand number {idx}"
                 )
 
         for idx, (found_res, res) in enumerate(
             zip(found_callee.function_type.outputs, op.result_types)
         ):
             if found_res != res:
                 raise VerifyException(
-                    f"result type mismatch: expected result type {found_res}, but provided {res} for result number {idx}"
+                    f"result type mismatch: expected result type {found_res}, but "
+                    f"provided {res} for result number {idx}"
                 )
 
         return

diff --git a/xdsl/dialects/memref_stream.py b/xdsl/dialects/memref_stream.py
@@ -471,18 +471,19 @@ class GenericOp(IRDLOperation):
     """
     inits = var_operand_def()
     """
-    Initial values for outputs. The outputs are at corresponding `init_indices`. The inits
-    may be set only for the imperfectly nested form.
+    Initial values for outputs. The outputs are at corresponding `init_indices`. The
+    inits may be set only for the imperfectly nested form.
     """
     indexing_maps = prop_def(ArrayAttr[AffineMapAttr])
     """
     Stride patterns that define the order of the input and output streams.
-    Like in linalg.generic, the indexing maps corresponding to inputs are followed by the
-    indexing maps for the outputs.
+    Like in linalg.generic, the indexing maps corresponding to inputs are followed by
+    the indexing maps for the outputs.
     """
     bounds = prop_def(ArrayAttr[IntegerAttr[IndexType]])
     """
-    The bounds of the iteration space, from the outermost loop inwards. All indexing maps must have the same number of dimensions as the length of `bounds`.
+    The bounds of the iteration space, from the outermost loop inwards.
+    All indexing maps must have the same number of dimensions as the length of `bounds`.
     """
 
     iterator_types = prop_def(ArrayAttr[IteratorTypeAttr])

diff --git a/xdsl/dialects/ptr.py b/xdsl/dialects/ptr.py
@@ -1,12 +1,16 @@
-########
-#  This is a port of the 'ptr' dialect proposed in this thread https://discourse.llvm.org/t/rfc-ptr-dialect-modularizing-ptr-ops-in-the-llvm-dialect/75142/21
-#  Main parts of the dialect has already been agreed upon but only the basic datatype is implemented upstream.
-#  When the upstream implementation is merged we should fix our implementation to be consistent with the mlir version.
-#  Current diviations:
-#  (1) Name of the dialect should be changed: ptr_xdsl -> ptr. We currently chose another name to avoid conflicts when loading our dialect to mlir-opt.
-#  (2) There is no ptr.to_ptr operation currently proposed, but there is a memref.to_ptr. We do this so that we can feed the results of convert-memref-to-ptr pass without any conflict.
-########
-
+"""
+This is a port of the 'ptr' dialect proposed in this thread https://discourse.llvm.org/t/rfc-ptr-dialect-modularizing-ptr-ops-in-the-llvm-dialect/75142/21
+Main parts of the dialect has already been agreed upon but only the basic datatype is
+implemented upstream.
+When the upstream implementation is merged we should fix our implementation to be
+consistent with the mlir version.
+Current deviations:
+ 1. Name of the dialect should be changed: ptr_xdsl -> ptr. We currently chose another
+ name to avoid conflicts when loading our dialect to mlir-opt.
+ 2. There is no ptr.to_ptr operation currently proposed, but there is a memref.to_ptr.
+ We do this so that we can feed the results of convert-memref-to-ptr pass without any
+ conflict.
+"""
 
 from xdsl.dialects.builtin import AnyAttr, IntegerAttrTypeConstr, MemRefType, UnitAttr
 from xdsl.ir import Dialect, ParametrizedAttribute, TypeAttribute
@@ -59,7 +63,7 @@ class StoreOp(IRDLOperation):
     syncscope = opt_prop_def(UnitAttr)
     ordering = opt_prop_def(UnitAttr)
 
-    assembly_format = "(`volatile` $volatile^)? $value `,` $addr (`atomic` (`syncscope` `(` $syncscope^ `)`)? $ordering^)? attr-dict `:` type($value) `,` type($addr)"
+    assembly_format = "(`volatile` $volatile^)? $value `,` $addr (`atomic` (`syncscope` `(` $syncscope^ `)`)? $ordering^)? attr-dict `:` type($value) `,` type($addr)"  # noqa: E501
 
 
 @irdl_op_definition
@@ -74,7 +78,7 @@ class LoadOp(IRDLOperation):
     ordering = opt_prop_def(UnitAttr)
     invariant = opt_prop_def(UnitAttr)
 
-    assembly_format = "(`volatile` $volatile^)? $addr (`atomic` (`syncscope` `(` $syncscope^ `)`)? $ordering^)? (`invariant` $invariant^)? attr-dict `:` type($addr) `->` type($res)"
+    assembly_format = "(`volatile` $volatile^)? $addr (`atomic` (`syncscope` `(` $syncscope^ `)`)? $ordering^)? (`invariant` $invariant^)? attr-dict `:` type($addr) `->` type($res)"  # noqa: E501
 
 
 @irdl_op_definition