Merge pull request #3344 from verilog-to-routing/feature-fix-cpp-format

AlexandreSinger · web-flow · commit a632849110cf · 2025-11-15T21:06:50.000-05:00
[CI] Fix CPP Format Issue
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -128,14 +128,27 @@ jobs:
       fail-fast: false
       matrix:
         include:
-        - { name: 'C/C++',       script: 'check-format.sh'   , with_python: 'no', pkgs: 'clang-format-18' }
-        - { name: 'Python',      script: 'check-format-py.sh', with_python: 'yes', pkgs: '' }
-        - { name: 'Python Lint', script: 'pylint_check.py'   , with_python: 'yes', pkgs: '' }
+        - { name: 'C/C++',       script: 'check-format.sh'   , with_python: 'yes', with_submodules: 'true', all_vtr_pkgs: 'yes', pkgs: 'clang-format-18' }
+        - { name: 'Python',      script: 'check-format-py.sh', with_python: 'yes', with_submodules: 'true', all_vtr_pkgs: 'yes', pkgs: '' }
+        - { name: 'Python Lint', script: 'pylint_check.py'   , with_python: 'yes', with_submodules: 'false', all_vtr_pkgs: 'no', pkgs: '' }
     name: 'F: ${{ matrix.name }}'
     steps:
 
     - uses: actions/checkout@v4
-      # NOTE: We do not need sub-modules. We do not check sub-modules for formatting.
+      with:
+        submodules: ${{ matrix.with_submodules }}
+      # NOTE: We usually do not need sub-modules. We do not check sub-modules
+      #       for formatting. However we may need to build make-format which
+      #       requires building VTR which will not build without submodules.
+      # TODO: We should relax this requirement.
+
+      # In order to perform a cpp format of VTR, make format is used which requires
+      # all of the vpr dependencies to be installed.
+      # TODO: This should be fixed so this test is not as heavy. We do not need
+      #       these dependencies to perform a simple format.
+    - name: Install dependencies
+      if: ${{ matrix.all_vtr_pkgs == 'yes' }}
+      run: ./install_apt_packages.sh
 
     # TODO: This should be on the same version of Python as would be found on
     #       Ubuntu 24.04 (3.12.3); however that version has some linting errors.
diff --git a/dev/check-format.sh b/dev/check-format.sh
@@ -8,7 +8,11 @@ if [ $clean -ne 0 ]; then
 else
     echo "Code Formatting Check"
     echo "====================="
-    make format"$1" > /dev/null 2>&1
+    make format"$1" > /dev/null
+    if [ $? -ne 0 ]; then
+        echo "make format failed!"
+        exit 1
+    fi
 
     valid_format=$(git diff | wc -l)
 
diff --git a/libs/librtlnumber/src/include/internal_bits.hpp b/libs/librtlnumber/src/include/internal_bits.hpp
@@ -27,14 +27,14 @@ constexpr short integer_t_size = (sizeof(integer_t) * 8);
     }
 
 #define unroll_1d(lut) \
-    {lut[_0], lut[_1], lut[_x], lut[_z]}
+    { lut[_0], lut[_1], lut[_x], lut[_z] }
 #define unroll_2d(lut) \
-    {unroll_1d(lut[_0]), unroll_1d(lut[_1]), unroll_1d(lut[_x]), unroll_1d(lut[_z])}
+    { unroll_1d(lut[_0]), unroll_1d(lut[_1]), unroll_1d(lut[_x]), unroll_1d(lut[_z]) }
 
 #define unroll_1d_invert(lut) \
-    {l_not[lut[_0]], l_not[lut[_1]], l_not[lut[_x]], l_not[lut[_z]]}
+    { l_not[lut[_0]], l_not[lut[_1]], l_not[lut[_x]], l_not[lut[_z]] }
 #define unroll_2d_invert(lut) \
