[PATCH] D51699: [clang-tidy] minor bug fix to AbseilMatcher.h

[Kirill Bobyrev via Phabricator via llvm-commits]

kbobyrev added a comment.

Hi @hugoeg! Thank you for the update. Eric was too fast for me to catch up :)

I'm not sure about the _correctness_ and maintainability; I would probably suggest something like `warning: absl/flags is reserved for future use` for the part of Abseil which is not released yet (since it's hard to follow and might be confusing). Also, I'm not sure about hard-coding the library names in general: some pieces of Abseil can become deprecated, other ones are added eventually (like "flags" in this case). Would matching `absl/` substring be enough here? I can't see any good reason users would call their directories `absl/` (and not expect Clang-Tidy to produce some warnings :) and that would make it easier for the Abseil team, because you wouldn't have to update hard-coded library names in a specific Clang-Tidy file each release (which is very easy to forget). That would also simplify the code a lot, although it's not a concern for me.

@ioeric, @hugoeg what do you think?

Also, if you manually upload diff, please consider adding `-U999999` to your Git or SVN invocation (see LLVM's Phabricator guide <https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface> for reference). That would help reviewers to get the context of the file which is up for a review.


Repository:
  rL LLVM

https://reviews.llvm.org/D51699