syn: avoid including private header from a public header & tidy

syn/synthesis.h was including syn/ir/Graph.h which was no accessible
to clients.  This was for a std::optional<Graph> which is replaced by
a std::unique_ptr<Graph>.

Misc other clang-tidy cleanups.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>

Author: maliberty

src/syn/include/syn/synthesis.h

@@ -3,13 +3,12 @@
 
 #pragma once
 
-#include <optional>
+#include <memory>
 #include <string>
 #include <vector>
 
 #include "db_sta/dbSta.hh"
 #include "odb/db.h"
-#include "syn/ir/Graph.h"
 #include "utl/Logger.h"
 
 namespace sta {
@@ -23,6 +22,7 @@ class Resizer;
 
 namespace syn {
 
+class Graph;
 class Net;
 
 class Synthesis : public sta::dbStaState
@@ -102,7 +102,7 @@ class Synthesis : public sta::dbStaState
   // which mirrors liberty's dont_use plus any user-set entries).
   bool dontUse(const sta::LibertyCell* cell) const;
 
-  Graph* graph() { return graph_ ? &*graph_ : nullptr; }
+  Graph* graph() { return graph_.get(); }
 
  private:
   // Resolve a net reference string to a Net.
@@ -111,7 +111,7 @@ class Synthesis : public sta::dbStaState
   odb::dbDatabase* db_ = nullptr;
   rsz::Resizer* resizer_ = nullptr;
   utl::Logger* logger_ = nullptr;
-  std::optional<Graph> graph_;
+  std::unique_ptr<Graph> graph_;
 };
 
 // Free-function entry points re-declared here so tests don't need to

src/syn/src/elab/backend_builder.cc

@@ -30,7 +30,7 @@ namespace slang_frontend {
 
 BackendGraphBuilder::BackendGraphBuilder()
 {
-  graph_.emplace();
+  graph_ = std::make_unique<syn::Graph>();
 }
 
 std::unique_ptr<BackendGraphBuilder> BackendGraphBuilder::start_new_graph(

src/syn/src/elab/backend_builder.h

@@ -11,7 +11,6 @@
 #include <cassert>
 #include <cstdint>
 #include <memory>
-#include <optional>
 #include <string>
 #include <string_view>
 #include <vector>
@@ -124,7 +123,7 @@ class BackendGraphBuilder : public BackendGraphBuilderBase
                  bool clk_polarity,
                  bool aload_polarity) override;
 
-  std::optional<syn::Graph> graph_;
+  std::unique_ptr<syn::Graph> graph_;
   unsigned next_id = 0;
 };
 

src/syn/src/elab/driver.cc

@@ -15,7 +15,6 @@
 
 #include "backend_builder.h"
 #include "blackboxes.h"
-#include "log_stubs.h"
 #include "slang/ast/Compilation.h"
 #include "slang/ast/SemanticFacts.h"
 #include "slang/ast/symbols/CompilationUnitSymbols.h"
@@ -37,10 +36,10 @@ using namespace slang_frontend;  // NOLINT(google-build-using-namespace)
 // Non-static so error.cc can call it from elaborateText. Not declared in
 // driver.h because the source-buffer parameter is only useful to the test
 // helper.
-std::optional<Graph> elaborateImpl(utl::Logger* logger,
-                                   const std::vector<std::string>& args,
-                                   sta::dbSta* sta,
-                                   std::optional<std::string> buffer)
+std::unique_ptr<Graph> elaborateImpl(utl::Logger* logger,
+                                     const std::vector<std::string>& args,
+                                     sta::dbSta* sta,
+                                     std::optional<std::string> buffer)
 {
   slang::driver::Driver driver;
   driver.addStandardArgs();
@@ -66,17 +65,17 @@ std::optional<Graph> elaborateImpl(utl::Logger* logger,
 
   if (!driver.parseCommandLine(static_cast<int>(c_args.size()),
                                const_cast<char**>(c_args.data()))) {
-    return std::nullopt;
+    return nullptr;
   }
 
   fixup_options(settings, driver);
   if (!driver.processOptions()) {
-    return std::nullopt;
+    return nullptr;
   }
   catch_forbidden_options(driver);
 
   if (!driver.parseAllSources()) {
-    return std::nullopt;
+    return nullptr;
   }
 
   auto compilation = driver.createCompilation();
@@ -91,12 +90,12 @@ std::optional<Graph> elaborateImpl(utl::Logger* logger,
 
   if (driver.diagEngine.getNumErrors()) {
     (void) driver.reportDiagnostics(/*quiet=*/false);
-    return std::nullopt;
+    return nullptr;
   }
 
   if (settings.ast_compilation_only.value_or(false)) {
     (void) driver.reportDiagnostics(/*quiet=*/false);
-    return std::nullopt;
+    return nullptr;
   }
 
   // We require exactly one top-level instance.
@@ -127,7 +126,7 @@ std::optional<Graph> elaborateImpl(utl::Logger* logger,
     slang::Diagnostics diags;
     diags.append_range(netlist.issued_diagnostics);
     diags.sort(driver.sourceManager);
-    for (int j = 0; j < (int) diags.size(); j++) {
+    for (int j = 0; j < diags.size(); j++) {
       if (j > 0 && diags[j] == diags[j - 1]) {
         continue;
       }
@@ -136,7 +135,7 @@ std::optional<Graph> elaborateImpl(utl::Logger* logger,
   }
 
   if (!driver.reportDiagnostics(/*quiet=*/false)) {
-    return std::nullopt;
+    return nullptr;
   }
 
   // Extract the graph from the top netlist's backend.
@@ -145,9 +144,9 @@ std::optional<Graph> elaborateImpl(utl::Logger* logger,
   return std::move(builder->graph_);
 }
 
-std::optional<Graph> elaborate(utl::Logger* logger,
-                               const std::vector<std::string>& args,
-                               sta::dbSta* sta)
+std::unique_ptr<Graph> elaborate(utl::Logger* logger,
+                                 const std::vector<std::string>& args,
+                                 sta::dbSta* sta)
 {
   return elaborateImpl(logger, args, sta, {});
 }

src/syn/src/elab/driver.h

@@ -8,7 +8,7 @@
 //
 #pragma once
 
-#include <optional>
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -28,10 +28,10 @@ namespace syn {
 // Arguments are passed through to slang's command-line parser
 // (file paths, -I, --top, defines, etc.).
 // If sta is non-null, Liberty cell definitions are imported as blackboxes.
-std::optional<Graph> elaborate(utl::Logger* logger,
-                               const std::vector<std::string>& args,
-                               sta::dbSta* sta = nullptr);
-std::optional<Graph> elaborateText(const std::string& source,
-                                   const std::vector<std::string>& args = {});
+std::unique_ptr<Graph> elaborate(utl::Logger* logger,
+                                 const std::vector<std::string>& args,
+                                 sta::dbSta* sta = nullptr);
+std::unique_ptr<Graph> elaborateText(const std::string& source,
+                                     const std::vector<std::string>& args = {});
 
 }  // namespace syn

src/syn/src/elab/error.cc

@@ -10,6 +10,7 @@
 //   - slang_frontend::reportError  (calls Logger::error)
 //   - syn::elaborateText           (default-constructs a utl::Logger)
 
+#include <memory>
 #include <optional>
 #include <string>
 #include <string_view>
@@ -22,13 +23,13 @@
 namespace syn {
 
 // Defined in driver.cc.
-std::optional<Graph> elaborateImpl(utl::Logger* logger,
-                                   const std::vector<std::string>& args,
-                                   sta::dbSta* sta,
-                                   std::optional<std::string> buffer);
+std::unique_ptr<Graph> elaborateImpl(utl::Logger* logger,
+                                     const std::vector<std::string>& args,
+                                     sta::dbSta* sta,
+                                     std::optional<std::string> buffer);
 
-std::optional<Graph> elaborateText(const std::string& source,
-                                   const std::vector<std::string>& args)
+std::unique_ptr<Graph> elaborateText(const std::string& source,
+                                     const std::vector<std::string>& args)
 {
   utl::Logger logger;
   return elaborateImpl(&logger, args, nullptr, source);

src/syn/src/flow/abc.cc

@@ -14,6 +14,7 @@
 #include "syn/ir/Graph.h"
 #include "syn/ir/Instance.h"
 #include "syn/ir/Net.h"
+#include "syn/synthesis.h"
 #include "utl/Logger.h"
 
 // ABC headers are already compiled within namespace abc by the build system.

src/syn/src/flow/bitblast.cc

@@ -320,7 +320,7 @@ struct BitblastedOperation
     // Stages 2..levels-1 (Kogge-Stone on odd indices only).
     for (int i = 1; i < levels; i++) {
       uint32_t stride = 1u << i;
-      for (int j = (int) n - 1; j >= (int) stride; j--) {
+      for (int j = (int) n - 1; j >= stride; j--) {
         if (j % 2 == 1) {
           Literal new_g = OR(g[j], AND(p[j], g[j - stride]));
           Literal new_p = AND(p[j], p[j - stride]);
@@ -594,7 +594,7 @@ struct BitblastedOperation
             Literal product = AND(a_bits[j], b_bits[k]);
 
             if ((k == b_len - 1 || j == a_len - 1)
-                && !(k == b_len - 1 && j == a_len - 1)) {
+                && (k != b_len - 1 || j != a_len - 1)) {
               product = !product;
               corrections[j + k]--;
             }

src/syn/src/flow/combinational_mapper.cc

@@ -66,7 +66,7 @@ static Truth6 fexprEval(sta::FuncExpr* fexpr,
     case Op::port: {
       // Find the index of this port in inputs
       int in_idx;
-      for (in_idx = 0; in_idx < (int) inputs.size(); in_idx++) {
+      for (in_idx = 0; in_idx < inputs.size(); in_idx++) {
         if (inputs[in_idx] == fexpr->port()) {
           break;
         }
@@ -677,8 +677,8 @@ void Mapping::prepareMatches(const int npriority_cuts,
     int psSlot = 0;
     ClassMatch* matchBuf = allocMatches(nmatches_max);
 
-    for (int i = -1; i < (int) cache[n1->fid].ps.size(); i++) {
-      for (int j = -1; j < (int) cache[n2->fid].ps.size(); j++) {
+    for (int i = -1; i < cache[n1->fid].ps.size(); i++) {
+      for (int j = -1; j < cache[n2->fid].ps.size(); j++) {
         Net* n1Cut = ((i == -1) ? t1 : cache[n1->fid].ps[i].cut);
         Net* n2Cut = ((j == -1) ? t2 : cache[n2->fid].ps[j].cut);
         int n1CutLen = (i == -1) ? 1 : cutLen(n1Cut);

src/syn/src/flow/constant_fold.cc

@@ -1,8 +1,10 @@
 // SPDX-License-Identifier: BSD-3-Clause
 // Copyright (c) 2026, The OpenROAD Authors
 
+#include "constant_fold.h"
+
+#include <cassert>
 #include <cstdint>
-#include <iostream>
 #include <optional>
 #include <utility>
 #include <vector>
@@ -82,7 +84,7 @@ static int log2exact(uint64_t val)
   return __builtin_ctzll(val);
 }
 
-void foldCombinationals(Graph& g)
+static void foldCombinationals(Graph& g)
 {
   std::vector<Net> new_output;
   g.forEachInstance([&](const Instance* inst) {
@@ -422,7 +424,7 @@ void foldCombinationals(Graph& g)
         }
         if (shift >= 0) {
           for (uint32_t i = 0; i < width; i++) {
-            if ((int) i < shift) {
+            if (i < shift) {
               new_output[i] = stripBuffers(g, op->a()[i]);
             } else {
               new_output[i] = Net::zero();
@@ -463,7 +465,7 @@ static BundleView sliceDff(Graph& g,
                     original->clearValue().slice(base, width));
 }
 
-bool foldSequentials(Graph& g)
+static bool foldSequentials(Graph& g)
 {
   // --- Flop optimization: replace DFF bits with constants ---
   //