Enable Pylint in CI and fix its errors

The main fixes were: * Specify encoding for all file opens. By default it depends on environment variables which is bad. * Use `with` to open files. Otherwise they don't necessarily get closed. There were also a few minor things like using `enumerate`, not using objects as default arguments, etc. In some cases I slightly refactored the code.
riscv · Nov 5, 2024 · 8eeb4a5 · 8eeb4a5
1 parent 95eaf6c
commit 8eeb4a5
Show file tree

Hide file tree

Showing 12 changed files with 191 additions and 153 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -21,11 +21,10 @@ repos:
     hooks:
       - id: black
 
-  # TODO: Enable this when lints are fixed.
-  # - repo: https://github.com/PyCQA/pylint
-  #   rev: v3.3.1
-  #   hooks:
-  #     - id: pylint
+  - repo: https://github.com/PyCQA/pylint
+    rev: v3.3.1
+    hooks:
+      - id: pylint
 
   - repo: https://github.com/RobertCraigie/pyright-python
     rev: v1.1.383

diff --git a/.pylintrc b/.pylintrc
@@ -0,0 +1,31 @@
+[MAIN]
+py-version = 3.9.0
+disable=
+    # Allow 'TODO:' in code.
+    fixme,
+    # Overly zealous duplicate code detection.
+    duplicate-code,
+    # These debatable style lints are quite annoying, and often push
+    # you into mixing up small changes (adding one statement to a function)
+    # with large refactors (splitting the function up into shorter functions).
+    too-few-public-methods,
+    too-many-arguments,
+    too-many-positional-arguments,
+    too-many-branches,
+    too-many-instance-attributes,
+    too-many-locals,
+    too-many-return-statements,
+    too-many-statements,
+    # Handled by Black.
+    line-too-long,
+    # This is technically correct but not that important.
+    logging-fstring-interpolation,
+    # TODO: These should be enabled but writing documentation for
+    # all of the code is not feasible in one go.
+    missing-module-docstring,
+    missing-function-docstring,
+    missing-class-docstring,
+
+# These names are fine when used sensibly. Without listing them here
+# Pylint will complain they are too short.
+good-names=c,i,j,k,id,pc
diff --git a/c_utils.py b/c_utils.py
@@ -43,7 +43,7 @@ def make_c(instr_dict: InstrDict):
         mask = ((1 << (end - begin + 1)) - 1) << begin
         arg_str += f"#define INSN_FIELD_{sanitized_name.upper()} {hex(mask)}\n"
 
-    with open(f"{os.path.dirname(__file__)}/encoding.h", "r") as file:
+    with open(f"{os.path.dirname(__file__)}/encoding.h", "r", encoding="utf-8") as file:
         enc_header = file.read()
 
     commit = os.popen('git log -1 --format="format:%h"').read()
@@ -75,5 +75,5 @@ def make_c(instr_dict: InstrDict):
 """
 
     # Write the modified output to the file
-    with open("encoding.out.h", "w") as enc_file:
+    with open("encoding.out.h", "w", encoding="utf-8") as enc_file:
         enc_file.write(output_str)
diff --git a/chisel_utils.py b/chisel_utils.py
@@ -21,7 +21,6 @@ def make_chisel(instr_dict: InstrDict, spinal_hdl: bool = False):
     if not spinal_hdl:
         extensions = instr_dict_2_extensions(instr_dict)
         for e in extensions:
-            e_instrs = filter(lambda i: instr_dict[i]["extension"][0] == e, instr_dict)
             if "rv64_" in e:
                 e_format = e.replace("rv64_", "").upper() + "64"
             elif "rv32_" in e:
@@ -31,10 +30,11 @@ def make_chisel(instr_dict: InstrDict, spinal_hdl: bool = False):
             else:
                 e_format = e.upper()
             chisel_names += f'  val {e_format+"Type"} = Map(\n'
-            for instr in e_instrs:
-                tmp_instr_name = '"' + instr.upper().replace(".", "_") + '"'
-                chisel_names += f'   {tmp_instr_name:<18s} -> BitPat("b{instr_dict[instr]["encoding"].replace("-","?")}"),\n'
-            chisel_names += f"  )\n"
+            for instr_name, instr in instr_dict.items():
+                if instr["extension"][0] == e:
+                    tmp_instr_name = '"' + instr_name.upper().replace(".", "_") + '"'
+                    chisel_names += f'   {tmp_instr_name:<18s} -> BitPat("b{instr["encoding"].replace("-","?")}"),\n'
+            chisel_names += "  )\n"
 
     for num, name in causes:
         cause_names_str += f'  val {name.lower().replace(" ","_")} = {hex(num)}\n'
@@ -63,12 +63,11 @@ def make_chisel(instr_dict: InstrDict, spinal_hdl: bool = False):
     csr_names_str += """    res.toArray
   }"""
 
-    if spinal_hdl:
-        chisel_file = open("inst.spinalhdl", "w")
-    else:
-        chisel_file = open("inst.chisel", "w")
-    chisel_file.write(
-        f"""
+    with open(
+        "inst.spinalhdl" if spinal_hdl else "inst.chisel", "w", encoding="utf-8"
+    ) as chisel_file:
+        chisel_file.write(
+            f"""
 /* Automatically generated by parse_opcodes */
 object Instructions {{
 {chisel_names}
@@ -80,5 +79,4 @@ def make_chisel(instr_dict: InstrDict, spinal_hdl: bool = False):
 {csr_names_str}
 }}
 """
-    )
-    chisel_file.close()
+        )
diff --git a/constants.py b/constants.py
@@ -44,7 +44,7 @@
 )
 
 
