Merge pull request #1915 from xlsynth:cdleary/2025-02-06-warnings-pas…

…sed-to-c-api

PiperOrigin-RevId: 727010347

xls/contrib/mlir/tools/xls_translate/xls_translate_from_mlir.cc

@@ -984,8 +984,6 @@ FailureOr<PackageInfo> importDslxInstantiation(
   ::llvm::StringRef path = file_import_op.getFilename();
 
   std::string module_name = "imported_module";
-  std::string_view stdlib_path = ::xls::GetDefaultDslxStdlibPath();
-  std::vector<std::filesystem::path> additional_search_paths = {};
   absl::StatusOr<std::string> package_string_or;
 
   // Note: this is not bullet proof. The experience if these are wrong would
@@ -1006,9 +1004,12 @@ FailureOr<PackageInfo> importDslxInstantiation(
 
   // Note: using a different pathname here else XLS considers this a circular
   // import.
-  package_string_or =
-      ::xls::ConvertDslxToIr(dslx, "<instantiated module>", module_name,
-                             stdlib_path, additional_search_paths);
+  const ::xls::ConvertDslxToIrOptions options{
+      .dslx_stdlib_path = ::xls::GetDefaultDslxStdlibPath(),
+      .warnings_as_errors = false,
+  };
+  package_string_or = ::xls::ConvertDslxToIr(dslx, "<instantiated module>",
+                                             module_name, options);
   if (!package_string_or.ok()) {
     llvm::errs() << "Failed to convert DSLX to IR: "
                  << package_string_or.status().message() << "\n";
@@ -1769,8 +1770,12 @@ absl::StatusOr<std::shared_ptr<const Package>> DslxPackageCache::import(
   if (it != cache.end()) {
     return it->second;
   }
-  absl::StatusOr<std::string> package_string_or = ::xls::ConvertDslxPathToIr(
-      fileName, ::xls::GetDefaultDslxStdlibPath(), {});
+  const ::xls::ConvertDslxToIrOptions options{
+      .dslx_stdlib_path = ::xls::GetDefaultDslxStdlibPath(),
+      .warnings_as_errors = false,
+  };
+  absl::StatusOr<std::string> package_string_or =
+      ::xls::ConvertDslxPathToIr(fileName, options);
   if (!package_string_or.ok()) {
     return package_string_or.status();
   }

xls/dslx/ir_convert/convert_options.h

@@ -40,10 +40,10 @@ struct ConvertOptions {
   // Should warnings be treated as errors?
   bool warnings_as_errors = true;
 
-  // Set of warnings that are enabled.
+  // Set of warnings used in DSLX typechecking.
   //
   // Note that this is only used in IR conversion routines that do typechecking.
-  WarningKindSet enabled_warnings = kDefaultWarningsSet;
+  WarningKindSet warnings = kDefaultWarningsSet;
 
   // Should #[test] and #[test_proc] entities be emitted to IR.
   bool convert_tests = false;

xls/dslx/ir_convert/ir_converter.cc

@@ -590,9 +590,9 @@ absl::StatusOr<PackageConversionData> ConvertFilesToPackage(
         "path to know where to resolve the entry function");
   }
   for (std::string_view path : paths) {
-    ImportData import_data(CreateImportData(
-        stdlib_path, dslx_paths, convert_options.enabled_warnings,
-        std::make_unique<RealFilesystem>()));
+    ImportData import_data(
+        CreateImportData(stdlib_path, dslx_paths, convert_options.warnings,
+                         std::make_unique<RealFilesystem>()));
     XLS_ASSIGN_OR_RETURN(std::string text,
                          import_data.vfs().GetFileContents(path));
     XLS_ASSIGN_OR_RETURN(std::string module_name, PathToName(path));

xls/dslx/ir_convert/ir_converter_main.cc

@@ -111,7 +111,7 @@ absl::Status RealMain(absl::Span<const std::string_view> paths) {
       .emit_fail_as_assert = emit_fail_as_assert,
       .verify_ir = verify_ir,
       .warnings_as_errors = warnings_as_errors,
-      .enabled_warnings = warnings,
+      .warnings = warnings,
       .convert_tests = convert_tests,
       .default_fifo_config = default_fifo_config,
   };

xls/dslx/run_routines/ir_test_runner.cc

@@ -204,7 +204,7 @@ absl::StatusOr<std::unique_ptr<AbstractParsedTestRunner>> MakeRunner(
       .emit_fail_as_assert = true,
       .verify_ir = false,
       .warnings_as_errors = false,
-      .enabled_warnings = kNoWarningsSet,
+      .warnings = kNoWarningsSet,
       .convert_tests = true,
   };
   absl::flat_hash_map<std::string, std::unique_ptr<Package>> packages;

xls/dslx/warning_collector.h

@@ -48,6 +48,8 @@ class WarningCollector {
 
   const std::vector<Entry>& warnings() const { return warnings_; }
 
+  bool empty() const { return warnings_.empty(); }
+
  private:
   const WarningKindSet enabled_;
   std::vector<Entry> warnings_;

xls/public/BUILD

@@ -47,6 +47,7 @@ cc_library(
         "//xls/dslx:mangle",
         "//xls/dslx:parse_and_typecheck",
         "//xls/dslx:virtualizable_file_system",
+        "//xls/dslx:warning_collector",
         "//xls/dslx:warning_kind",
         "//xls/dslx/frontend:ast",
         "//xls/dslx/frontend:pos",
@@ -61,7 +62,9 @@ cc_library(
         "//xls/tools:proto_to_dslx",
         "//xls/tools:scheduling_options_flags_cc_proto",
         "@com_google_absl//absl/log",
+        "@com_google_absl//absl/status",
         "@com_google_absl//absl/status:statusor",
+        "@com_google_absl//absl/strings",
         "@com_google_absl//absl/types:span",
     ],
 )
@@ -164,6 +167,7 @@ cc_library(
         "@com_google_absl//absl/log:check",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings:str_format",
+        "@com_google_absl//absl/types:span",
     ],
 )
 

xls/public/c_api.cc

@@ -58,39 +58,119 @@ void xls_init_xls(const char* usage, int argc, char* argv[]) {
   (void)(xls::InitXls(usage, argc, argv));
 }
 
+bool xls_convert_dslx_to_ir_with_warnings(
+    const char* dslx, const char* path, const char* module_name,
+    const char* dslx_stdlib_path, const char* additional_search_paths[],
+    size_t additional_search_paths_count, const char* enable_warnings[],
+    size_t enable_warnings_count, const char* disable_warnings[],
+    size_t disable_warnings_count, bool warnings_as_errors,
+    char*** warnings_out, size_t* warnings_out_count, char** error_out,
+    char** ir_out) {
+  CHECK(dslx != nullptr);
+  CHECK(path != nullptr);
+  CHECK(dslx_stdlib_path != nullptr);
+  CHECK(error_out != nullptr);
+
+  std::vector<std::filesystem::path> additional_search_paths_cpp =
+      xls::ToCppPaths(additional_search_paths, additional_search_paths_count);
+  std::vector<std::string_view> enable_warnings_cpp =
+      xls::ToCppStringViews(enable_warnings, enable_warnings_count);
+  std::vector<std::string_view> disable_warnings_cpp =
+      xls::ToCppStringViews(disable_warnings, disable_warnings_count);
+
+  std::vector<std::string> warnings_out_cpp;
+
+  const xls::ConvertDslxToIrOptions options = {
+      .dslx_stdlib_path = dslx_stdlib_path,
+      .additional_search_paths = additional_search_paths_cpp,
+      .enable_warnings = enable_warnings_cpp,
+      .disable_warnings = disable_warnings_cpp,
+      .warnings_as_errors = warnings_as_errors,
+      .warnings_out = &warnings_out_cpp,
+  };
+
+  absl::StatusOr<std::string> result =
+      xls::ConvertDslxToIr(dslx, path, module_name, options);
+
+  if (warnings_out != nullptr) {
+    xls::ToOwnedCStrings(warnings_out_cpp, warnings_out, warnings_out_count);
+  } else if (warnings_out_count != nullptr) {
+    *warnings_out_count = warnings_out_cpp.size();
+  }
+
+  return xls::ReturnStringHelper(result, error_out, ir_out);
+}
+
 bool xls_convert_dslx_to_ir(const char* dslx, const char* path,
                             const char* module_name,
                             const char* dslx_stdlib_path,
                             const char* additional_search_paths[],
                             size_t additional_search_paths_count,
                             char** error_out, char** ir_out) {
-  CHECK(dslx != nullptr);
+  const char* enable_warnings[] = {};
+  const char* disable_warnings[] = {};
+  return xls_convert_dslx_to_ir_with_warnings(
+      dslx, path, module_name, dslx_stdlib_path, additional_search_paths,
+      additional_search_paths_count, enable_warnings, 0, disable_warnings, 0,
+      /*warnings_as_errors=*/false,
+      /*warnings_out=*/nullptr,
+      /*warnings_out_count=*/nullptr, error_out, ir_out);
+}
+
+bool xls_convert_dslx_path_to_ir_with_warnings(
+    const char* path, const char* dslx_stdlib_path,
+    const char* additional_search_paths[], size_t additional_search_paths_count,
+    const char* enable_warnings[], size_t enable_warnings_count,
+    const char* disable_warnings[], size_t disable_warnings_count,
+    bool warnings_as_errors, char*** warnings_out, size_t* warnings_out_count,
+    char** error_out, char** ir_out) {
   CHECK(path != nullptr);
   CHECK(dslx_stdlib_path != nullptr);
   CHECK(error_out != nullptr);
+  if (warnings_out != nullptr) {
+    CHECK(warnings_out_count != nullptr);
+  }
 
   std::vector<std::filesystem::path> additional_search_paths_cpp =
-      xls::ToCpp(additional_search_paths, additional_search_paths_count);
+      xls::ToCppPaths(additional_search_paths, additional_search_paths_count);
+  std::vector<std::string_view> enable_warnings_cpp =
+      xls::ToCppStringViews(enable_warnings, enable_warnings_count);
+  std::vector<std::string_view> disable_warnings_cpp =
+      xls::ToCppStringViews(disable_warnings, disable_warnings_count);
+
+  std::vector<std::string> warnings_out_cpp;
+
+  const xls::ConvertDslxToIrOptions options = {
+      .dslx_stdlib_path = dslx_stdlib_path,
+      .additional_search_paths = additional_search_paths_cpp,
+      .enable_warnings = enable_warnings_cpp,
+      .disable_warnings = disable_warnings_cpp,
+      .warnings_as_errors = warnings_as_errors,
+      .warnings_out = &warnings_out_cpp,
+  };
+  absl::StatusOr<std::string> result = xls::ConvertDslxPathToIr(path, options);
+
+  if (warnings_out != nullptr) {
+    xls::ToOwnedCStrings(warnings_out_cpp, warnings_out, warnings_out_count);
+  } else if (warnings_out_count != nullptr) {
+    *warnings_out_count = warnings_out_cpp.size();
+  }
 
-  absl::StatusOr<std::string> result = xls::ConvertDslxToIr(
-      dslx, path, module_name, dslx_stdlib_path, additional_search_paths_cpp);
   return xls::ReturnStringHelper(result, error_out, ir_out);
 }
 
 bool xls_convert_dslx_path_to_ir(const char* path, const char* dslx_stdlib_path,
                                  const char* additional_search_paths[],
                                  size_t additional_search_paths_count,
                                  char** error_out, char** ir_out) {
-  CHECK(path != nullptr);
-  CHECK(dslx_stdlib_path != nullptr);
-  CHECK(error_out != nullptr);
-
-  std::vector<std::filesystem::path> additional_search_paths_cpp =
-      xls::ToCpp(additional_search_paths, additional_search_paths_count);
-
-  absl::StatusOr<std::string> result = xls::ConvertDslxPathToIr(
-      path, dslx_stdlib_path, additional_search_paths_cpp);
-  return xls::ReturnStringHelper(result, error_out, ir_out);
+  const char* enable_warnings[] = {};
+  const char* disable_warnings[] = {};
+  return xls_convert_dslx_path_to_ir_with_warnings(
+      path, dslx_stdlib_path, additional_search_paths,
+      additional_search_paths_count, enable_warnings, 0, disable_warnings, 0,
+      /*warnings_as_errors=*/false,
+      /*warnings_out=*/nullptr,
+      /*warnings_out_count=*/nullptr, error_out, ir_out);
 }
 
 bool xls_optimize_ir(const char* ir, const char* top, char** error_out,
@@ -461,8 +541,22 @@ void xls_package_free(struct xls_package* p) {
   delete reinterpret_cast<xls::Package*>(p);
 }
 
-void xls_c_str_free(char* c_str) {
-  free(c_str);
+void xls_c_str_free(char* c_str) { free(c_str); }
+
+void xls_c_strs_free(char** c_strs, size_t count) {
+  CHECK_EQ(count == 0, c_strs == nullptr) << absl::StreamFormat(
+      "xls_c_strs_free expected consistency between zero-size and "
+      "array being nullptr; count = %d, c_strs = %p",
+      count, c_strs);
+
+  if (c_strs == nullptr) {
+    return;
+  }
+
+  for (size_t i = 0; i < count; ++i) {
+    free(c_strs[i]);
+  }
+  delete[] c_strs;
 }
 
 bool xls_value_to_string(const struct xls_value* v, char** string_out) {

xls/public/c_api.h

@@ -60,11 +60,39 @@ bool xls_convert_dslx_to_ir(const char* dslx, const char* path,
                             size_t additional_search_paths_count,
                             char** error_out, char** ir_out);
 
+// As above, but also takes `enable_warnings` and `disable_warnings` which
+// are arrays of warning names to enable or disable respectively against the
+// default warning set.
+//
+// Precondition: if `warnings_out` is provided then `warnings_out_count` must
+// also be provided.
+//
+// Note that if the `warnings_out_count` is populated with zero (i.e. there were
+// no warnings) then we can return `nullptr` for `warnings_out`.
+bool xls_convert_dslx_to_ir_with_warnings(
+    const char* dslx, const char* path, const char* module_name,
+    const char* dslx_stdlib_path, const char* additional_search_paths[],
+    size_t additional_search_paths_count, const char* enable_warnings[],
+    size_t enable_warnings_count, const char* disable_warnings[],
+    size_t disable_warnings_count, bool warnings_as_errors,
+    char*** warnings_out, size_t* warnings_out_count, char** error_out,
+    char** ir_out);
+
 bool xls_convert_dslx_path_to_ir(const char* path, const char* dslx_stdlib_path,
                                  const char* additional_search_paths[],
                                  size_t additional_search_paths_count,
                                  char** error_out, char** ir_out);
 
+// As above `xls_convert_dslx_to_ir_with_warnings`, but for a filesystem path
+// instead of DSLX text.
+bool xls_convert_dslx_path_to_ir_with_warnings(
+    const char* path, const char* dslx_stdlib_path,
+    const char* additional_search_paths[], size_t additional_search_paths_count,
+    const char* enable_warnings[], size_t enable_warnings_count,
+    const char* disable_warnings[], size_t disable_warnings_count,
+    bool warnings_as_errors, char*** warnings_out, size_t* warnings_out_count,
+    char** error_out, char** ir_out);
+
 bool xls_optimize_ir(const char* ir, const char* top, char** error_out,
                      char** ir_out);
 
@@ -237,6 +265,9 @@ void xls_package_free(struct xls_package* p);
 // just call `free` directly).
 void xls_c_str_free(char* c_str);
 
+// Frees an array of C strings that were provided by this XLS public API.
+void xls_c_strs_free(char** c_strs, size_t count);
+
 // Returns a string representation of the given IR package `p`.
 bool xls_package_to_string(const struct xls_package* p, char** string_out);
 

xls/public/c_api_dslx.cc

@@ -67,7 +67,7 @@ struct xls_dslx_import_data* xls_dslx_import_data_create(
     size_t additional_search_paths_count) {
   std::filesystem::path cpp_stdlib_path{dslx_stdlib_path};
   std::vector<std::filesystem::path> cpp_additional_search_paths =
-      xls::ToCpp(additional_search_paths, additional_search_paths_count);
+      xls::ToCppPaths(additional_search_paths, additional_search_paths_count);
   xls::dslx::ImportData import_data = CreateImportData(
       cpp_stdlib_path, cpp_additional_search_paths, xls::dslx::kAllWarningsSet,
       std::make_unique<xls::dslx::RealFilesystem>());

xls/public/c_api_impl_helpers.cc

@@ -24,13 +24,15 @@
 
 #include "absl/log/check.h"
 #include "absl/strings/str_format.h"
+#include "absl/types/span.h"
 #include "xls/ir/format_preference.h"
 #include "xls/public/c_api_format_preference.h"
 
 namespace xls {
 
-std::vector<std::filesystem::path> ToCpp(const char* additional_search_paths[],
-                                         size_t additional_search_paths_count) {
+std::vector<std::filesystem::path> ToCppPaths(
+    const char* additional_search_paths[],
+    size_t additional_search_paths_count) {
   std::vector<std::filesystem::path> additional_search_paths_cpp;
   additional_search_paths_cpp.reserve(additional_search_paths_count);
   for (size_t i = 0; i < additional_search_paths_count; ++i) {
@@ -41,10 +43,39 @@ std::vector<std::filesystem::path> ToCpp(const char* additional_search_paths[],
   return additional_search_paths_cpp;
 }
 
+std::vector<std::string_view> ToCppStringViews(const char* strings[],
+                                               size_t strings_count) {
+  std::vector<std::string_view> strings_cpp;
+  strings_cpp.reserve(strings_count);
+  for (size_t i = 0; i < strings_count; ++i) {
+    const char* string = strings[i];
+    CHECK(string != nullptr);
+    strings_cpp.push_back(string);
+  }
+  return strings_cpp;
+}
+
 char* ToOwnedCString(std::string_view s) { return strndup(s.data(), s.size()); }
 
-// Helper function that we can use to adapt to the common C API pattern when
-// we're returning an `absl::StatusOr<std::string>` value.
+void ToOwnedCStrings(absl::Span<const std::string> cpp, char*** c_out,
+                     size_t* c_out_count) {
+  CHECK(c_out != nullptr);
+  CHECK(c_out_count != nullptr);
+
+  // For the empty array case we avoid a needless zero-sized allocation.
+  if (cpp.empty()) {
+    *c_out = nullptr;
+    *c_out_count = 0;
+    return;
+  }
+
+  *c_out = new char*[cpp.size()];
+  *c_out_count = cpp.size();
+  for (size_t i = 0; i < cpp.size(); ++i) {
+    (*c_out)[i] = ToOwnedCString(cpp[i]);
+  }
+}
+
 bool ReturnStringHelper(absl::StatusOr<std::string>& to_return,
                         char** error_out, char** value_out) {
   if (to_return.ok()) {