This is an archive of the discontinued LLVM Phabricator instance.

[flang] Update isa/cast method calls to function calls where possible
AcceptedPublic

Authored by tpopp on May 26 2023, 5:18 AM.

Details

Summary

This was mostly done automatically with
https://github.com/llvm/llvm-project/compare/main...tpopp:llvm-project:tidy-cast-check

Some manual changes were done for .td files and macros.

Context:

Diff Detail

Event Timeline

tpopp created this revision.May 26 2023, 5:18 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
tpopp requested review of this revision.May 26 2023, 5:18 AM

Why not using mlir::isa/mlir::dyn_cast? I know it's just an alias but most of the code in flang is using mlir:: right now so it would be more consistent.

There are couple of clang-format issues firing in the pre-commit ci.

tpopp updated this revision to Diff 526433.May 29 2023, 5:48 AM

Replace llvm:: namespaces with mlir:: and format code.

tpopp added a comment.May 29 2023, 5:51 AM

Why not using mlir::isa/mlir::dyn_cast? I know it's just an alias but most of the code in flang is using mlir:: right now so it would be more consistent.

There are couple of clang-format issues firing in the pre-commit ci.

Whoops, the format issues are hopefully resolved now. I replaced llvm:: instances with mlir:: instances as you asked. I had no preferences. In the mlir/ directory, the intention is to remove all the namespace prefixes as soon as the methods are removed, so the choice mattered less, but I agree mlir:: makes sense here.

tpopp added a comment.May 29 2023, 5:54 AM

Do you know if this would be acceptable? I assumed it wouldn't be allowed because it's a temporary check for a specific refactor as opposed to a generally agreed upon C++ style for example. I'm happy to share the code in any way; I just assumed upstreaming temporary code wasn't appropriate.

Do you know if this would be acceptable? I assumed it wouldn't be allowed because it's a temporary check for a specific refactor as opposed to a generally agreed upon C++ style for example. I'm happy to share the code in any way; I just assumed upstreaming temporary code wasn't appropriate.

The members are still there, so it would still be possible for someone to call them. But if you think that they will be swiftly removed it is probably ok for these to be cleaned up by hand.

Do you know if this would be acceptable? I assumed it wouldn't be allowed because it's a temporary check for a specific refactor as opposed to a generally agreed upon C++ style for example. I'm happy to share the code in any way; I just assumed upstreaming temporary code wasn't appropriate.

The members are still there, so it would still be possible for someone to call them. But if you think that they will be swiftly removed it is probably ok for these to be cleaned up by hand.

My plan is to remove all references before deprecating the methods and further remove any new occurrences at the time I remove the methods, so I'm prepared for the extra work. I didn't even consider the possibility of clang-tidy helping me with the enforcement! I also don't know if the build bots will enforce this for me when the methods are annotated as deprecated(?).

tblah added a subscriber: tblah.Jun 2 2023, 2:17 AM
This revision is now accepted and ready to land.Jun 5 2023, 1:28 AM