Rework warning flags for {ir_converter,interpreter}_main.

xls/dslx/BUILD

@@ -712,6 +712,7 @@ cc_test(
         "//xls/common:xls_gunit_main",
         "//xls/common/status:matchers",
         "@com_google_googletest//:gtest",
+        "@com_google_absl//absl/status:status_matchers",
     ],
 )
 

xls/dslx/interpreter_main.cc

@@ -145,13 +145,8 @@ absl::StatusOr<TestResult> RealMain(
     std::optional<std::string_view> xml_output_file, EvaluatorType evaluator) {
   XLS_ASSIGN_OR_RETURN(
       WarningKindSet warnings,
-      WarningKindSetFromDisabledString(absl::GetFlag(FLAGS_disable_warnings)));
-
-  XLS_ASSIGN_OR_RETURN(
-      const WarningKindSet warnings_to_enable,
-      WarningKindSetFromString(absl::GetFlag(FLAGS_enable_warnings)));
-
-  warnings |= warnings_to_enable;
+      GetWarningsSetFromFlags(absl::GetFlag(FLAGS_enable_warnings),
+                              absl::GetFlag(FLAGS_disable_warnings)));
 
   RealFilesystem vfs;
 

xls/dslx/ir_convert/ir_converter_main.cc

@@ -93,9 +93,13 @@ absl::Status RealMain(absl::Span<const std::string_view> paths) {
   bool verify_ir = ir_converter_options.verify();
   bool convert_tests = ir_converter_options.convert_tests();
   bool warnings_as_errors = ir_converter_options.warnings_as_errors();
-  XLS_ASSIGN_OR_RETURN(WarningKindSet enabled_warnings,
-                       WarningKindSetFromDisabledString(
-                           ir_converter_options.disable_warnings()));
+
+  // Start with the default set, then enable the to-enable and then disable the
+  // to-disable.
+  XLS_ASSIGN_OR_RETURN(
+      WarningKindSet warnings,
+      GetWarningsSetFromFlags(ir_converter_options.enable_warnings(),
+                              ir_converter_options.disable_warnings()));
   std::optional<FifoConfig> default_fifo_config;
   if (ir_converter_options.has_default_fifo_config()) {
     XLS_ASSIGN_OR_RETURN(
@@ -107,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 = enabled_warnings,
+      .enabled_warnings = warnings,
       .convert_tests = convert_tests,
       .default_fifo_config = default_fifo_config,
   };

xls/dslx/ir_convert/ir_converter_options_flags.cc

@@ -55,6 +55,9 @@ ABSL_FLAG(bool, verify, true,
 ABSL_FLAG(std::optional<std::string>, disable_warnings, std::nullopt,
           "Comma-delimited list of warnings to disable -- not generally "
           "recommended, but can be used in exceptional circumstances");
+ABSL_FLAG(std::optional<std::string>, enable_warnings, std::nullopt,
+          "Comma-delimited list of warnings to enable -- this is only useful "
+          "if/when some warnings are disabled in the default warning set");
 ABSL_FLAG(bool, warnings_as_errors, true,
           "Whether to fail early, as an error, if warnings are detected");
 ABSL_FLAG(std::optional<std::string>, interface_proto_file, std::nullopt,

xls/dslx/ir_convert/ir_converter_options_flags.proto

@@ -36,4 +36,5 @@ message IrConverterOptionsFlagsProto {
   optional string interface_proto_file = 11;
   optional string interface_textproto_file = 12;
   optional FifoConfigProto default_fifo_config = 13;
+  optional string enable_warnings = 14;
 }

xls/dslx/tests/errors/error_modules_test.py

@@ -1286,6 +1286,18 @@ def test_unsized_array_type(self):
         stderr,
     )
 
+  def test_already_exhaustive_match_warning(self):
+    # Note: this flag is disabled by default, for now.
+    stderr = self._run(
+        'xls/dslx/tests/errors/unnecessary_trailing_match_pattern.x',
+        want_err_retcode=False)
+    self.assertNotIn('Match is already exhaustive', stderr)
+
+    stderr = self._run(
+        'xls/dslx/tests/errors/unnecessary_trailing_match_pattern.x',
+        enable_warnings={'already_exhaustive_match'}, want_err_retcode=True)
+    self.assertIn('Match is already exhaustive', stderr)
+
 
 if __name__ == '__main__':
   test_base.main()

xls/dslx/tests/errors/unnecessary_trailing_match_pattern.x

@@ -0,0 +1,21 @@
+// Copyright 2025 The XLS Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+fn f(x: bool) -> u32 {
+    match x {
+        true => u32:0,
+        false => u32:1,
+        _ => u32:2,
+    }
+}

xls/dslx/warning_kind.cc

@@ -101,4 +101,29 @@ absl::StatusOr<WarningKindSet> WarningKindSetFromString(
   return enabled;
 }
 
+std::string WarningKindSetToString(WarningKindSet set) {
+  std::vector<std::string_view> enabled_warnings;
+  for (WarningKind kind : kAllWarningKinds) {
+    if (WarningIsEnabled(set, kind)) {
+      enabled_warnings.push_back(WarningKindToString(kind).value());
+    }
+  }
+  return absl::StrJoin(enabled_warnings, ",");
+}
+
+absl::StatusOr<WarningKindSet> GetWarningsSetFromFlags(
+    std::string_view enable_warnings, std::string_view disable_warnings) {
+  XLS_ASSIGN_OR_RETURN(WarningKindSet enabled,
+                       WarningKindSetFromString(enable_warnings));
+  XLS_ASSIGN_OR_RETURN(WarningKindSet disabled,
+                       WarningKindSetFromString(disable_warnings));
+  if ((enabled & disabled) != kNoWarningsSet) {
+    return absl::InvalidArgumentError(absl::StrFormat(
+        "Cannot both enable and disable the same warning(s); enabled: %s "
+        "disabled: %s",
+        WarningKindSetToString(enabled), WarningKindSetToString(disabled)));
+  }
+  return (kDefaultWarningsSet | enabled) & Complement(disabled);
+}
+
 }  // namespace xls::dslx

xls/dslx/warning_kind.h

@@ -72,6 +72,24 @@ inline constexpr WarningKindSet kNoWarningsSet = WarningKindSet{0};
 inline constexpr WarningKindSet kAllWarningsSet =
     WarningKindSet{(WarningKindInt{1} << kWarningKindCount) - 1};
 
+// Set intersection.
+inline WarningKindSet operator&(WarningKindSet a, WarningKindSet b) {
+  return WarningKindSet{a.value() & b.value()};
+}
+
+// Set union.
+inline WarningKindSet operator|(WarningKindSet a, WarningKindSet b) {
+  return WarningKindSet{a.value() | b.value()};
+}
+
+// Returns the complement of a warning set.
+//
+// Note that we define this instead of operator~ because it has an existing
+// overload for STRONG_INT_TYPE.
+inline WarningKindSet Complement(WarningKindSet a) {
+  return WarningKindSet{~a.value() & kAllWarningsSet.value()};
+}
+
 // Disables "warning" out of "set" and returns that updated result.
 constexpr WarningKindSet DisableWarning(WarningKindSet set,
                                         WarningKind warning) {
@@ -93,8 +111,8 @@ inline bool WarningIsEnabled(WarningKindSet set, WarningKind warning) {
 // TODO(cdleary): 2025-02-03 Enable "already exhaustive match" by default after
 // some propagation time.
 inline constexpr WarningKindSet kDefaultWarningsSet = DisableWarning(
-    DisableWarning(kAllWarningsSet, WarningKind::kAlreadyExhaustiveMatch),
-    WarningKind::kShouldUseAssert);
+    DisableWarning(kAllWarningsSet, WarningKind::kShouldUseAssert),
+    WarningKind::kAlreadyExhaustiveMatch);
 
 // Converts a string representation of a warnings to its corresponding enum
 // value.
@@ -112,6 +130,14 @@ absl::StatusOr<WarningKindSet> WarningKindSetFromDisabledString(
 absl::StatusOr<WarningKindSet> WarningKindSetFromString(
     std::string_view enabled_string);
 
+std::string WarningKindSetToString(WarningKindSet set);
+
+// Returns the default warning set with the modifications given in flags.
+//
+// If flags are contradictory, returns an error.
+absl::StatusOr<WarningKindSet> GetWarningsSetFromFlags(
+    std::string_view enable_warnings, std::string_view disable_warnings);
+
 }  // namespace xls::dslx
 
 #endif  // XLS_DSLX_WARNING_KIND_H_

xls/dslx/warning_kind_test.cc

@@ -16,6 +16,7 @@
 
 #include <string_view>
 
+#include "absl/status/status_matchers.h"
 #include "gtest/gtest.h"
 #include "xls/common/status/matchers.h"
 
@@ -56,11 +57,59 @@ TEST(WarningKindTest, DefaultSetAnyMissing) {
       WarningIsEnabled(kDefaultWarningsSet, WarningKind::kShouldUseAssert));
 }
 
+TEST(WarningKindTest, Complement) {
+  EXPECT_EQ(Complement(kAllWarningsSet), kNoWarningsSet);
+  EXPECT_EQ(Complement(kNoWarningsSet), kAllWarningsSet);
+}
+
+TEST(WarningKindTest, SetIntersection) {
+  EXPECT_EQ(kAllWarningsSet & kAllWarningsSet, kAllWarningsSet);
+  EXPECT_EQ(kAllWarningsSet & kNoWarningsSet, kNoWarningsSet);
+  EXPECT_EQ(kNoWarningsSet & kAllWarningsSet, kNoWarningsSet);
+  EXPECT_EQ(kNoWarningsSet & kNoWarningsSet, kNoWarningsSet);
+  EXPECT_EQ(DisableWarning(kAllWarningsSet, WarningKind::kShouldUseAssert) &
+                EnableWarning(kNoWarningsSet, WarningKind::kShouldUseAssert),
+            kNoWarningsSet);
+}
+
+TEST(WarningKindTest, SetUnion) {
+  EXPECT_EQ(kAllWarningsSet | kAllWarningsSet, kAllWarningsSet);
+  EXPECT_EQ(kAllWarningsSet | kNoWarningsSet, kAllWarningsSet);
+  EXPECT_EQ(kNoWarningsSet | kAllWarningsSet, kAllWarningsSet);
+  EXPECT_EQ(kNoWarningsSet | kNoWarningsSet, kNoWarningsSet);
+  EXPECT_EQ(DisableWarning(kAllWarningsSet, WarningKind::kShouldUseAssert) |
+                EnableWarning(kNoWarningsSet, WarningKind::kShouldUseAssert),
+            kAllWarningsSet);
+}
+
 TEST(WarningKindTest, WarningKindSetFromString) {
   XLS_ASSERT_OK_AND_ASSIGN(WarningKindSet set,
                            WarningKindSetFromString("should_use_assert"));
   ASSERT_TRUE(WarningIsEnabled(set, WarningKind::kShouldUseAssert));
 }
 
+TEST(WarningKindTest, GetWarningsSetFromFlagsEmpty) {
+  XLS_ASSERT_OK_AND_ASSIGN(WarningKindSet set, GetWarningsSetFromFlags("", ""));
+  EXPECT_EQ(set, kDefaultWarningsSet);
+}
+
+TEST(WarningKindTest, GetWarningsSetFromFlagsEmptyEnable) {
+  XLS_ASSERT_OK_AND_ASSIGN(WarningKindSet set,
+                           GetWarningsSetFromFlags("", "constant_naming"));
+  EXPECT_EQ(set,
+            DisableWarning(kDefaultWarningsSet, WarningKind::kConstantNaming));
+}
+
+TEST(WarningKindTest, GetWarningsSetFromFlagsContradiction) {
+  absl::StatusOr<WarningKindSet> set =
+      GetWarningsSetFromFlags("constant_naming", "constant_naming");
+  EXPECT_THAT(set.status(),
+              absl_testing::StatusIs(
+                  absl::StatusCode::kInvalidArgument,
+                  testing::HasSubstr(
+                      "Cannot both enable and disable the same warning(s); "
+                      "enabled: constant_naming disabled: constant_naming")));
+}
+
 }  // namespace
 }  // namespace xls::dslx

xls/modules/zstd/memory/mem_reader.x

@@ -201,10 +201,6 @@ proc MemReaderInternal<
                     State { fsm: Fsm::REQUEST, ..state }
                 }
             },
-            _ => {
-                fail!("invalid_state", false);
-                state
-            },
         };
 
         next_state