-def read_csv(filename: str):
+def read_int_map_csv(filename: str) -> "list[tuple[int, str]]":
     """
     Reads a CSV file and returns a list of tuples.
     Each tuple contains an integer value (from the first column) and a string (from the second column).
@@ -55,20 +55,26 @@ def read_csv(filename: str):
     Returns:
         list of tuple: A list of (int, str) tuples extracted from the CSV file.
     """
-    with open(filename) as f:
+    with open(filename, encoding="utf-8") as f:
         csv_reader = csv.reader(f, skipinitialspace=True)
         return [(int(row[0], 0), row[1]) for row in csv_reader]
 
 
-causes = read_csv("causes.csv")
-csrs = read_csv("csrs.csv")
-csrs32 = read_csv("csrs32.csv")
+causes = read_int_map_csv("causes.csv")
+csrs = read_int_map_csv("csrs.csv")
+csrs32 = read_int_map_csv("csrs32.csv")
 
-# Load the argument lookup table (arg_lut) from a CSV file, mapping argument names to their bit positions
-arg_lut = {
-    row[0]: (int(row[1]), int(row[2]))
-    for row in csv.reader(open("arg_lut.csv"), skipinitialspace=True)
-}
+
+def read_arg_lut_csv(filename: str) -> "dict[str, tuple[int, int]]":
+    """
+    Load the argument lookup table (arg_lut) from a CSV file, mapping argument names to their bit positions.
+    """
+    with open(filename, encoding="utf-8") as f:
+        csv_reader = csv.reader(f, skipinitialspace=True)
+        return {row[0]: (int(row[1]), int(row[2])) for row in csv_reader}
+
+
+arg_lut = read_arg_lut_csv("arg_lut.csv")
 
 # for mop
 arg_lut["mop_r_t_30"] = (30, 30)

diff --git a/go_utils.py b/go_utils.py
@@ -1,5 +1,6 @@
 import logging
 import pprint
+import subprocess
 import sys
 
 from shared_utils import InstrDict, signed
@@ -49,14 +50,12 @@ def make_go(instr_dict: InstrDict):
     return &inst{{ {hex(opcode)}, {hex(funct3)}, {hex(rs1)}, {hex(rs2)}, {signed(csr,12)}, {hex(funct7)} }}
 """
 
-    with open("inst.go", "w") as file:
+    with open("inst.go", "w", encoding="utf-8") as file:
         file.write(prelude)
         file.write(instr_str)
         file.write(endoffile)
 
     try:
-        import subprocess
-
-        subprocess.run(["go", "fmt", "inst.go"])
-    except:
+        subprocess.run(["go", "fmt", "inst.go"], check=True)
+    except:  # pylint: disable=bare-except
         pass
diff --git a/latex_utils.py b/latex_utils.py
@@ -10,7 +10,6 @@
 
 
 def make_priv_latex_table():
-    latex_file = open("priv-instr-table.tex", "w")
     type_list = ["R-type", "I-type"]
     system_instr = ["_h", "_s", "_system", "_svinval", "64_h", "_svinval_h"]
     dataset_list = [(system_instr, "Trap-Return Instructions", ["sret", "mret"], False)]
@@ -75,9 +74,8 @@ def make_priv_latex_table():
         )
     )
     caption = "\\caption{RISC-V Privileged Instructions}"
-    make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
-
-    latex_file.close()
+    with open("priv-instr-table.tex", "w", encoding="utf-8") as latex_file:
+        make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
 
 
 def make_latex_table():
@@ -101,84 +99,89 @@ def make_latex_table():
     riscv-isa-manual.
     """
     # open the file and use it as a pointer for all further dumps
