# \[PSA\] Deprecating cast/isa methods in some classes

**URL:** <https://discourse.llvm.org/t/psa-deprecating-cast-isa-methods-in-some-classes/70909>\
**Category:** Deprecation & Important Refactoring\
**Created:** [May 26, 2023, 1:08pm UTC](https://discourse.llvm.org/t/psa-deprecating-cast-isa-methods-in-some-classes/70909 "2023-05-26T13:08:23Z")\
**Posts on this page:** 1\
**Page:** 1

<div class="post-metadata">

**Author:** ![tpopp](https://sea1.discourse-cdn.com/flex021/user_avatar/discourse.llvm.org/tpopp/32/429_2.png) [@tpopp](https://discourse.llvm.org/u/tpopp)\
**Post date:** [May 26, 2023, 1:08pm UTC](https://discourse.llvm.org/t/psa-deprecating-cast-isa-methods-in-some-classes/70909/1 "2023-05-26T13:08:23Z")

</div>

# [PSA] Deprecating {Attribute,Type,Value}::{cast,isa,isa\_and\_nonnull,dyn\_cast,dyn\_cast\_or\_null} methods

## Context

As noted on the deprecation page, [Deprecations & Current Refactoring - MLIR](https://mlir.llvm.org/deprecation/), and originally discussed on Discourse, [Preferred casting style going forward](https://discourse.llvm.org/t/preferred-casting-style-going-forward/68443), free function isa/cast/… functionality is preferred to method calls. Concretely:

```auto
// Preferred
llvm::cast<Value>(*this);

// Deprecated
this->cast<Value>();

```

At this point, `Attribute`, `Type`, `Value`, `Op`, `Location`, and `PointerUnion` classes and subclasses supported both function and method based casts. I have created a clang-tidy check to provide updating of most call sites of these methods to be function calls, and with some manual changes MLIR can mark these methods as deprecated.

One can see what these changes look like in the commits listed here: [Commits · llvm/llvm-project · GitHub](https://github.com/llvm/llvm-project/commits?author=tpopp&since=2023-04-30&until=2023-05-26)

## Request

Please comment if there are objections to this change, objections to how this is being handled, or requests for how to approach it. Otherwise, I hope to mark the methods in MLIR as deprecated next week and to remove the methods 1-2 months later.

As an example of a potentially valid concern, this change will result in mixed call styles because now some classes are updated to use function calls, while others only support method calls, so please comment if you consider this a blocking concern.

Below, one will find the tooling to automate a large portion of the changes, but I did the `.td` file changes manually with find-next + vim-macros

## What to Do

> **[GitHub - tpopp/llvm-project at tidy-cast-check](https://github.com/tpopp/llvm-project/tree/tidy-cast-check)**
>
> tidy-cast-check

The linked branch has 3 commits:

- 2 commits that provide clang-tidy tooling to handle almost all C++ code. They probably could have been combined, but I didn’t verify for sure, so I left them separately. Running the first commit and second commit one after the other covered all C++ code in MLIR. Running at only the second commit handled all of C++ code in Flang.
- 1 commit showing the deprecated methods. Due to `llvm::TypeSwitch` having a call to the dyn\_cast method, I did not mark those methods as deprecated.

Note: These commits only handle C++ code that can be analyzed, so C++ code inside `.td` files and in macros are not automatically updated.

Note: This clang-tidy does some additional changes that are not strictly necessary:

1. `llvm::PointerUnion` calls were updated, although this functionality cannot be easily removed after MLIR call sites are updated.
2. `dyn_cast_or_null` is replaced with `dyn_cast_if_present`

**Warning** : I wanted to begin this announcement, but, assuming no objections, I will update my branch one more time next week at the same time that I announce the deprecation and then update the commands here with the correct hashes.

As an example, updating Flang could be done with:

```auto
curl https://github.com/tpopp/llvm-project/compare/fe7afcf70c93223a16ec7a2a5e07c4ace16c9a04...tpopp:llvm-project:f5d2f889fe00d3396494ad8e605bbee47e046d20.patch | git apply
git add -u
git commit -m "temporary clang-tidy patch"
ninja -C $BUILD_DIR check-flang
ninja -C $BUILD_DIR clang-tidy
run-clang-tidy -clang-tidy-binary=$BUILD_DIR/bin/clang-tidy -checks='-*,misc-cast-functions' flang/* -header-filter=flang/ -fix

```

One will afterwards have to manually update any other code that is missed, such as the mentioned macros and `.td` files.

## What is Incomplete

I did not have time to carry this farther to completion. The process of deprecating Location’s methods is incomplete but should be short and easy when someone handles it. Furthermore, any classes that did not already provide the needed functionality for `llvm::cast()` style calls are not updated now. An example of this is `AffineExpr` which only provides the methods.

## Bonus Request

If one is an owner of a class, such as `AffineExpr`, please consider implementing LLVM’s `CastInfo` for your class, so the usage of cast/isa becomes more consistent. Almost all call sites of your class’ methods should be updated simply by patching my clang-tidy script and adding a call `AddMatchers("::mlir::YourClass", "Callee");` before rerunning the `clang-tidy` process

### Thank you

All feedback is welcome  
Tres