[Mlir-commits] [mlir] [MLIR][Toy] Fix incorrect return value in PrintExprAST codegen (PR #226924)

[llvmlistbot at llvm.org]

https://github.com/vaachang created https://github.com/llvm/llvm-project/pull/226924

This PR fixes a logic error in the Toy tutorial's `MLIRGen.cpp` where a failed
`toy.print` code generation incorrectly returned `mlir::success()` instead of
`mlir::failure()`.

### Problem

In `mlirGen(ExprASTList &blockAST)`, the code currently reads:

    if (auto *print = dyn_cast<PrintExprAST>(expr.get())) {
      if (mlir::failed(mlirGen(*print)))
        return mlir::success();
      continue;
    }

When `mlirGen(*print)` fails (e.g., due to an invalid argument or an internal
error), `mlir::failed(...)` is `true`. However, the function returns
`mlir::success()`, which tells the caller that the whole block was generated
successfully. As a result, `mlirGen(FunctionAST&)` does not erase the invalid
function and does not return `nullptr`. The module may later fail verification,
but the original error is masked.

### Fix

Change the erroneous `return mlir::success();` to `return mlir::failure();`:

    if (mlir::failed(mlirGen(*print)))
      return mlir::failure();

This makes the error handling consistent with the other failure paths in the
same function (e.g., variable declarations and generic expressions).

### Test

No new test is added because this is a straightforward logic fix in an example
tutorial. The change simply propagates the failure correctly; the existing
behavior for successful code generation is unchanged.

### Additional context

Found while reviewing the Toy tutorial code. The same pattern is already
correct in other error paths within the same file.

>From bc3a01b9dd12f16381bfe14fdb4d78fa5a7c059e Mon Sep 17 00:00:00 2001
From: vabchang <vabchang at qq.com>
Date: Mon, 28 Sep 2026 17:01:47 +0800
Subject: [PATCH] [MLIR][Toy] Fix error handling in PrintExprAST codegen

The `mlirGen` function for `PrintExprAST` incorrectly returned
`mlir::success()` when code generation failed. This patch corrects
it to return `mlir::failure()` to properly propagate the error.
---
 mlir/examples/toy/Ch2/mlir/MLIRGen.cpp | 2 +-
 mlir/examples/toy/Ch3/mlir/MLIRGen.cpp | 2 +-
 mlir/examples/toy/Ch4/mlir/MLIRGen.cpp | 2 +-
 mlir/examples/toy/Ch5/mlir/MLIRGen.cpp | 2 +-
 mlir/examples/toy/Ch6/mlir/MLIRGen.cpp | 2 +-
 mlir/examples/toy/Ch7/mlir/MLIRGen.cpp | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/mlir/examples/toy/Ch2/mlir/MLIRGen.cpp b/mlir/examples/toy/Ch2/mlir/MLIRGen.cpp
index a9592bcd7814a..e44a0977cd637 100644
--- a/mlir/examples/toy/Ch2/mlir/MLIRGen.cpp
+++ b/mlir/examples/toy/Ch2/mlir/MLIRGen.cpp
@@ -416,7 +416,7 @@ class MLIRGenImpl {
         return mlirGen(*ret);
       if (auto *print = dyn_cast<PrintExprAST>(expr.get())) {
         if (mlir::failed(mlirGen(*print)))
-          return mlir::success();
+          return mlir::failure();
         continue;
       }
 
diff --git a/mlir/examples/toy/Ch3/mlir/MLIRGen.cpp b/mlir/examples/toy/Ch3/mlir/MLIRGen.cpp
index 8c21951948496..a550ceef1af9d 100644
--- a/mlir/examples/toy/Ch3/mlir/MLIRGen.cpp
+++ b/mlir/examples/toy/Ch3/mlir/MLIRGen.cpp
@@ -416,7 +416,7 @@ class MLIRGenImpl {
         return mlirGen(*ret);
       if (auto *print = dyn_cast<PrintExprAST>(expr.get())) {
         if (mlir::failed(mlirGen(*print)))
-          return mlir::success();
+          return mlir::failure();
         continue;
       }
 
diff --git a/mlir/examples/toy/Ch4/mlir/MLIRGen.cpp b/mlir/examples/toy/Ch4/mlir/MLIRGen.cpp
index 6b7ab40299be5..644b706098114 100644
--- a/mlir/examples/toy/Ch4/mlir/MLIRGen.cpp
+++ b/mlir/examples/toy/Ch4/mlir/MLIRGen.cpp
@@ -420,7 +420,7 @@ class MLIRGenImpl {
         return mlirGen(*ret);
       if (auto *print = dyn_cast<PrintExprAST>(expr.get())) {
         if (mlir::failed(mlirGen(*print)))
-          return mlir::success();
+          return mlir::failure();
         continue;
       }
 
diff --git a/mlir/examples/toy/Ch5/mlir/MLIRGen.cpp b/mlir/examples/toy/Ch5/mlir/MLIRGen.cpp
index 6b7ab40299be5..644b706098114 100644
--- a/mlir/examples/toy/Ch5/mlir/MLIRGen.cpp
+++ b/mlir/examples/toy/Ch5/mlir/MLIRGen.cpp
@@ -420,7 +420,7 @@ class MLIRGenImpl {
         return mlirGen(*ret);
       if (auto *print = dyn_cast<PrintExprAST>(expr.get())) {
         if (mlir::failed(mlirGen(*print)))
-          return mlir::success();
+          return mlir::failure();
         continue;
       }
 
diff --git a/mlir/examples/toy/Ch6/mlir/MLIRGen.cpp b/mlir/examples/toy/Ch6/mlir/MLIRGen.cpp
index 6b7ab40299be5..644b706098114 100644
--- a/mlir/examples/toy/Ch6/mlir/MLIRGen.cpp
+++ b/mlir/examples/toy/Ch6/mlir/MLIRGen.cpp
@@ -420,7 +420,7 @@ class MLIRGenImpl {
         return mlirGen(*ret);
       if (auto *print = dyn_cast<PrintExprAST>(expr.get())) {
         if (mlir::failed(mlirGen(*print)))
-          return mlir::success();
+          return mlir::failure();
         continue;
       }
 
diff --git a/mlir/examples/toy/Ch7/mlir/MLIRGen.cpp b/mlir/examples/toy/Ch7/mlir/MLIRGen.cpp
index 73133247335dc..c72df8890b38d 100644
--- a/mlir/examples/toy/Ch7/mlir/MLIRGen.cpp
+++ b/mlir/examples/toy/Ch7/mlir/MLIRGen.cpp
@@ -640,7 +640,7 @@ class MLIRGenImpl {
         return mlirGen(*ret);
       if (auto *print = dyn_cast<PrintExprAST>(expr.get())) {
         if (mlir::failed(mlirGen(*print)))
-          return mlir::success();
+          return mlir::failure();
         continue;
       }