[PATCH] D54408: [ASTMatchers] Add matchers available through casting to derived

Sat May 30 10:03:09 PDT 2020

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D54408#2056251 <https://reviews.llvm.org/D54408#2056251>, @steveire wrote:

> @aaron.ballman  I think we agreed in Belfast in November (after the most recent comment) to get this in as it is and not be as draconian about `auto`. Is anything blocking this?

Nothing is blocking this, but I think we may have slightly different understandings of what we agreed to. Reviewers asking for code to follow the coding guidelines is not draconian; it's expected for code to follow the coding guidelines and for patch authors to field requests for changes like these. I re-reviewed the things I was asking for changes on and have added a comment where I definitely insist on changes. LGTM with that nit fixed though.

================
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:705
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name) {
+  auto NestedResult = getMatchingMatchersImpl(StaticType, ExactOnly);
+
----------------
aaron.ballman wrote:
> Don't use `auto` here please.
Please change this use of `auto` -- I have no idea what the type is for `NestedResult` without hunting for it and so I have no idea what the type is for `Item` and whether using a const reference is reasonable or not (after doing the hunting, it looks fine though).

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54408/new/

https://reviews.llvm.org/D54408