This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Update method cast calls to function calls
ClosedPublic

Authored by tpopp on May 11 2023, 2:17 AM.

Details

Summary

The MLIR classes Type/Attribute/Operation/Op/Value support
cast/dyn_cast/isa/dyn_cast_or_null functionality through llvm's doCast
functionality in addition to defining methods with the same name.
This change begins the migration of uses of the method to the
corresponding function call as has been decided as more consistent.

Note that there still exist classes that only define methods directly,
such as AffineExpr, and this does not include work currently to support
a functional cast/isa call.

Context:

Implementation:
This follows a previous patch that updated calls
op.cast<T>()-> cast<T>(op). However some cases could not handle an
unprefixed cast call due to occurrences of variables named cast, or
occurring inside of class definitions which would resolve to the method.
All C++ files that did not work automatically with cast<T>() are
updated here to llvm::cast and similar with the intention that they
can be easily updated after the methods are removed through a
find-replace.

See https://github.com/llvm/llvm-project/compare/main...tpopp:llvm-project:tidy-cast-check
for the clang-tidy check that is used and then update printed
occurrences of the function to include llvm:: before.

One can then run the following:

ninja clang-tidy

run-clang-tidy -clang-tidy-binary=$PWD/bin/clang-tidy -checks='-*,misc-cast-functions'\
                 -export-fixes /tmp/cast/casts.yaml mlir/*

./bin/clang-apply-replacements /tmp/cast/

Diff Detail

Event Timeline

tpopp created this revision.May 11 2023, 2:17 AM
tpopp requested review of this revision.May 11 2023, 2:17 AM
tpopp updated this revision to Diff 521278.May 11 2023, 5:53 AM

Also refactor header files

springerm accepted this revision.May 11 2023, 8:35 AM
This revision is now accepted and ready to land.May 11 2023, 8:35 AM
tpopp updated this revision to Diff 521345.May 11 2023, 9:28 AM

Undo some changes that were handled by parent commit.

rriddle accepted this revision.May 12 2023, 1:51 AM

LGTM! Nice! I'd ask if you can drop the llvm:: where it isn't necessary, but that can be more easily done with the class methods themselves are removed.

tpopp added a comment.May 12 2023, 2:02 AM

LGTM! Nice! I'd ask if you can drop the llvm:: where it isn't necessary, but that can be more easily done with the class methods themselves are removed.

Yes, I'm being a bit less perfect in the changes with the plan of doing a find-replace-all after the methods are removed. It's not perfect, but I suspect it will save me many hours, at least with the approaches I've thought of.