[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-19 Thread Mitchell via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGdf7086fe: [clang-tidy] modernize-use-override new option AllowOverrideAndFinal (authored by mitchell-stellar). Changed prior to commit: https://reviews.llvm.org/D70165?vs=229632=230040#toc

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-19 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar reopened this revision. mitchell-stellar added a comment. This revision is now accepted and ready to land. Reopening in order to correct the accidentally swapped commits between this ticket and https://reviews.llvm.org/D69238. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-19 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D70165#1751402 , @mitchell-stellar wrote: > Oops, it looks like I mixed up this ticket with > https://reviews.llvm.org/D69238. Sorry about that. Would you like me to > revert both commits and then re-commit with the

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-19 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added a comment. Oops, it looks like I mixed up this ticket with https://reviews.llvm.org/D69238. Sorry about that. Would you like me to revert both commits and then re-commit with the correct links, etc.? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D70165#1750606 , @mitchell-stellar wrote: > Yes, there was a failing unit test that had to be fixed. I reverted the first > commit and then committed a version that fixed the failing test. I mean, the commit message

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Mitchell via Phabricator via cfe-commits
mitchell-stellar added a comment. Yes, there was a failing unit test that had to be fixed. I reverted the first commit and then committed a version that fixed the failing test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70165/new/

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added subscribers: mitchell-stellar, lebedev.ri. lebedev.ri added a comment. @poelmanc @mitchell-stellar I'm confused by the diff - did this land? Was the correct patch committed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70165/new/

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-18 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. Ah, so it *really* landed in rG06f3dabe4a2e85a32ade27c0769b6084c828a206 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70165/new/

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-16 Thread Mitchell via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG50e99563fb04: [clang-tidy] modernize-use-override new option AllowOverrideAndFinal (authored by mitchell-stellar). Changed prior to commit: https://reviews.llvm.org/D70165?vs=229012=229632#toc

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-14 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc added a comment. In D70165#1745530 , @alexfh wrote: > While I have no objections against this patch, I wonder whether someone had a > chance to ask GCC developers about this? Is it a conscious choice to suggest > `override` when `final` is

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-14 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. While I have no objections against this patch, I wonder whether someone had a chance to ask GCC developers about this? Is it a conscious choice to suggest `override` when `final` is present? What's the argument for doing so? Repository: rCTE Clang Tools Extra

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-13 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc added a comment. In D70165#1744007 , @JonasToth wrote: > LGTM! > Did you check on a real code-base that suffers from the issue, if that works > as expected? Thanks! I have now run it on our real code base and it worked as expected. I lack

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-13 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. LGTM! Did you check on a real code-base that suffers from the issue, if that works as expected? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

2019-11-12 Thread Conrad Poelman via Phabricator via cfe-commits
poelmanc created this revision. poelmanc added reviewers: alexfh, djasper. Herald added subscribers: cfe-commits, mgehre. Herald added a project: clang. In addition to adding `override` wherever possible, clang-tidy's `modernize-use-override` nicely removes `virtual` when `override` or `final`