-    {unroll_1d_invert(lut[_0]), unroll_1d_invert(lut[_1]), unroll_1d_invert(lut[_x]), unroll_1d_invert(lut[_z])}
+    { unroll_1d_invert(lut[_0]), unroll_1d_invert(lut[_1]), unroll_1d_invert(lut[_x]), unroll_1d_invert(lut[_z]) }
 
 namespace BitSpace {
 typedef uint8_t bit_value_t;
diff --git a/odin_ii/regression_test/tools/ODIN_CONFIG.py b/odin_ii/regression_test/tools/ODIN_CONFIG.py
@@ -1542,7 +1542,7 @@ def buildChildren(self, child_, nodeName_):
 
 
 def usage():
-    print USAGE_TEXT
+    print(USAGE_TEXT)
     sys.exit(1)
 
 
diff --git a/odin_ii/regression_test/tools/odin_config_maker.py b/odin_ii/regression_test/tools/odin_config_maker.py
@@ -28,13 +28,15 @@ def main(argv=None):
 
     # Check our options for input errors
     if not options.arch:
-        print "\tDid not get an architecture file; use odin_config_maker.py -h for help."
+        print("\tDid not get an architecture file; use odin_config_maker.py -h for help.")
         return -1
     if options.individual is None and options.directory is None and options.files is None:
-        print "\tDid not get any input options; use odin_config_maker.py -h for help."
+        print("\tDid not get any input options; use odin_config_maker.py -h for help.")
         return -1
     if options.individual is not None and options.directory is not None:
-        print "\tThe -i and -d options are mutually exclusive; use odin_config_maker.py -h for help."
+        print(
+            "\tThe -i and -d options are mutually exclusive; use odin_config_maker.py -h for help."
+        )
         return -1
 
     # Create our Config Files
@@ -54,7 +56,7 @@ def main(argv=None):
             )
 
         for file in file_list:
-            print file[file.rfind("/") + 1 : file.rfind(".v")]
+            print(file[file.rfind("/") + 1 : file.rfind(".v")])
             base = file[file.rfind("/") + 1 : file.rfind(".v")]
             create_odin_projects(options, [file], base, path)
 
@@ -78,7 +80,7 @@ def main(argv=None):
 
         create_odin_projects(options, file_list, base, path)
     else:
-        print "Something Failed!"
+        print("Something Failed!")
         return -1
 
 
diff --git a/odin_ii/regression_test/tools/synth_using_odin.py b/odin_ii/regression_test/tools/synth_using_odin.py
@@ -9,7 +9,7 @@
 
 
 if len(sys.argv) is not 2:
-    print "usage: " + sys.argv[0] + " <dir>"
+    print("usage: " + sys.argv[0] + " <dir>")
 
 path = abspath(sys.argv[1]) + "/"
 slog = open(path + "ODIN_success.lst", "w")
diff --git a/odin_ii/regression_test/tools/synth_using_quartus.py b/odin_ii/regression_test/tools/synth_using_quartus.py
@@ -19,8 +19,8 @@
 
     # print sys.argv
     if len(sys.argv) != 2:
-        print "Take our Verilog benchmarks and synthesize them using QIS (Quartus)"
-        print "Usage: GenQuartusBlifs.py <Verilog Source Dir>"
+        print("Take our Verilog benchmarks and synthesize them using QIS (Quartus)")
+        print("Usage: GenQuartusBlifs.py <Verilog Source Dir>")
         sys.exit(0)
 
     projNames = map(odin.trimDotV, filter(odin.isVerilog, os.listdir(sys.argv[1])))
@@ -68,11 +68,11 @@
 
     process = subprocess.Popen(clean, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, shell=True)
     (out, err) = process.communicate()
-    print out + "\n"
+    print(out + "\n")
 
     process = subprocess.Popen(create, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, shell=True)
     (out, err) = process.communicate()
-    print out + "\n"
+    print(out + "\n")
 
     process = subprocess.Popen(move, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, shell=True)
     (out, err) = process.communicate()
diff --git a/odin_ii/regression_test/tools/synth_using_vl2mv.py b/odin_ii/regression_test/tools/synth_using_vl2mv.py
@@ -10,7 +10,7 @@
 
 
 if len(sys.argv) is not 2:
-    print "usage: " + sys.argv[0] + " <dir>"
+    print("usage: " + sys.argv[0] + " <dir>")
 
 path = abspath(sys.argv[1]) + "/"
 os.system('mkdir -p "' + path + 'VL2MV_Blifs/"')
