[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. In https://reviews.llvm.org/D34030#967749, @bruno wrote: > The change seems good to me in general. I wonder if this will hit any broken > assumption in the code. Did you run other tests beside unittests? I run the tests available by building the `clang-test` only. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Generally this looks good. Is there some way we can prevent this bug from recurring by construction (eg, by making a `return` ill-formed, or making it "just work")? Wrapping the code block in the `DEF_TRAVERSE_*` invocation in a lambda would help, but would probably significantly increase the compile-time (and possibly code size) cost of `RecursiveASTVisitor`, so I'd prefer a different solution if possible. We could move the post-code-block actions into the destructor of an RAII object, but then we can't suppress running them when the code block returns false. Ideas welcome. Comment at: include/clang/AST/RecursiveASTVisitor.h:1587 TRY_TO(TraverseType(D->getType())); return true; }) Same problem here? Comment at: include/clang/AST/RecursiveASTVisitor.h:2430 TRY_TO(TraverseDecl(S->getBlockDecl())); return true; // no child statements to loop through. }) Likewise. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
bruno added a comment. Herald added a subscriber: mgrang. The change seems good to me in general. I wonder if this will hit any broken assumption in the code. Did you run other tests beside unittests? https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Ping. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Ping. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Ping. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a reviewer: bruno. MontyKutyi added a comment. Ping. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Ping. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Ping. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi updated this revision to Diff 117367.
MontyKutyi added a comment.
Updated to the latest trunk version.
https://reviews.llvm.org/D34030
Files:
include/clang/AST/RecursiveASTVisitor.h
unittests/AST/PostOrderASTVisitor.cpp
Index: unittests/AST/PostOrderASTVisitor.cpp
===
--- unittests/AST/PostOrderASTVisitor.cpp
+++ unittests/AST/PostOrderASTVisitor.cpp
@@ -15,6 +15,7 @@
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Tooling/Tooling.h"
#include "gtest/gtest.h"
+#include
using namespace clang;
@@ -75,7 +76,33 @@
}
};
-}
+
+// Serializes the AST. It is not complete! It only serializes the Statement
+// and the Declaration nodes.
+class ASTSerializerVisitor : public RecursiveASTVisitor {
+private:
+ std::vector
+ const bool PostOrderTraverse;
+
+public:
+ ASTSerializerVisitor(bool PostOrderTraverse,
+ std::vector )
+ : VisitedNodes(VisitedNodes), PostOrderTraverse(PostOrderTraverse) {}
+
+ bool shouldTraversePostOrder() const { return PostOrderTraverse; }
+
+ bool VisitStmt(Stmt *S) {
+VisitedNodes.push_back(S);
+return true;
+ }
+
+ bool VisitDecl(Decl *D) {
+VisitedNodes.push_back(D);
+return true;
+ }
+};
+
+} // anonymous namespace
TEST(RecursiveASTVisitor, PostOrderTraversal) {
auto ASTUnit = tooling::buildASTFromCode(
@@ -126,3 +153,30 @@
ASSERT_EQ(expected[I], Visitor.VisitedNodes[I]);
}
}
+
+TEST(RecursiveASTVisitor, PrePostComparisonTest) {
+ auto ASTUnit = tooling::buildASTFromCode("template class X {};"
+ "template class X;");
+
+ auto TU = ASTUnit->getASTContext().getTranslationUnitDecl();
+
+ std::vector PreorderNodeList, PostorderNodeList;
+
+ ASTSerializerVisitor PreVisitor(false, PreorderNodeList);
+ PreVisitor.TraverseTranslationUnitDecl(TU);
+
+ ASTSerializerVisitor PostVisitor(true, PostorderNodeList);
+ PostVisitor.TraverseTranslationUnitDecl(TU);
+
+ // The number of visited nodes must be independent of the ordering mode.
+ ASSERT_EQ(PreorderNodeList.size(), PostorderNodeList.size());
+
+ std::sort(PreorderNodeList.begin(), PreorderNodeList.end());
+ std::sort(PostorderNodeList.begin(), PostorderNodeList.end());
+
+ // Both traversal must visit the same nodes.
+ ASSERT_EQ(std::mismatch(PreorderNodeList.begin(), PreorderNodeList.end(),
+ PostorderNodeList.begin())
+.first,
+PreorderNodeList.end());
+}
Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -1835,11 +1835,10 @@
TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc()));
\
if (!getDerived().shouldVisitTemplateInstantiations() &&
\
D->getTemplateSpecializationKind() != TSK_ExplicitSpecialization)
\
- /* Returning from here skips traversing the
\
- declaration context of the *TemplateSpecializationDecl
\
- (embedded in the DEF_TRAVERSE_DECL() macro)
\
- which contains the instantiated members of the template. */
\
- return true;
\
+ /* Skip traversing the declaration context of the
\
+ *TemplateSpecializationDecl (embedded in the DEF_TRAVERSE_DECL()
\
+ macro) which contains the instantiated members of the template. */
\
+ ShouldVisitChildren = false;
\
})
DEF_TRAVERSE_TMPL_SPEC_DECL(Class)
Index: unittests/AST/PostOrderASTVisitor.cpp
===
--- unittests/AST/PostOrderASTVisitor.cpp
+++ unittests/AST/PostOrderASTVisitor.cpp
@@ -15,6 +15,7 @@
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Tooling/Tooling.h"
#include "gtest/gtest.h"
+#include
using namespace clang;
@@ -75,7 +76,33 @@
}
};
-}
+
+// Serializes the AST. It is not complete! It only serializes the Statement
+// and the Declaration nodes.
+class ASTSerializerVisitor : public RecursiveASTVisitor {
+private:
+ std::vector
+ const bool PostOrderTraverse;
+
+public:
+ ASTSerializerVisitor(bool PostOrderTraverse,
+ std::vector )
+ : VisitedNodes(VisitedNodes), PostOrderTraverse(PostOrderTraverse) {}
+
+ bool shouldTraversePostOrder() const { return PostOrderTraverse; }
+
+ bool VisitStmt(Stmt *S) {
+VisitedNodes.push_back(S);
+return true;
+ }
+
+ bool VisitDecl(Decl *D) {
+VisitedNodes.push_back(D);
+return true;
+ }
+};
+
+} // anonymous namespace
TEST(RecursiveASTVisitor, PostOrderTraversal) {
auto ASTUnit = tooling::buildASTFromCode(
@@ -126,3
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Ping. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Ping. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Any further request on this? https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi updated this revision to Diff 108678.
MontyKutyi added a comment.
Did the renamings.
https://reviews.llvm.org/D34030
Files:
include/clang/AST/RecursiveASTVisitor.h
unittests/AST/PostOrderASTVisitor.cpp
Index: unittests/AST/PostOrderASTVisitor.cpp
===
--- unittests/AST/PostOrderASTVisitor.cpp
+++ unittests/AST/PostOrderASTVisitor.cpp
@@ -15,6 +15,7 @@
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Tooling/Tooling.h"
#include "gtest/gtest.h"
+#include
using namespace clang;
@@ -75,7 +76,33 @@
}
};
-}
+
+// Serializes the AST. It is not complete! It only serializes the Statement
+// and the Declaration nodes.
+class ASTSerializerVisitor : public RecursiveASTVisitor {
+private:
+ std::vector
+ const bool PostOrderTraverse;
+
+public:
+ ASTSerializerVisitor(bool PostOrderTraverse,
+ std::vector )
+ : VisitedNodes(VisitedNodes), PostOrderTraverse(PostOrderTraverse) {}
+
+ bool shouldTraversePostOrder() const { return PostOrderTraverse; }
+
+ bool VisitStmt(Stmt *S) {
+VisitedNodes.push_back(S);
+return true;
+ }
+
+ bool VisitDecl(Decl *D) {
+VisitedNodes.push_back(D);
+return true;
+ }
+};
+
+} // anonymous namespace
TEST(RecursiveASTVisitor, PostOrderTraversal) {
auto ASTUnit = tooling::buildASTFromCode(
@@ -126,3 +153,30 @@
ASSERT_EQ(expected[I], Visitor.VisitedNodes[I]);
}
}
+
+TEST(RecursiveASTVisitor, PrePostComparisonTest) {
+ auto ASTUnit = tooling::buildASTFromCode("template class X {};"
+ "template class X;");
+
+ auto TU = ASTUnit->getASTContext().getTranslationUnitDecl();
+
+ std::vector PreorderNodeList, PostorderNodeList;
+
+ ASTSerializerVisitor PreVisitor(false, PreorderNodeList);
+ PreVisitor.TraverseTranslationUnitDecl(TU);
+
+ ASTSerializerVisitor PostVisitor(true, PostorderNodeList);
+ PostVisitor.TraverseTranslationUnitDecl(TU);
+
+ // The number of visited nodes must be independent of the ordering mode.
+ ASSERT_EQ(PreorderNodeList.size(), PostorderNodeList.size());
+
+ std::sort(PreorderNodeList.begin(), PreorderNodeList.end());
+ std::sort(PostorderNodeList.begin(), PostorderNodeList.end());
+
+ // Both traversal must visit the same nodes.
+ ASSERT_EQ(std::mismatch(PreorderNodeList.begin(), PreorderNodeList.end(),
+ PostorderNodeList.begin())
+.first,
+PreorderNodeList.end());
+}
Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -1802,11 +1802,10 @@
TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc()));
\
if (!getDerived().shouldVisitTemplateInstantiations() &&
\
D->getTemplateSpecializationKind() != TSK_ExplicitSpecialization)
\
- /* Returning from here skips traversing the
\
- declaration context of the *TemplateSpecializationDecl
\
- (embedded in the DEF_TRAVERSE_DECL() macro)
\
- which contains the instantiated members of the template. */
\
- return true;
\
+ /* Skip traversing the declaration context of the
\
+ *TemplateSpecializationDecl (embedded in the DEF_TRAVERSE_DECL()
\
+ macro) which contains the instantiated members of the template. */
\
+ ShouldVisitChildren = false;
\
})
DEF_TRAVERSE_TMPL_SPEC_DECL(Class)
Index: unittests/AST/PostOrderASTVisitor.cpp
===
--- unittests/AST/PostOrderASTVisitor.cpp
+++ unittests/AST/PostOrderASTVisitor.cpp
@@ -15,6 +15,7 @@
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Tooling/Tooling.h"
#include "gtest/gtest.h"
+#include
using namespace clang;
@@ -75,7 +76,33 @@
}
};
-}
+
+// Serializes the AST. It is not complete! It only serializes the Statement
+// and the Declaration nodes.
+class ASTSerializerVisitor : public RecursiveASTVisitor {
+private:
+ std::vector
+ const bool PostOrderTraverse;
+
+public:
+ ASTSerializerVisitor(bool PostOrderTraverse,
+ std::vector )
+ : VisitedNodes(VisitedNodes), PostOrderTraverse(PostOrderTraverse) {}
+
+ bool shouldTraversePostOrder() const { return PostOrderTraverse; }
+
+ bool VisitStmt(Stmt *S) {
+VisitedNodes.push_back(S);
+return true;
+ }
+
+ bool VisitDecl(Decl *D) {
+VisitedNodes.push_back(D);
+return true;
+ }
+};
+
+} // anonymous namespace
TEST(RecursiveASTVisitor, PostOrderTraversal) {
auto ASTUnit = tooling::buildASTFromCode(
@@ -126,3 +153,30 @@
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
bruno added a comment. Nice! One more comment: please also fix names of local variables, member variables and function parameters. See https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly. Yes, there is precedence for violations in other places in this file, but that doesn't mean we should keep adding those :-) https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi updated this revision to Diff 108626.
MontyKutyi added a comment.
I run the clang-format with -style=llvm on the added code parts.
https://reviews.llvm.org/D34030
Files:
include/clang/AST/RecursiveASTVisitor.h
unittests/AST/PostOrderASTVisitor.cpp
Index: unittests/AST/PostOrderASTVisitor.cpp
===
--- unittests/AST/PostOrderASTVisitor.cpp
+++ unittests/AST/PostOrderASTVisitor.cpp
@@ -15,6 +15,7 @@
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Tooling/Tooling.h"
#include "gtest/gtest.h"
+#include
using namespace clang;
@@ -75,7 +76,32 @@
}
};
-}
+
+// Serializes the AST. It is not complete! It only serializes the Statement
+// and the Declaration nodes.
+class ASTSerializerVisitor : public RecursiveASTVisitor {
+private:
+ std::vector
+ const bool visitPostOrder;
+
+public:
+ ASTSerializerVisitor(bool visitPostOrder, std::vector )
+ : visitedNodes(visitedNodes), visitPostOrder(visitPostOrder) {}
+
+ bool shouldTraversePostOrder() const { return visitPostOrder; }
+
+ bool VisitStmt(Stmt *s) {
+visitedNodes.push_back(s);
+return true;
+ }
+
+ bool VisitDecl(Decl *d) {
+visitedNodes.push_back(d);
+return true;
+ }
+};
+
+} // anonymous namespace
TEST(RecursiveASTVisitor, PostOrderTraversal) {
auto ASTUnit = tooling::buildASTFromCode(
@@ -126,3 +152,30 @@
ASSERT_EQ(expected[I], Visitor.VisitedNodes[I]);
}
}
+
+TEST(RecursiveASTVisitor, PrePostComparisonTest) {
+ auto ASTUnit = tooling::buildASTFromCode("template class X {};"
+ "template class X;");
+
+ auto TU = ASTUnit->getASTContext().getTranslationUnitDecl();
+
+ std::vector preorderNodeList, postorderNodeList;
+
+ ASTSerializerVisitor PreVisitor(false, preorderNodeList);
+ PreVisitor.TraverseTranslationUnitDecl(TU);
+
+ ASTSerializerVisitor PostVisitor(true, postorderNodeList);
+ PostVisitor.TraverseTranslationUnitDecl(TU);
+
+ // The number of visited nodes must be independent of the ordering mode.
+ ASSERT_EQ(preorderNodeList.size(), postorderNodeList.size());
+
+ std::sort(preorderNodeList.begin(), preorderNodeList.end());
+ std::sort(postorderNodeList.begin(), postorderNodeList.end());
+
+ // Both traversal must visit the same nodes.
+ ASSERT_EQ(std::mismatch(preorderNodeList.begin(), preorderNodeList.end(),
+ postorderNodeList.begin())
+.first,
+preorderNodeList.end());
+}
Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -1802,11 +1802,10 @@
TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc()));
\
if (!getDerived().shouldVisitTemplateInstantiations() &&
\
D->getTemplateSpecializationKind() != TSK_ExplicitSpecialization)
\
- /* Returning from here skips traversing the
\
- declaration context of the *TemplateSpecializationDecl
\
- (embedded in the DEF_TRAVERSE_DECL() macro)
\
- which contains the instantiated members of the template. */
\
- return true;
\
+ /* Skip traversing the declaration context of the
\
+ *TemplateSpecializationDecl (embedded in the DEF_TRAVERSE_DECL()
\
+ macro) which contains the instantiated members of the template. */
\
+ ShouldVisitChildren = false;
\
})
DEF_TRAVERSE_TMPL_SPEC_DECL(Class)
Index: unittests/AST/PostOrderASTVisitor.cpp
===
--- unittests/AST/PostOrderASTVisitor.cpp
+++ unittests/AST/PostOrderASTVisitor.cpp
@@ -15,6 +15,7 @@
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Tooling/Tooling.h"
#include "gtest/gtest.h"
+#include
using namespace clang;
@@ -75,7 +76,32 @@
}
};
-}
+
+// Serializes the AST. It is not complete! It only serializes the Statement
+// and the Declaration nodes.
+class ASTSerializerVisitor : public RecursiveASTVisitor {
+private:
+ std::vector
+ const bool visitPostOrder;
+
+public:
+ ASTSerializerVisitor(bool visitPostOrder, std::vector )
+ : visitedNodes(visitedNodes), visitPostOrder(visitPostOrder) {}
+
+ bool shouldTraversePostOrder() const { return visitPostOrder; }
+
+ bool VisitStmt(Stmt *s) {
+visitedNodes.push_back(s);
+return true;
+ }
+
+ bool VisitDecl(Decl *d) {
+visitedNodes.push_back(d);
+return true;
+ }
+};
+
+} // anonymous namespace
TEST(RecursiveASTVisitor, PostOrderTraversal) {
auto ASTUnit = tooling::buildASTFromCode(
@@ -126,3 +152,30 @@
ASSERT_EQ(expected[I],
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Is there any particular parameter for the clang-format what I should use? If I just run it without any parameter it changes lines of the original test too. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
bruno added a comment. In https://reviews.llvm.org/D34030#821282, @MontyKutyi wrote: > Added test for the fix. Great! The test code you added doesn't seem to be compatible with clang style, can you run clang format and update the test? https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi updated this revision to Diff 108241.
MontyKutyi added a comment.
Added test for the fix.
https://reviews.llvm.org/D34030
Files:
include/clang/AST/RecursiveASTVisitor.h
unittests/AST/PostOrderASTVisitor.cpp
Index: unittests/AST/PostOrderASTVisitor.cpp
===
--- unittests/AST/PostOrderASTVisitor.cpp
+++ unittests/AST/PostOrderASTVisitor.cpp
@@ -15,6 +15,7 @@
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Tooling/Tooling.h"
#include "gtest/gtest.h"
+#include
using namespace clang;
@@ -75,7 +76,47 @@
}
};
-}
+ // Serializes the AST. It is not complete! It only serializes the Statement
+ // and the Declaration nodes.
+ class ASTSerializerVisitor
+: public RecursiveASTVisitor
+ {
+ private:
+std::vector& visitedNodes;
+const bool visitPostOrder;
+ public:
+
+ASTSerializerVisitor(bool visitPostOrder, std::vector& visitedNodes)
+ : visitedNodes (visitedNodes)
+ , visitPostOrder (visitPostOrder)
+{}
+
+bool shouldTraversePostOrder() const
+{
+ return visitPostOrder;
+}
+
+bool VisitStmt(Stmt *s)
+{
+ visitedNodes.push_back(s);
+ return true;
+}
+
+bool PostVisitStmt(Stmt *s)
+{
+ if (visitPostOrder)
+visitedNodes.push_back(s);
+ return true;
+}
+
+bool VisitDecl(Decl *d)
+{
+ visitedNodes.push_back(d);
+ return true;
+}
+ };
+
+} // anonymous namespace
TEST(RecursiveASTVisitor, PostOrderTraversal) {
auto ASTUnit = tooling::buildASTFromCode(
@@ -126,3 +167,32 @@
ASSERT_EQ(expected[I], Visitor.VisitedNodes[I]);
}
}
+
+TEST(RecursiveASTVisitor, PrePostComparisonTest) {
+ auto ASTUnit = tooling::buildASTFromCode(
+"template class X {};"
+"template class X;"
+ );
+
+ auto TU = ASTUnit->getASTContext().getTranslationUnitDecl();
+
+ std::vector preorderNodeList, postorderNodeList;
+
+ ASTSerializerVisitor PreVisitor(false, preorderNodeList);
+ PreVisitor.TraverseTranslationUnitDecl(TU);
+
+ ASTSerializerVisitor PostVisitor(true, postorderNodeList);
+ PostVisitor.TraverseTranslationUnitDecl(TU);
+
+ // The number of visited nodes must be independent of the ordering mode.
+ ASSERT_EQ(preorderNodeList.size(), postorderNodeList.size());
+
+ std::sort(preorderNodeList.begin(), preorderNodeList.end());
+ std::sort(postorderNodeList.begin(), postorderNodeList.end());
+
+ // Both traversal must visit the same nodes.
+ ASSERT_EQ(std::mismatch(preorderNodeList.begin(),
+ preorderNodeList.end(),
+ postorderNodeList.begin()).first,
+preorderNodeList.end());
+}
Index: include/clang/AST/RecursiveASTVisitor.h
===
--- include/clang/AST/RecursiveASTVisitor.h
+++ include/clang/AST/RecursiveASTVisitor.h
@@ -1802,11 +1802,10 @@
TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc())); \
if (!getDerived().shouldVisitTemplateInstantiations() && \
D->getTemplateSpecializationKind() != TSK_ExplicitSpecialization) \
- /* Returning from here skips traversing the \
- declaration context of the *TemplateSpecializationDecl\
- (embedded in the DEF_TRAVERSE_DECL() macro) \
- which contains the instantiated members of the template. */ \
- return true; \
+ /* Skip traversing the declaration context of the\
+ *TemplateSpecializationDecl (embedded in the DEF_TRAVERSE_DECL() \
+ macro) which contains the instantiated members of the template. */\
+ ShouldVisitChildren = false; \
})
DEF_TRAVERSE_TMPL_SPEC_DECL(Class)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
bruno added a comment. In https://reviews.llvm.org/D34030#819699, @MontyKutyi wrote: > As I can see the `clang/test` contains a lot of different simple tests, but > for testing this I think it is not enough to run the clang with some > arguments on a specific input. So I should create a new executable which > uses the postorder mode of the RecursiveASTVisitor. Am I right or is there > another preferred way for doing this? The place you're looking for here is in unittests. For example, see unittests/AST/PostOrderASTVisitor.cpp. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. As I can see the `clang/test` contains a lot of different simple tests, but for testing this I think it is not enough to run the clang with some arguments on a specific input. So I should create a new executable which uses the postorder mode of the RecursiveASTVisitor. Am I right or is there another preferred way for doing this? https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi updated this revision to Diff 108013. MontyKutyi added a comment. Updated the comment. https://reviews.llvm.org/D34030 Files: include/clang/AST/RecursiveASTVisitor.h Index: include/clang/AST/RecursiveASTVisitor.h === --- include/clang/AST/RecursiveASTVisitor.h +++ include/clang/AST/RecursiveASTVisitor.h @@ -1802,11 +1802,10 @@ TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc())); \ if (!getDerived().shouldVisitTemplateInstantiations() && \ D->getTemplateSpecializationKind() != TSK_ExplicitSpecialization) \ - /* Returning from here skips traversing the \ - declaration context of the *TemplateSpecializationDecl \ - (embedded in the DEF_TRAVERSE_DECL() macro) \ - which contains the instantiated members of the template. */ \ - return true; \ + /* Skip traversing the declaration context of the \ + *TemplateSpecializationDecl (embedded in the DEF_TRAVERSE_DECL() \ + macro) which contains the instantiated members of the template. */ \ + ShouldVisitChildren = false; \ }) DEF_TRAVERSE_TMPL_SPEC_DECL(Class) Index: include/clang/AST/RecursiveASTVisitor.h === --- include/clang/AST/RecursiveASTVisitor.h +++ include/clang/AST/RecursiveASTVisitor.h @@ -1802,11 +1802,10 @@ TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc())); \ if (!getDerived().shouldVisitTemplateInstantiations() && \ D->getTemplateSpecializationKind() != TSK_ExplicitSpecialization) \ - /* Returning from here skips traversing the \ - declaration context of the *TemplateSpecializationDecl\ - (embedded in the DEF_TRAVERSE_DECL() macro) \ - which contains the instantiated members of the template. */ \ - return true; \ + /* Skip traversing the declaration context of the\ + *TemplateSpecializationDecl (embedded in the DEF_TRAVERSE_DECL() \ + macro) which contains the instantiated members of the template. */\ + ShouldVisitChildren = false; \ }) DEF_TRAVERSE_TMPL_SPEC_DECL(Class) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
bruno added a comment. Hi Peter, Please add a testcase for this change, it also seems that you should update the comment. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Ping. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi added a comment. Could anybody take a look at on this please? Thanks, Peter https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.
MontyKutyi updated this revision to Diff 101929. MontyKutyi added a comment. Just added the whole file instead of a small surrounding to be able to check the `DEF_TRAVERSE_DECL` macro too. https://reviews.llvm.org/D34030 Files: include/clang/AST/RecursiveASTVisitor.h Index: include/clang/AST/RecursiveASTVisitor.h === --- include/clang/AST/RecursiveASTVisitor.h +++ include/clang/AST/RecursiveASTVisitor.h @@ -1795,7 +1795,7 @@ declaration context of the *TemplateSpecializationDecl \ (embedded in the DEF_TRAVERSE_DECL() macro) \ which contains the instantiated members of the template. */ \ - return true; \ + ShouldVisitChildren = false; \ }) DEF_TRAVERSE_TMPL_SPEC_DECL(Class) Index: include/clang/AST/RecursiveASTVisitor.h === --- include/clang/AST/RecursiveASTVisitor.h +++ include/clang/AST/RecursiveASTVisitor.h @@ -1795,7 +1795,7 @@ declaration context of the *TemplateSpecializationDecl\ (embedded in the DEF_TRAVERSE_DECL() macro) \ which contains the instantiated members of the template. */ \ - return true; \ + ShouldVisitChildren = false; \ }) DEF_TRAVERSE_TMPL_SPEC_DECL(Class) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
