This is an archive of the discontinued LLVM Phabricator instance.

[pseudo] Add ForestNode descendants iterator, print ambiguous/opaque node stats.
ClosedPublic

Authored by sammccall on Jun 30 2022, 11:04 AM.

Diff Detail

Event Timeline

sammccall created this revision.Jun 30 2022, 11:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 11:04 AM
Herald added a subscriber: mgrang. · View Herald Transcript
sammccall requested review of this revision.Jun 30 2022, 11:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 30 2022, 11:04 AM

Results on the first 1k lines of SemaCodeComplete.cpp:

Forest bytes: 809824 nodes: 47378
GSS bytes: 24736 nodes: 41925

1490 Ambiguous nodes:
  518 type-name
  377 simple-type-specifier
  137 namespace-name
  135 nested-name-specifier
   65 postfix-expression
   45 decl-specifier-seq
   40 template-argument
   37 parameter-declaration
   26 relational-expression
   25 member-declaration
   21 virt-specifier
   18 function-definition
   16 mem-initializer-id
    9 literal
    7 user-defined-literal
    4 condition
    2 declaration-seq
    2 logical-or-expression
    2 simple-declaration
    2 statement
    1 logical-and-expression
    1 type-specifier-seq

0 Opaque nodes:
hokein accepted this revision.Jun 30 2022, 11:40 AM

Nice!

clang-tools-extra/pseudo/tool/ClangPseudo.cpp
68

I can foresee we will use it in other places (e.g. our internal metric tool), I think it is worth to expose it. It is ok for now.

70

By reading the code, I believe the unsigned and SymbolID here are swap accidentally.

This revision is now accepted and ready to land.Jun 30 2022, 11:40 AM
sammccall marked an inline comment as done.Jun 30 2022, 12:20 PM
sammccall added inline comments.
clang-tools-extra/pseudo/tool/ClangPseudo.cpp
68

I had the same thought, but I wasn't sure we'll want exactly the same thing.

e.g. the breakdown-by-SymbolID is a somewhat specialized data structure, our (private) mapreduce analysis might want to use mapreduce counters for this.

I'd rather just share the iterator for now, and only expose the simple aggregation stuff once we know it's actually reusable.

tschuett added inline comments.
clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
202

std::iterator is deprecated in C++17 and creates warnings.

hokein added inline comments.Aug 9 2022, 5:51 AM
clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
202

This seems to be fixed in d9e5462da61c3e2137a21a868a36f7022a39b59e.