Cleanup

Signed-off-by: G Ramalingam <grama@microsoft.com>

Author: gramalingam

docs/tutorial/rewriter/node_value_checkers.md

@@ -179,9 +179,16 @@ This means you should be careful when designing patterns with multiple alternati
 
 ## Error Handling
 
-Checkers can return either:
-- `True`: Check passed, continue matching
-- `False`: Check failed, pattern does not match
-- `MatchResult`: More detailed result with potential failure reasons
+Both check functions (including condition functions and node/value-level checkers) and
+rewrite functions support the same conventions for indicating failure:
 
-If a checker raises an exception, it will be caught and treated as a match failure, allowing patterns to fail gracefully when encountering unexpected conditions.
+- **`MatchResult` with `.fail()`** *(recommended)*: Return `MatchResult().fail("reason", source)` to indicate failure with a descriptive reason and optional source node/value. This provides the most useful diagnostic information for debugging.
+- **Raise `MatchFailureError`** *(recommended)*: Raise `MatchFailureError("reason", source_node_or_value)` to indicate failure. This is especially convenient in utility functions called from a check or rewrite, since it avoids having to explicitly propagate failure status through the call chain.
+- **Return `None` or `False`**: These indicate failure without providing a reason. They are supported but not recommended, since a failure reason is valuable for debugging why a rule did not apply.
+
+Including a descriptive failure reason is strongly encouraged. The rewriter's tracing infrastructure
+uses these reasons to report why rules failed to match, which is essential for diagnosing
+issues when developing or debugging rewrite rules.
+
+For **check functions**, success is indicated by returning `True` or a truthy `MatchResult`.
+For **rewrite functions**, success is indicated by returning one or more `ir.Value` results.

onnxscript/rewriter/_rewrite_rule.py

@@ -223,8 +223,9 @@ def get_replacement(self, match: _basics.MatchResult) -> ReplacementSubgraph | N
             match.fail(e.reason, list(e.failure_sources))
             return None
         # Support the same failure conventions as check functions for uniformity:
-        # - None or False: simple failure indicator (deprecated for False, but supported)
-        # - Falsy MatchResult: failure with reason/source info
+        # - None or False: indicates failure without a reason (not recommended)
+        # - Falsy MatchResult: failure with reason/source info (recommended)
+        # - MatchFailureError exception: failure with reason/source info (recommended)
         if new_outputs is None or new_outputs is False:
             return None
         if isinstance(new_outputs, _basics.MatchResult):

onnxscript/rewriter/pattern_base_test.py

@@ -322,8 +322,7 @@ def test_rewrite_returning_ir_value_succeeds(self):
 
         # Use a non-self-referential pattern to avoid infinite rewrite loops
         def add_zero_pattern(op, x):
-            zero = pattern.Constant(0.0)
-            return op.Add(x, zero)
+            return op.Add(x, 0.0)
 
         def identity_replacement(op, x):
             return op.Identity(x)