diff --git a/odin_ii/usefull_tools/restore_blackboxed_latches_from_blif_file.py b/odin_ii/usefull_tools/restore_blackboxed_latches_from_blif_file.py
@@ -49,20 +49,24 @@ def main():
     # print "DEBUG: RESTORED_BLIF_FILE: {}".format(args.restoredBlifFile)
 
     if args.inplace:
-        print "Inplace Restortation:"
+        print("Inplace Restortation:")
     elif not args.restoredBlifFile:
-        print '\n\nERROR: Must Specify Either Inplace Restoration "-i,--inplace" or a file to restore to "-r\--restoredBlifFile"\nExiting...\n'
+        print(
+            '\n\nERROR: Must Specify Either Inplace Restoration "-i,--inplace" or a file to restore to "-r\--restoredBlifFile"\nExiting...\n'
+        )
         parser.print_help()
         return -1
 
     if not os.path.isfile(args.blifFileToRestore):
-        print '\n\nERROR: BLIF File to Restore "{}" doesn not exist\nExiting...\n'.format(
-            args.blifFileToRestore
+        print(
+            '\n\nERROR: BLIF File to Restore "{}" doesn not exist\nExiting...\n'.format(
+                args.blifFileToRestore
+            )
         )
         parser.print_help()
         return -1
 
-    print "Restoring original inputs, ouptuts, types, controls and init_vals:"
+    print("Restoring original inputs, ouptuts, types, controls and init_vals:")
 
     if args.inplace:
         args.restoredBlifFile = "restored.blif.tmp"
@@ -81,15 +85,15 @@ def main():
                 line = line.replace(src, target)
             outfile.write(line)
 
-    print "Removing BlackBoxed Latch Model:"
+    print("Removing BlackBoxed Latch Model:")
 
     ignore = False
     for line in fileinput.input(args.restoredBlifFile, inplace=True):
         if not ignore:
             if line.startswith(".model bb_latch"):
                 ignore = True
             else:
-                print line,
+                print(line),
         if ignore and line.isspace():
             ignore = False
 
@@ -98,9 +102,9 @@ def main():
         dest_filename = os.path.join(args.blifFileToRestore)
         shutil.move(src_filename, dest_filename)
 
-        print "BLIF File Restored. See: {}".format(args.blifFileToRestore)
+        print("BLIF File Restored. See: {}".format(args.blifFileToRestore))
     else:
-        print "BLIF File Restored. See: {}".format(args.restoredBlifFile)
+        print("BLIF File Restored. See: {}".format(args.restoredBlifFile))
 
     return
 
diff --git a/vpr/src/base/read_netlist.cpp b/vpr/src/base/read_netlist.cpp
@@ -319,11 +319,11 @@ static void sync_clustered_and_atom_netlists(ClusteredNetlist& clb_nlist,
 }
 
 static void process_complex_block(pugi::xml_node clb_block,
-                                const ClusterBlockId index,
-                                int& num_primitives,
-                                const pugiutil::loc_data& loc_data,
-                                const std::unordered_map<std::string, int>& logical_block_type_name_to_index,
-                                ClusteredNetlist& clb_nlist) {
+                                  const ClusterBlockId index,
+                                  int& num_primitives,
+                                  const pugiutil::loc_data& loc_data,
+                                  const std::unordered_map<std::string, int>& logical_block_type_name_to_index,
+                                  ClusteredNetlist& clb_nlist) {
     const auto& logical_block_types = g_vpr_ctx.device().logical_block_types;
 
     //Parse cb attributes
diff --git a/vpr/src/pack/prepack.cpp b/vpr/src/pack/prepack.cpp
@@ -169,7 +169,6 @@ static std::vector<t_pack_patterns> alloc_and_load_pack_patterns(const std::vect
 
     std::vector<t_pack_patterns> packing_patterns = alloc_and_init_pattern_list_from_hash(pattern_names);
 
-
     /* load packing patterns by traversing the edges to find edges belonging to pattern */
     for (size_t i = 0; i < pattern_names.size(); i++) {
         for (const t_logical_block_type& type : logical_block_types) {
diff --git a/vpr/src/route/check_route.cpp b/vpr/src/route/check_route.cpp
@@ -740,23 +740,23 @@ static bool check_non_configurable_edges(const Netlist<>& net_list,
             //forward/reverse edge is used.
             std::vector<t_node_edge> dedupped_difference;
             std::ranges::copy_if(difference,
-                         std::back_inserter(dedupped_difference),
-                         [&](t_node_edge forward_edge) {
-                             VTR_ASSERT_MSG(!routing_edges.contains(forward_edge), "Difference should not contain used routing edges");
-
-                             t_node_edge reverse_edge = {forward_edge.to_node, forward_edge.from_node};
-
-                             //Check whether the reverse edge was used
-                             if (rr_edges.contains(reverse_edge) && routing_edges.contains(reverse_edge)) {
-                                 //The reverse edge exists in the set of rr_edges, and was used
-                                 //by the routing.
-                                 //
-                                 //We can therefore safely ignore the fact that this (forward) edge is un-used
-                                 return false; //Drop from difference
-                             } else {
-                                 return true; //Keep, this edge should have been used
-                             }
-                         });
+                                 std::back_inserter(dedupped_difference),
+                                 [&](t_node_edge forward_edge) {
+                                     VTR_ASSERT_MSG(!routing_edges.contains(forward_edge), "Difference should not contain used routing edges");
+
+                                     t_node_edge reverse_edge = {forward_edge.to_node, forward_edge.from_node};
+
+                                     //Check whether the reverse edge was used
+                                     if (rr_edges.contains(reverse_edge) && routing_edges.contains(reverse_edge)) {
+                                         //The reverse edge exists in the set of rr_edges, and was used
+                                         //by the routing.
+                                         //
+                                         //We can therefore safely ignore the fact that this (forward) edge is un-used
+                                         return false; //Drop from difference
+                                     } else {
+                                         return true; //Keep, this edge should have been used
+                                     }
+                                 });
 
             //At this point only valid missing node pairs are in the set
             if (!dedupped_difference.empty()) {
diff --git a/vpr/src/route/rr_graph_generation/rr_graph_intra_cluster.cpp b/vpr/src/route/rr_graph_generation/rr_graph_intra_cluster.cpp
@@ -202,7 +202,7 @@ static std::vector<int> get_cluster_block_pins(t_physical_tile_type_ptr physical
     bool found_sub_tile = false;
 
     // Iterate over all the sub-tiles to find the sub-tile instance that the cluster block is mapped to.
-    for (const t_sub_tile& sub_tile: physical_tile->sub_tiles) {
+    for (const t_sub_tile& sub_tile : physical_tile->sub_tiles) {
         if (sub_tile.capacity.is_in_range(sub_tile_index)) {
             // This sub-tile type is the one that the cluster block is mapped to.
             found_sub_tile = true;
diff --git a/vpr/src/server/taskresolver.cpp b/vpr/src/server/taskresolver.cpp
@@ -8,7 +8,6 @@
 #include "telegramoptions.h"
 #include <gtk/gtk.h>
 
-
 #include <ezgl/application.hpp>
 
 namespace server {
diff --git a/vpr/src/util/vpr_utils.cpp b/vpr/src/util/vpr_utils.cpp
@@ -1372,9 +1372,7 @@ std::tuple<int, int, std::string, std::string> parse_direct_pin_name(std::string
         std::string source_string{src_string};
 
         // Replace '.' and '[' characters with ' '
-        std::ranges::replace_if(source_string,
-            [](char c) noexcept { return c == '.' || c == '[' || c == ':' || c == ']'; },
-            ' ');
+        std::ranges::replace_if(source_string, [](char c) noexcept { return c == '.' || c == '[' || c == ':' || c == ']'; }, ' ');
 
         std::istringstream source_iss(source_string);
         int start_pin_index, end_pin_index;
diff --git a/vtr_flow/scripts/benchtracker/plotter-offline.py b/vtr_flow/scripts/benchtracker/plotter-offline.py
diff --git a/vtr_flow/scripts/download_titan.py b/vtr_flow/scripts/download_titan.py
diff --git a/vtr_flow/scripts/python_libs/vtr/parse_vtr_task.py b/vtr_flow/scripts/python_libs/vtr/parse_vtr_task.py
diff --git a/vtr_flow/scripts/spice/run_spice.py b/vtr_flow/scripts/spice/run_spice.py
