teemperor created this revision.
teemperor added reviewers: zaks.anna, v.g.vassilev, doug.gregor, chandlerc.
teemperor added a subscriber: cfe-commits.
This patch adds postorder traversal support to the RecursiveASTVisitor.
This feature needs to be explicitly enabled by overriding
shouldTraverse
bkramer added a comment.
IMO a 20% release binary size increase is not acceptable. Is there a way to
compile in support only if it's actually used? Maybe some constexpr or template
magic?
http://reviews.llvm.org/D20382
___
cfe-commits mailing list
teemperor updated this revision to Diff 60176.
teemperor added a comment.
Reduced executable size bloat. Made postorder and preorder mutually exclusive
and postorder also uses the normal Visit* methods for callbacks.
http://reviews.llvm.org/D20382
Files:
include/clang/AST/RecursiveASTVisitor
teemperor added a comment.
Agreed. The new patch is now reusing the Visit methods so that the executable
size stays the same.
The downside is that you can no longer create a Visitor that is both post- and
preorder traversing,
but I don't think there is any major use case for that.
http://revie
teemperor added a comment.
ping
http://reviews.llvm.org/D20382
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
v.g.vassilev added a comment.
@bkramer are those modifications enough to accept this patch? It is holding
back quite a lot of ongoing development from @teemperor as part of his GSoC
project.
http://reviews.llvm.org/D20382
___
cfe-commits mailing l
bkramer accepted this revision.
bkramer added a comment.
This revision is now accepted and ready to land.
If there's no more executable size bloat this seems fine to me.
http://reviews.llvm.org/D20382
___
cfe-commits mailing list
cfe-commits@lists.l
rsmith added inline comments.
Comment at: include/clang/AST/RecursiveASTVisitor.h:630-635
@@ -593,1 +629,8 @@
+ if (getDerived().shouldTraversePostOrder()) {
+for (auto Iter = ReverseLocalQueue.rbegin();
+ Iter != ReverseLocalQueue.rend(); ++Iter) {
+ TRY_TO(Po
teemperor updated this revision to Diff 62015.
teemperor added a comment.
- Stmt traversal is now always postorder, even if child statements don't
support iterative traversal (thanks Richard).
http://reviews.llvm.org/D20382
Files:
include/clang/AST/RecursiveASTVisitor.h
unittests/AST/CMake
v.g.vassilev closed this revision.
v.g.vassilev added a comment.
Landed in r274348 and r274349.
http://reviews.llvm.org/D20382
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
teemperor added a comment.
The motivation for this patch is a hashing algorithm for all AST nodes
which reuses child hash values to be O(n) and therefore needs postorder support
(think Java's Object.hashCode() but on AST nodes as an example).
The full code that currently uses this feature can be
klimek added a reviewer: bkramer.
klimek added a comment.
Generally makes sense; adding d0k for additional thoughts.
http://reviews.llvm.org/D20382
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
bkramer added a comment.
Having postorder traversal makes sense to me. The thing I'm worried about is
how much this will bloat object code. RecursiveASTVisitor is already a major
contributor to the size of clang's binary and we've hit issues with it in the
past (hitting .obj size limits on Wind
teemperor added a comment.
Size changes to clang's binary (build with clang):
Release: 63367728 Byte -> 6326141 Byte (0.2% less, -106kB)
Debug: 1239669408 Byte -> 1264216256 Byte (2% increase, +24.5 MB)
http://reviews.llvm.org/D20382
___
cfe-commi
teemperor added a comment.
The previous stats were wrong (only applied this patch, not the patch using the
code):
Release:
63311672 Byte -> 77212960 Byte (+22% or +13.8 MB)
http://reviews.llvm.org/D20382
___
cfe-commits mailing list
cfe-commits@li
15 matches
Mail list logo
