[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcf39bdb49086: [clangd] Implement Decl canonicalization rules for rename (authored by kbobyrev). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 307013. kbobyrev marked an inline comment as done. kbobyrev added a comment. Get rid of forward declaration. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

hokein added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:78 +const NamedDecl *canonicalRenameDecl(const NamedDecl *D); + nit: maybe we can just move the function definition to here. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. Thanks, this looks great now. I think we can get rid of the `clangToolingRefactoring` dependence from clangd CMake files, clangd now should have no use of any functions there, could you take

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 306988. kbobyrev added a comment. Make use of TemplatedDecl. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files: clang-tools-extra/clangd/refactor/Rename.cpp

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 306987. kbobyrev marked 4 inline comments as done. kbobyrev added a comment. Resolve comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

hokein added a comment. Thanks, this looks nicer now. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:247 + if (const auto *Destructor = dyn_cast(D)) +return canonicalRenameDecl(Destructor->getParent()); + if (const auto *VarTemplate = dyn_cast(D))

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 306334. kbobyrev marked 8 inline comments as done. kbobyrev added a comment. Resolve review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev planned changes to this revision. kbobyrev added a comment. Need to address the comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 ___

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

hokein added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:274 +return canonicalRenameDecl(Function); + return D; +} I think we should call `getCanonicalDecl` before returning the final result (calling it at the beginning

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 306027. kbobyrev added a comment. Remove leftover test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files: clang-tools-extra/clangd/refactor/Rename.cpp Index:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:239 + if (const auto *Template = D->getPrimaryTemplate()) +return canonicalRenameDecl(Template); + return D; hokein wrote: > the `auto` + varies `canonicalRenameDecl`

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 306026. kbobyrev marked 2 inline comments as done. kbobyrev added a comment. Resolve some comments, simplify the code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

hokein added a comment. didn't finish all parts (looked at the FunctionDecl and RecordDecl), I think we need more documentation/comments in the code to specify the canonical behavior (templates are tricky). Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:239 + if

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304893. kbobyrev added a comment. Remove unused header (was added for llvm_unreachable). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304787. kbobyrev added a comment. Completely remove new tests from this patch (to be provided in a different one). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304784. kbobyrev added a comment. Add a test with static class member. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304785. kbobyrev added a comment. Remove (now) outdated comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files: clang-tools-extra/clangd/refactor/Rename.cpp

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304783. kbobyrev added a comment. Add missing user-defined conversion test and a couple of missing cases with ctor/dtor out-of-line implementation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304776. kbobyrev marked 5 inline comments as done. kbobyrev added a comment. - Handle VarDecl - Handle FunctionTemplateDecl - Remove FieldDecl handling (leave for the future patches) - Simplify code - Prevent regressions by adding more tests Repository:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

hokein added a comment. don't dig into details, first round of comments. I think the scope is a bit ambiguous, my reading is that it 1) removes old clang-rename API usage *and* 2) fixes some existing bugs. I would suggest just scoping it on 1). Comment at:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304729. kbobyrev added a comment. Use early return in FieldDecl canonicalization. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304500. kbobyrev added a comment. I think this is the first version that is ready for a review. Let me know if you have any questions, @hokein! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev planned changes to this revision. kbobyrev added a comment. Still WIP. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 ___ cfe-commits mailing list

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304466. kbobyrev added a comment. Rebase on top of master. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files: clang-tools-extra/clangd/refactor/Rename.cpp

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304464. kbobyrev added a comment. Actually reset RenameTests.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files: clang-tools-extra/clangd/refactor/Rename.cpp

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 304463. kbobyrev added a comment. Reset unittests to HEAD. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files: clang-tools-extra/clangd/refactor/Rename.cpp

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev planned changes to this revision. kbobyrev added a comment. Still WIP, moving test changes to a separate chain of patches (starts with D91102 ) to make review easier and start the pipeline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 303940. kbobyrev added a comment. Finish fields canonicalization. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 303733. kbobyrev added a comment. Store progress. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files: clang-tools-extra/clangd/refactor/Rename.cpp

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev updated this revision to Diff 303397. kbobyrev added a comment. Continue implementation, integrate more tests from clang-rename. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71880/new/ https://reviews.llvm.org/D71880 Files:

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61107 tests passed, 0 failed and 728 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev planned changes to this revision. kbobyrev added a comment. This is a WIP draft. Few items to address before pushing for a review - More tests: renaming is a complex feature and I want to make sure there are no regressions/corner-cases I did not handle. It might make sense to adopt a

[PATCH] D71880: [clangd] Implement Decl canonicalization rules for rename

kbobyrev created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This patch introduces new canonicalization rules which are used for AST-based rename in Clangd. By comparing two canonical