[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

This revision was automatically updated to reflect the committed changes. Closed by commit rGb2666ccca027: [clangd] DefineOutline wont copy virtual specifiers on methods (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D75429?vs=247779=247828#toc Repository: rG LLVM

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land. Thanks, LGTM with a few small comments, please address those before landing. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:262 + } +

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

njames93 updated this revision to Diff 247779. njames93 added a comment. - Addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75429/new/ https://reviews.llvm.org/D75429 Files:

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

njames93 updated this revision to Diff 247766. njames93 added a comment. - Tweaked format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75429/new/ https://reviews.llvm.org/D75429 Files: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

kadircet added a comment. In D75429#1901120 , @njames93 wrote: > In D75429#1901073 , @kadircet wrote: > > > In D75429#1901018 , @njames93 > > wrote: > > > > > - Fixed clang

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

njames93 added a comment. In D75429#1901073 , @kadircet wrote: > In D75429#1901018 , @njames93 wrote: > > > - Fixed clang tidy warning, wont fix format as it's contradicting with the > > style of the rest of the

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:232 +} +CharSourceRange DelRange = CharSourceRange::getTokenRange( +AttrTokens->front().location(), AttrTokens->back().location()); let's use

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

kadircet added a comment. In D75429#1901018 , @njames93 wrote: > - Fixed clang tidy warning, wont fix format as it's contradicting with the > style of the rest of the function I believe the formatting linter messages results from the fact that you are

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

njames93 updated this revision to Diff 247640. njames93 added a comment. - Fixed clang tidy warning, wont fix format as it's contradicting with the style of the rest of the function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75429/new/

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

njames93 updated this revision to Diff 247621. njames93 marked 3 inline comments as done. njames93 added a comment. - added virtual virtual Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75429/new/ https://reviews.llvm.org/D75429 Files:

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

njames93 updated this revision to Diff 247620. njames93 added a comment. - Macro handling more in line with rest of clangd Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75429/new/ https://reviews.llvm.org/D75429 Files:

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:244 +bool Any = false; +// Clang allows duplicating virtual specifiers so check for multiple +// occurances. njames93 wrote: > kadircet wrote: > >

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:220 +assert(Attr->getLocation().isValid()); +if (Attr->getLocation().isMacroID()) { + Errors =

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

kadircet added a comment. In D75429#1900709 , @njames93 wrote: > I'm not entirely sure how to get the spelledTokens working in a good macro > safe way? I don't really follow this comment, could you elaborate? `TB.expandedTokens` always refer to a

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

njames93 added a comment. I'm not entirely sure how to get the spelledTokens working in a good macro safe way? Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:220 +assert(Attr->getLocation().isValid()); +if

[PATCH] D75429: [clangd] DefineOutline won't copy virtual specifiers on methods

njames93 updated this revision to Diff 247601. njames93 marked 8 inline comments as done. njames93 added a comment. - Refactor and extra test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75429/new/ https://reviews.llvm.org/D75429 Files: