[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

This revision was automatically updated to reflect the committed changes. Closed by commit rL359868: C.128 override, virtual keyword handling (authored by teemperor, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.l

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

kwk added a comment. In D61440#1489064 , @teemperor wrote: > Looks good now, thanks! Thank you for being patient with me! Can you merge the change for me please? I don't have commit rights, yet. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

teemperor added a comment. Looks good now, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61440/new/ https://reviews.llvm.org/D61440 ___ lldb-commits mailing list lldb-commits@lists.llvm.org htt

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

kwk added a comment. @teemperor I've squashed my commits without the last one that did those many changes in the file `lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp`. I hope at least the result is fine now. Repository: rG LLVM Github Monorepo CHANGES SINC

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

kkleine updated this revision to Diff 197922. kkleine added a comment. - Address formatting issues brought up in https://reviews.llvm.org/D61440#1488450 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61440/new/ https://reviews.llvm.org/D61440 File

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

teemperor added a comment. You don't have to use arc, you can also just update the code via the "Update diff" button and upload a patch file. So it doesn't really matter how you organize your local git branch as long as you can produce a patch file from the changes. Phabricator takes care of ma

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

kwk added a comment. In D61440#1488470 , @teemperor wrote: > Not sure how you invoke git clang-format, but I assume you specified the > wrong commit range... That is correct in this case I've used clang format directly on the whole file. Sorry if that

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

teemperor added a comment. Not sure how you invoke git clang-format, but I assume you specified the wrong commit range... I personally use some custom command that executes this `git rebase -i --autosquash -x 'git clang-format master && git commit -a --amend --no-edit' master` which essentiall

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

kkleine updated this revision to Diff 197848. kkleine added a comment. - Address formatting issues brought up in https://reviews.llvm.org/D61440#1488450 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61440/new/ https://reviews.llvm.org/D61440 File

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

kwk added a comment. In D61440#1488450 , @teemperor wrote: > FYI, I still think there are some style issues there (e.g. the > AppleObjCRuntimeV2.cpp change makes it longer than 80 chars) but I can just > clang-format this before committing. > > Otherwise

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

teemperor accepted this revision. teemperor added a comment. FYI, I still think there are some style issues there (e.g. the AppleObjCRuntimeV2.cpp change makes it longer than 80 chars) but I can just clang-format this before committing. Otherwise, unless someone objects to this change I'll merg

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

teemperor added a comment. In D61440#1488424 , @kwk wrote: > In D61440#1488174 , @teemperor wrote: > > > LGTM module some minor code style issues (You removed some `virtual`s but > > didn't fix the indentation of t

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

kkleine updated this revision to Diff 197846. kkleine added a comment. - Fix https://reviews.llvm.org/D61440#inline-545102 - Fix https://reviews.llvm.org/D61440#inline-545098 - Fix https://reviews.llvm.org/D61440#inline-545099 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

kwk added a comment. @teemperor I would have loved to use clang-format for this but apparently the LLDB code is still manually formatted. It would be a bliss to just use the LLVM clang-format style and never ever talk about "style" again. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

kwk added a comment. In D61440#1488174 , @teemperor wrote: > LGTM module some minor code style issues (You removed some `virtual`s but > didn't fix the indentation of the parameters on the next line, see the inline > comments for examples). @teemperor

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM module some minor code style issues (You removed some `virtual`s but didn't fix the indentation of the parameters on the next line, see the inline comments for examples). ===

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

shafik added a comment. Thank you! LGTM, in general we avoid "large" refactoring changes to avoid polluting the blame list but the changes are relatively local and they are good changes that can catch real bugs in the future. I would like a second set of eyes though. Repository: rG LLVM Gi

[Lldb-commits] [PATCH] D61440: C.128 override, virtual keyword handling

kkleine created this revision. Herald added subscribers: lldb-commits, kadircet, arphaman. Herald added a project: LLDB. According to [C128] "Virtual functions should specify exactly one of `virtual`, `override`, or `final`", I've added override where a virtual function is overriden but the explic