This is an archive of the discontinued LLVM Phabricator instance.

[ASTMatchers][clang-tidy][NFC] Hoist 'forEachTemplateArgument' matcher into the core library
ClosedPublic

Authored by whisperity on May 11 2022, 6:35 AM.

Details

Summary

Fixes the FIXME: related to adding forEachTemplateArgument to the core AST Matchers library.

Diff Detail

Event Timeline

whisperity created this revision.May 11 2022, 6:35 AM
whisperity requested review of this revision.May 11 2022, 6:36 AM

Do you expect to use this matcher in a new check in the immediate future? We usually don't push specialized matchers into ASTMatchers.h but instead try to keep them next to the only check using them. This reduces compile time overhead for everyone building Clang. (It's not that there's anything wrong with this one, it's more just resistance to adding matches unless they're sufficiently necessary.)

Do you expect to use this matcher in a new check in the immediate future?

D124446 would like to use it.

aaron.ballman accepted this revision.May 12 2022, 8:14 AM

Do you expect to use this matcher in a new check in the immediate future?

D124446 would like to use it.

Thanks! Then I think the only thing missing here are updates to https://github.com/llvm/llvm-project/blob/main/clang/lib/ASTMatchers/Dynamic/Registry.cpp.

LGTM with that change made.

This revision is now accepted and ready to land.May 12 2022, 8:14 AM

Oh, you should probably also add a release note for the new matcher to clang/docs/ReleaseNotes.rst

  • Added to ASTMatchers/Registry.cpp
  • Updated with release notes

Could you please check if I did it the right way? I did not know about this file, and I am still not exactly sure about its purpose.

Could you please check if I did it the right way? I did not know about this file, and I am still not exactly sure about its purpose.

Yup, you did it correct! This is the magic that allows clang-query to work -- it sets up the dynamic mapping of C++ matchers, so if you forget to add something to that list, it'll be available to folks in C++ but not in clang-query.

This revision was landed with ongoing or failed builds.May 13 2022, 3:56 AM
This revision was automatically updated to reflect the committed changes.