Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate uses of bar.dyn_cast<Foo>() to dyn_cast<Foo>(bar) #394

Merged
merged 4 commits into from
Apr 3, 2024

Conversation

j2kun
Copy link
Contributor

@j2kun j2kun commented Apr 3, 2024

I tried building this with a more recent llvm commit, and saw all these deprecation warnings. Thought as a test PR I could migrate some of them. Note I couldn't migrate all of them without bumping the commit because dyn_cast is not supported for Affine*Expr at the current pinned commit.

```bash
perl -i -pe 's/([^ ]*)\.dyn_cast<([^>]*)>\(\)/llvm::dyn_cast<\2>(\1)/g' **/*.cpp **/*.h **/*.cc
```
@ivanradanov
Copy link
Collaborator

ivanradanov commented Apr 3, 2024

Thanks for the PR. Usually we use the mlir namespace, and it also has dyn_cast, so I don't think we need llvm::. That would make it a bit more concise.

Other than that you may need to run clang-format to get the format happy.

Copy link
Member

@ftynse ftynse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM modulo Ivan's comments. An LLVM version bump is also welcome.

@j2kun
Copy link
Contributor Author

j2kun commented Apr 3, 2024

A proper version bump will require a few breaking API migrations, so for now I'm working with the pinned LLVM commit, but I will opportunistically migrate as I become more familiar with the project.

@j2kun
Copy link
Contributor Author

j2kun commented Apr 3, 2024

Ready for a final check (and someone will need to merge it)

@j2kun j2kun changed the title Migrate uses of .dyn_cast to llvm::dyn_cast Migrate uses of bar.dyn_cast<Foo>() to dyn_cast<Foo>(bar) Apr 3, 2024
@ftynse ftynse merged commit 95ee215 into llvm:main Apr 3, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants