[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

JonasToth added a comment. In D58731#1413888 , @MyDeveloperDay wrote: > @JonasToth I left a comment in the commit needed to fix the index.rst, which > I don't think your later review fixes, sphinx complained about the rst file > being an unreferenced

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

MyDeveloperDay added a comment. @JonasToth I left a comment in the commit needed to fix the index.rst, which I don't think your later review fixes, sphinx complained about the rst file being an unreferenced octtree https://reviews.llvm.org/rG5fbeff797a9dba504f08f14c4fa59b6f1076fe72#inline-2691

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

JonasToth added a comment. Thank you for the patch! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

This revision was automatically updated to reflect the committed changes. Closed by commit rL355093: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions (authored by JonasToth, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

MyDeveloperDay added a comment. In D58731#1413695 , @alexfh wrote: > In D58731#1413552 , @JonasToth wrote: > > > In D58731#1413498 , > > @MyDeveloperDay wrote: > > > > > In

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added a comment. In D58731#1413552 , @JonasToth wrote: > In D58731#1413498 , @MyDeveloperDay > wrote: > > > In D58731#1413476 , @lewmpk wrote: > > > > > I'm happy to

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

JonasToth added a comment. In D58731#1413498 , @MyDeveloperDay wrote: > In D58731#1413476 , @lewmpk wrote: > > > I'm happy to land this ASAP but I don't have commit rights > > > So one of us could land it for

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

MyDeveloperDay added a comment. In D58731#1413476 , @lewmpk wrote: > I'm happy to land this ASAP but I don't have commit rights So one of us could land it for you.. (I've not personally done that before as I'm a bit green too! but I do have commit

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

lewmpk added a comment. I'm happy to land this ASAP but I don't have commit rights CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

MyDeveloperDay added a comment. In D58731#1412930 , @lewmpk wrote: > cleaned up documentation Are you planning on landing this anytime soon given that it was accepted? I would like to land D57087: [clang-tidy] add OverrideMacro to

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

lewmpk updated this revision to Diff 188642. lewmpk added a comment. cleaned up documentation CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/modernize-use-override.rst:5 == Will be good idea to remove duplicated empty line. Comment at:

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

lewmpk updated this revision to Diff 188638. lewmpk added a comment. - fixed tests - fixed typo in documentation CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added a comment. In D58731#1412843 , @alexfh wrote: > In D58731#1412831 , @lewmpk wrote: > > > I'm trying to find a way to run the test. If someone else has already > > tested it then yes please. > > > If

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added a comment. In D58731#1412831 , @lewmpk wrote: > I'm trying to find a way to run the test. If someone else has already tested > it then yes please. If you've set up the build with ninja, `ninja check-clang-tools` should do the right thing.

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

lewmpk added a comment. I'm trying to find a way to run the test. If someone else has already tested it then yes please. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731 ___ cfe-commits mailing list

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. Looks good! Do you need someone to commit the patch for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

lewmpk added a comment. thanks for the feedback everyone (and the warm welcome) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

lewmpk updated this revision to Diff 188628. lewmpk added a comment. addressed comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added inline comments. Comment at: test/clang-tidy/modernize-use-override-no-destructors.cpp:6 +struct Base { + virtual ~Base(){}; + virtual void f(){}; Remove the semicolons after methods. Clang-format the test (but ensure clang-format doesn't break

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added inline comments. Comment at: clang-tidy/modernize/UseOverrideCheck.h:23 + : ClangTidyCheck(Name, Context), +IgnoreDestructors(Options.get("IgnoreDestructors", false)) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override;

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

alexfh added a comment. Thank you for the contribution! Please see the comments inline. Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:22 // Only register the matcher for C++11. - if (getLangOpts().CPlusPlus11) -

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

lewmpk marked 2 inline comments as done and an inline comment as not done. lewmpk added inline comments. Comment at: clang-tidy/modernize/UseOverrideCheck.h:23 + : ClangTidyCheck(Name, Context), +IgnoreDestructors(Options.get("IgnoreDestructors", false)) {} void

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

lewmpk updated this revision to Diff 188622. lewmpk marked an inline comment as done. lewmpk added a comment. - addressed comments - provided tests - updated documentation - updated release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

MyDeveloperDay added a comment. Please resubmit this patch with full context git diff --cached -U99 > patch_to_submitt.diff You need to add some documention describing the new option into the check in docs/clang-tidy/checks/modernize .rst Repository: rCTE Clang Tools Extra

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

JonasToth added a comment. I think this change is worth mentioning in the release notes as well (cte/docs/ReleaseNotes.rst) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58731/new/ https://reviews.llvm.org/D58731

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

JonasToth added a comment. Welcome to the LLVM community and thank you for the patch lewmpk! Please add unit tests for the changes you made to the check. They live in `clang-tools-extra/test/clang-tidy/modernize-`. It is common to add a additional test-file to ensure the configuration

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

lewmpk created this revision. lewmpk added reviewers: alexfh, alexfh_. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. Herald added a project: clang. Addresses the bugzilla bug #30397. modernize-use-override suggests that destructors require the override specifier and the