[PATCH] D72233: Add a new AST matcher 'optionally'.

Rihan Yang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 7 19:59:20 PST 2020


air20 marked an inline comment as done.
air20 added a comment.

In D72233#1808890 <https://reviews.llvm.org/D72233#1808890>, @aaron.ballman wrote:

> In D72233#1808125 <https://reviews.llvm.org/D72233#1808125>, @air20 wrote:
>
> > In D72233#1807698 <https://reviews.llvm.org/D72233#1807698>, @aaron.ballman wrote:
> >
> > > LGTM aside from a minor nit. Do you need someone to commit on your behalf?
> >
> >
> > How do I submit a patch from here?
>
>
> You would need git commit credentials for the project (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access). If you don't have those credentials, I'm happy to commit on your behalf.


I don't have commit access for now. It'd be great if you can submit it for me :D



================
Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:282
   REGISTER_MATCHER(hasInitStatement);
+  REGISTER_MATCHER(hasInitializer);
   REGISTER_MATCHER(hasKeywordSelector);
----------------
aaron.ballman wrote:
> air20 wrote:
> > aaron.ballman wrote:
> > > Why did this one move? Please keep the list sorted alphabetically.
> > I think this wasn’t sorted correctly before.
> I think it was sorted properly before (`i` comes before `S` even if you ignore capitalization), but regardless, it's a change unrelated to the patch, which we usually ask to be a separate patch.
I trust my Vim for this :) I think uppercase letters have a smaller ASCII value so they come first.

Line 354-355 are sorted correctly, for example.
```
REGISTER_MATCHER(isConstQualified);
REGISTER_MATCHER(isConstexpr);
```

I'd rather not upload another patch for this though.


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

https://reviews.llvm.org/D72233





More information about the cfe-commits mailing list