[cfe-commits] r160265 - in /cfe/trunk: include/clang/Tooling/CommandLineClangTool.h lib/Tooling/CMakeLists.txt lib/Tooling/CommandLineClangTool.cpp test/CMakeLists.txt test/Tooling/clang-ast-dump.cpp tools/CMakeLists.txt tools/Makefile tools/clan

Manuel Klimek klimek at google.com
Thu Jul 26 05:37:46 PDT 2012


On Thu, Jul 26, 2012 at 2:22 PM, Alexander Kornienko <alexfh at google.com> wrote:
>> I think it will make the review easier if we don't mix up the feature
>> design discussion about the list-decl feature with the code review of
>> the filtering change, which Doug already said he's in favor of.
>
> Sounds reasonable. Here's a separate diff for -ast-dump-filter option.

One last nit:
+    bool TraverseDecl(Decl *D) {
+      if (filterMatches(D)) {

I'd probably do
if (NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
  if (filterMatches(ND)) {
  }
}

That way we don't use the empty string to match. I know it's not a big
thing, but it seems slightly cleaner to me.

Apart from that looks good to me, but obviously needs approval from
Doug before submitting.

Cheers,
/Manuel



More information about the cfe-commits mailing list