-    latex_file = open("instr-table.tex", "w")
-
-    # create the rv32i table first. Here we set the caption to empty. We use the
-    # files rv_i and rv32_i to capture instructions relevant for rv32i
-    # configuration. The dataset is a list of 4-element tuples :
-    # (list_of_extensions, title, list_of_instructions, include_pseudo_ops). If list_of_instructions
-    # is empty then it indicates that all instructions of the all the extensions
-    # in list_of_extensions need to be dumped. If not empty, then only the
-    # instructions listed in list_of_instructions will be dumped into latex.
-    caption = ""
-    type_list = ["R-type", "I-type", "S-type", "B-type", "U-type", "J-type"]
-    dataset_list: list[tuple[list[str], str, list[str], bool]] = [
-        (["_i", "32_i"], "RV32I Base Instruction Set", [], False)
-    ]
-    dataset_list.append((["_i"], "", ["fence_tso", "pause"], True))
-    make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
-
-    type_list = ["R-type", "I-type", "S-type"]
-    dataset_list = [
-        (["64_i"], "RV64I Base Instruction Set (in addition to RV32I)", [], False)
-    ]
-    dataset_list.append(
-        (["_zifencei"], "RV32/RV64 Zifencei Standard Extension", [], False)
-    )
-    dataset_list.append((["_zicsr"], "RV32/RV64 Zicsr Standard Extension", [], False))
-    dataset_list.append((["_m", "32_m"], "RV32M Standard Extension", [], False))
-    dataset_list.append(
-        (["64_m"], "RV64M Standard Extension (in addition to RV32M)", [], False)
-    )
-    make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
-
-    type_list = ["R-type"]
-    dataset_list = [(["_a"], "RV32A Standard Extension", [], False)]
-    dataset_list.append(
-        (["64_a"], "RV64A Standard Extension (in addition to RV32A)", [], False)
-    )
-    make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
-
-    type_list = ["R-type", "R4-type", "I-type", "S-type"]
-    dataset_list = [(["_f"], "RV32F Standard Extension", [], False)]
-    dataset_list.append(
-        (["64_f"], "RV64F Standard Extension (in addition to RV32F)", [], False)
-    )
-    make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
+    with open("instr-table.tex", "w", encoding="utf-8") as latex_file:
+
+        # create the rv32i table first. Here we set the caption to empty. We use the
+        # files rv_i and rv32_i to capture instructions relevant for rv32i
+        # configuration. The dataset is a list of 4-element tuples :
+        # (list_of_extensions, title, list_of_instructions, include_pseudo_ops). If list_of_instructions
+        # is empty then it indicates that all instructions of the all the extensions
+        # in list_of_extensions need to be dumped. If not empty, then only the
+        # instructions listed in list_of_instructions will be dumped into latex.
+        caption = ""
+        type_list = ["R-type", "I-type", "S-type", "B-type", "U-type", "J-type"]
+        dataset_list: list[tuple[list[str], str, list[str], bool]] = [
+            (["_i", "32_i"], "RV32I Base Instruction Set", [], False)
+        ]
+        dataset_list.append((["_i"], "", ["fence_tso", "pause"], True))
+        make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
+
+        type_list = ["R-type", "I-type", "S-type"]
+        dataset_list = [
+            (["64_i"], "RV64I Base Instruction Set (in addition to RV32I)", [], False)
+        ]
+        dataset_list.append(
+            (["_zifencei"], "RV32/RV64 Zifencei Standard Extension", [], False)
+        )
+        dataset_list.append(
+            (["_zicsr"], "RV32/RV64 Zicsr Standard Extension", [], False)
+        )
+        dataset_list.append((["_m", "32_m"], "RV32M Standard Extension", [], False))
+        dataset_list.append(
+            (["64_m"], "RV64M Standard Extension (in addition to RV32M)", [], False)
+        )
+        make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
 
-    type_list = ["R-type", "R4-type", "I-type", "S-type"]
-    dataset_list = [(["_d"], "RV32D Standard Extension", [], False)]
-    dataset_list.append(
-        (["64_d"], "RV64D Standard Extension (in addition to RV32D)", [], False)
-    )
-    make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
+        type_list = ["R-type"]
+        dataset_list = [(["_a"], "RV32A Standard Extension", [], False)]
+        dataset_list.append(
+            (["64_a"], "RV64A Standard Extension (in addition to RV32A)", [], False)
+        )
+        make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
 
