Return DataType directly from Val::getDataType() instead of optional

[wujingyue]

Summary

Val::getDataType() previously returned std::optional<DataType> and threw when dtype was Null, so the optional added no value and forced callers to use .value(), valueOrError(), or *.

Changes

• Val::getDataType() now returns DataType directly; still throws if dtype_ is Null.
• Statement::getDataType() removed (it only returned std::nullopt for Exprs; getDataType() is only used on Vals).
• Call sites updated throughout: removed .value(), valueOrError(), *, and .has_value() where they were unwrapping getDataType().
• fusion.cpp: removed redundant NVF_ERROR in aliasOutputToInput (check was redundant after the API change).

Build verified with _bn.

Made with Cursor

[github-actions]

Review updated until commit e53cfe5

Description

• Changed Val::getDataType() to return DataType directly instead of std::optional<DataType>, removing the need for callers to unwrap the optional

• Removed Statement::getDataType() as it only returned std::nullopt for Exprs and was unused

• Updated all call sites to remove .value(), valueOrError(), *, and .has_value() calls

• Removed redundant NVF_ERROR check in fusion.cpp aliasOutputToInput that became unnecessary after the API change

Changes walkthrough

	Relevant files

PR Reviewer Guide

Here are some key observations to aid the review process:

🧪 PR contains tests

⚡ Recommended focus areas for review

API Change Verification This PR makes a breaking API change where Val::getDataType() now returns DataType directly instead of std::optional<DataType>. While the PR description states it "still throws if dtype_ is Null", ensure that all callers that previously relied on the optional behavior (e.g., checking .has_value() before accessing) are properly handled. The diff shows removal of .value(), valueOrError(), and * dereferences, which appears correct, but a comprehensive test run should verify no runtime issues. indent(os, 1) << input << " " << input->getDataType() << '\n'; } os << "outputs:" << '\n'; for (auto output : sort_val_by_name(getAllOutputs(group))) { indent(os, 1) << output << " " << output->getDataType() << '\n'; Reference vs Pointer Member Change In the PredicateChecker class, member variables pred_elimination_ and non_predicated_exprs_ were changed from references (const PredicateElimination&, const std::unordered_set<const Expr*>&) to pointers (const PredicateElimination*, const std::unordered_set<const Expr*>*). This is reflected in the constructor initialization at lines 407-408 and the member declarations at lines 946-947. Ensure this change is intentional and doesn't introduce null pointer dereference risks. const PredicateElimination* pred_elimination_; const std::unordered_set<const Expr*>* non_predicated_exprs_;

[greptile-apps]

Greptile Summary

This PR refactors Val::getDataType() to return DataType directly instead of std::optional<DataType>. Since the method already throws when dtype is Null, the optional wrapper added no value and only forced callers to unwrap with .value(), valueOrError(), or *.

Changes:

• API simplification: Val::getDataType() signature changed from returning std::optional<DataType> to DataType
• Removed dead code: Statement::getDataType() removed entirely (only returned nullopt for Exprs)
• Mechanical updates: All 55 files updated to remove unwrapping operations (.value(), valueOrError(), *, .has_value())
• Cleanup: Removed redundant NVF_ERROR in fusion.cpp::aliasOutputToInput that checked has_value() (now redundant since getDataType() throws)
• Cleanup: Removed unreachable else branch in type_promotion.cpp::getValueType() that was dead code
• Modernization: Several files updated to use std::ranges algorithms

The refactor is consistent and thorough across all call sites. Build verified with _bn per PR description.

Confidence Score: 5/5

• This PR is safe to merge - it's a well-executed mechanical refactor with consistent changes across all call sites
• Large but mechanical refactor that simplifies the API. All getDataType() call sites consistently updated. No remaining unwrapping operations found. Build verified. The change actually improves code quality by removing unnecessary optional wrapper and cleaning up dead code.
• No files require special attention - all changes are mechanical and consistent

Important Files Changed

Filename	Overview
csrc/ir/base_nodes.h	Removed Statement::getDataType() and changed Val::getDataType() signature from returning std::optional<DataType> to DataType directly
csrc/ir/base_nodes.cpp	Updated getDataType() implementation to return DataType directly; removed .value() from call sites; modernized with std::ranges
csrc/type_promotion.cpp	Removed redundant has_value() checks and dead code; updated all getDataType() call sites to use direct return value
csrc/fusion.cpp	Removed redundant NVF_ERROR checking has_value() in aliasOutputToInput; updated getValType() and getDataType() call sites
csrc/codegen.cpp	Updated getDataType() call sites removing .value(); changed inline comments from C-style to C++ style; modernized with std::ranges
csrc/expr_simplifier.cpp	Removed dereference operators and .value() from getDataType() calls; modernized algorithm usage with std::ranges
csrc/device_lower/validation.cpp	Updated getDataType() call sites removing .value(); modernized with std::ranges::find and std::ranges::copy_if
csrc/ir/internal_nodes.cpp	Removed .value() unwrapping from getDataType() calls throughout; modernized with std::ranges::find

_{Last reviewed commit: e53cfe5}

[greptile-apps]

_{53 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{53 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[wujingyue]

!test

[greptile-apps]

_{53 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[wujingyue]

!test

[greptile-apps]

_{53 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{53 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{53 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{53 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[wujingyue]

!test

[wujingyue]

!test

[wujingyue]

!test

[greptile-apps]

Additional Comments (1)

csrc/ir/internal_nodes.cpp, line 1523
Inconsistent unwrapping: BroadcastOp uses valueOrError() but SqueezeOp doesn't unwrap the optional

  auto out_type = valueOrError(out->getValType());
  auto in_type = valueOrError(in->getValType());

[wujingyue]

!test