This revision was automatically updated to reflect the committed changes.
Closed by commit rL290873: [clang-move] Only move used helper declarations.
(authored by hokein).
Changed prior to commit:
https://reviews.llvm.org/D27673?vs=82804&id=82847#toc
Repository:
rL LLVM
https://reviews.llvm
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Awesome! Let's ship it!
https://reviews.llvm.org/D27673
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
hokein updated this revision to Diff 82804.
hokein marked 3 inline comments as done.
hokein added a comment.
Add more test cases.
https://reviews.llvm.org/D27673
Files:
clang-move/CMakeLists.txt
clang-move/ClangMove.cpp
clang-move/ClangMove.h
clang-move/HelperDeclRefGraph.cpp
clang-mo
ioeric added inline comments.
Comment at: test/clang-move/move-used-helper-decls.cpp:1
+// RUN: mkdir -p %T/used-helper-decls
+// RUN: cp %S/Inputs/helper_decls_test* %T/used-helper-decls/
Can you also add test cases where class members are treated as the same n
hokein updated this revision to Diff 81763.
hokein added a comment.
Update outdated comment.
https://reviews.llvm.org/D27673
Files:
clang-move/CMakeLists.txt
clang-move/ClangMove.cpp
clang-move/ClangMove.h
clang-move/HelperDeclRefGraph.cpp
clang-move/HelperDeclRefGraph.h
test/clang-
hokein added a comment.
Thanks for the awesome suggestions on the names. I refactored the patch, hope
it is clearer now.
Comment at: clang-move/UsedHelperDeclFinder.cpp:30
+ Result = FD;
+ if (const auto *RD = dyn_cast(FD->getParent()))
+Result = RD;
---
hokein updated this revision to Diff 81762.
hokein marked 3 inline comments as done.
hokein added a comment.
refactoring the patch.
https://reviews.llvm.org/D27673
Files:
clang-move/CMakeLists.txt
clang-move/ClangMove.cpp
clang-move/ClangMove.h
clang-move/HelperDeclRefGraph.cpp
clang-
ioeric added a comment.
Code is almost good. I'm just still a bit confused by names.
Comment at: clang-move/ClangMove.cpp:459
//
- auto InOldCCNamedOrGlobalNamespace =
- allOf(hasParent(decl(a
hokein added inline comments.
Comment at: clang-move/ClangMove.cpp:492
+ isDefinition(), unless(InMovedClass), InOldCC,
+ anyOf(isStaticStorageClass(), hasParent(namespaceDecl(isAnonymous();
+ auto HelperFuncOrVar = namedDecl(anyOf(functionDecl(IsOldCCHelperDefinit
hokein updated this revision to Diff 81741.
hokein marked 15 inline comments as done.
hokein added a comment.
- address code review comments
- add more tests
https://reviews.llvm.org/D27673
Files:
clang-move/CMakeLists.txt
clang-move/ClangMove.cpp
clang-move/ClangMove.h
clang-move/Use
ioeric added inline comments.
Comment at: clang-move/UsedHelperDeclFinder.cpp:22
+// by a single node which belongs to that class.
+const Decl *getOutmostEnclosingClassOrFunDecl(const Decl *D) {
+ const auto *DC = D->getDeclContext();
Maybe just `getOutermostDec
ioeric added a comment.
Second rounds. Will start reviewing `CallGraph` code next.
Comment at: clang-move/ClangMove.cpp:492
+ isDefinition(), unless(InMovedClass), InOldCC,
+ anyOf(isStaticStorageClass(), hasParent(namespaceDecl(isAnonymous();
+ auto HelperFuncOr
hokein added inline comments.
Comment at: clang-move/ClangMove.cpp:492
+ isDefinition(), unless(InMovedClass), InOldCC,
+ anyOf(isStaticStorageClass(), hasParent(namespaceDecl(isAnonymous();
+ auto HelperFuncOrVar = namedDecl(anyOf(functionDecl(IsOldCCHelperDefinit
hokein updated this revision to Diff 81413.
hokein marked 18 inline comments as done.
hokein added a comment.
- Address review comments.
- Add more test cases.
https://reviews.llvm.org/D27673
Files:
clang-move/CMakeLists.txt
clang-move/ClangMove.cpp
clang-move/ClangMove.h
clang-move/Use
ioeric added a comment.
First round of comments.
Comment at: clang-move/ClangMove.cpp:492
+ isDefinition(), unless(InMovedClass), InOldCC,
+ anyOf(isStaticStorageClass(), hasParent(namespaceDecl(isAnonymous();
+ auto HelperFuncOrVar = namedDecl(anyOf(functionDecl
hokein updated this revision to Diff 81079.
hokein added a comment.
Fix code style.
https://reviews.llvm.org/D27673
Files:
clang-move/CMakeLists.txt
clang-move/ClangMove.cpp
clang-move/ClangMove.h
clang-move/UsedHelperDeclFinder.cpp
clang-move/UsedHelperDeclFinder.h
test/clang-move/
hokein created this revision.
hokein added a reviewer: ioeric.
hokein added a subscriber: cfe-commits.
Herald added a subscriber: mgorny.
Instead of moving all the helper declarations blindly, this patch
implement an AST-based call graph solution to make clang-move only move used
helper decls to n
17 matches
Mail list logo