-    type_list = ["R-type", "R4-type", "I-type", "S-type"]
-    dataset_list = [(["_q"], "RV32Q Standard Extension", [], False)]
-    dataset_list.append(
-        (["64_q"], "RV64Q Standard Extension (in addition to RV32Q)", [], False)
-    )
-    make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
+        type_list = ["R-type", "R4-type", "I-type", "S-type"]
+        dataset_list = [(["_f"], "RV32F Standard Extension", [], False)]
+        dataset_list.append(
+            (["64_f"], "RV64F Standard Extension (in addition to RV32F)", [], False)
+        )
+        make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
 
-    caption = "\\caption{Instruction listing for RISC-V}"
-    type_list = ["R-type", "R4-type", "I-type", "S-type"]
-    dataset_list = [
-        (["_zfh", "_d_zfh", "_q_zfh"], "RV32Zfh Standard Extension", [], False)
-    ]
-    dataset_list.append(
-        (["64_zfh"], "RV64Zfh Standard Extension (in addition to RV32Zfh)", [], False)
-    )
-    make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
+        type_list = ["R-type", "R4-type", "I-type", "S-type"]
+        dataset_list = [(["_d"], "RV32D Standard Extension", [], False)]
+        dataset_list.append(
+            (["64_d"], "RV64D Standard Extension (in addition to RV32D)", [], False)
+        )
+        make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
 
-    ## The following is demo to show that Compressed instructions can also be
-    # dumped in the same manner as above
+        type_list = ["R-type", "R4-type", "I-type", "S-type"]
+        dataset_list = [(["_q"], "RV32Q Standard Extension", [], False)]
+        dataset_list.append(
+            (["64_q"], "RV64Q Standard Extension (in addition to RV32Q)", [], False)
+        )
+        make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
+
+        caption = "\\caption{Instruction listing for RISC-V}"
+        type_list = ["R-type", "R4-type", "I-type", "S-type"]
+        dataset_list = [
+            (["_zfh", "_d_zfh", "_q_zfh"], "RV32Zfh Standard Extension", [], False)
+        ]
+        dataset_list.append(
+            (
+                ["64_zfh"],
+                "RV64Zfh Standard Extension (in addition to RV32Zfh)",
+                [],
+                False,
+            )
+        )
+        make_ext_latex_table(type_list, dataset_list, latex_file, 32, caption)
 
-    # type_list = ['']
-    # dataset_list = [(['_c', '32_c', '32_c_f','_c_d'],'RV32C Standard Extension', [])]
-    # dataset_list.append((['64_c'],'RV64C Standard Extension (in addition to RV32C)', []))
-    # make_ext_latex_table(type_list, dataset_list, latex_file, 16, caption)
+        ## The following is demo to show that Compressed instructions can also be
+        # dumped in the same manner as above
 
-    latex_file.close()
+        # type_list = ['']
+        # dataset_list = [(['_c', '32_c', '32_c_f','_c_d'],'RV32C Standard Extension', [])]
+        # dataset_list.append((['64_c'],'RV64C Standard Extension (in addition to RV32C)', []))
+        # make_ext_latex_table(type_list, dataset_list, latex_file, 16, caption)
 
 
 def make_ext_latex_table(
@@ -319,8 +322,7 @@ def make_ext_latex_table(
         # for each argument/string of 1s or 0s, create a multicolumn latex table
         # entry
         entry = ""
-        for r in range(len(fields)):
-            (msb, lsb, name) = fields[r]
+        for r, (msb, lsb, name) in enumerate(fields):
             if r == len(fields) - 1:
                 entry += (
                     f"\\multicolumn{{{msb - lsb + 1}}}{{|c|}}{{{name}}} & {t} \\\\\n"
@@ -379,7 +381,7 @@ def make_ext_latex_table(
                 encoding = instr_dict[inst]["encoding"]
             for r in range(0, ilen):
                 x = encoding[r]
-                if ((msb, ilen - 1 - r + 1)) in latex_fixed_fields:
+                if (msb, ilen - 1 - r + 1) in latex_fixed_fields:
                     fields.append((msb, ilen - 1 - r + 1, y))
                     msb = ilen - 1 - r
                     y = ""
@@ -397,8 +399,7 @@ def make_ext_latex_table(
 
             fields.sort(key=lambda y: y[0], reverse=True)
             entry = ""
-            for r in range(len(fields)):
-                (msb, lsb, name) = fields[r]
+            for r, (msb, lsb, name) in enumerate(fields):
                 if r == len(fields) - 1:
                     entry += f'\\multicolumn{{{msb - lsb + 1}}}{{|c|}}{{{name}}} & {inst.upper().replace("_",".")} \\\\\n'
                 elif r == 0: