[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

2020-05-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D80753#2065067 , @Daniel599 wrote: > Thank you @njames93, I already started and took a bit different approach. > In case of a conflict, leaving it to clang-tidy itself didn't help as it > added both of the fix-its together

[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp:4-8 +template +class initializer_list { +public: + initializer_list() noexcept {} +}; This isn't needed for the test case and

[PATCH] D80884: [ASTMatchers] Force c++ unittests to specify correct language standard

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: klimek, hokein, gribozavr, jvikstrom. Herald added a project: clang. Herald added a subscriber: cfe-commits. Force the unittests on c++ code for matchers to specify the correct standard. Repository: rG LLVM Github Monorepo

[PATCH] D80887: [clang-tidy] ignore builtin varargs from pro-type-vararg-check

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, mgehre. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. Herald added a project: clang. Disables the check from warning on some built in vararg functions, Address Clang-tidy should not consider

[PATCH] D80884: [ASTMatchers] Force c++ unittests to specify correct language standard

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D80884#2065270 , @gribozavr2 wrote: > This is an interesting question. Since Clang accepts certain constructs in > older C++ standard versions as extensions, shouldn't we test AST matchers in > those modes as well? I think

[PATCH] D80631: [clang-tidy] RenamerClangTidyChecks ignore builtin and command line macros

2020-05-30 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG44119626dedf: [clang-tidy] RenamerClangTidyChecks ignore builtin and command line macros (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80877: [clang-tidy] Added MacroDefiniton docs for readability-identifier-naming

2020-05-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Updates the docs to include `MacroDefinition` documentation. The docs are still missing `ObjCIVar` however I don't have a clue about

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

2020-05-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh. Herald added a project: clang. Herald added a subscriber: cfe-commits. Motivated by clang-tidy crashed for unknown command line argument. Repository: rG LLVM Github

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

2020-05-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 267483. njames93 added a comment. Fix compile error Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80879/new/ https://reviews.llvm.org/D80879 Files: clang-tools-extra/clang-query/tool/ClangQuery.cpp

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 267501. njames93 added a comment. Added test coverage. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80879/new/ https://reviews.llvm.org/D80879 Files: clang-tools-extra/clang-query/tool/ClangQuery.cpp

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf23ddbe3c3ae: clang-tidy and clang-query wont crash with invalid command line options (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D80879#2065364 , @thakis wrote: > Test fix looks good as far as I can tell. Landing and seeing what the bot > says is ok. It's fine for some of the bots to be red for a short while as > long as it isn't hours. Thanks! I'm

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 267505. njames93 added a comment. Actually fix the windows builds Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80879/new/ https://reviews.llvm.org/D80879 Files:

[PATCH] D80887: [clang-tidy] ignore builtin varargs from pro-type-vararg-check

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:21 +// FIXME: Add any more builtin variadics that shouldn't trigger this +static constexpr StringRef AllowedVariadics[]

[PATCH] D80631: [clang-tidy] RenamerClangTidyChecks ignore builtin and command line macros

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D80631#2065168 , @MaskRay wrote: > `arc` adds many unneeded tags from Phabricator. You can drop `Reviewers:` > `Subscribers:` `Tags:` and the text `Summary:` with the following script: > > arcfilter () { > arc amend >

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG595212569157: clang-tidy and clang-query wont crash with invalid command line options (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80887: [clang-tidy] ignore builtin varargs from pro-type-vararg-check

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 267497. njames93 added a comment. Remove unnecessary fixme. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80887/new/ https://reviews.llvm.org/D80887 Files:

[PATCH] D80887: [clang-tidy] ignore builtin varargs from pro-type-vararg-check

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:21 +// FIXME: Add any more builtin variadics that shouldn't trigger this +static constexpr StringRef AllowedVariadics[]

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 267504. njames93 added a comment. Hopefully fix windows test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80879/new/ https://reviews.llvm.org/D80879 Files:

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

2020-05-31 Thread Nathan James via Phabricator via cfe-commits
njames93 reopened this revision. njames93 added a comment. This revision is now accepted and ready to land. In D80879#2065350 , @thakis wrote: > This breaks check-clang-tools on windows: > http://45.33.8.238/win/16485/step_8.txt > > Looks like you need

[PATCH] D80884: [ASTMatchers] Force c++ unittests to specify correct language standard

2020-06-01 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb6d23f2efc64: [ASTMatchers] Force c++ unittests to specify correct language standard (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80896: [clang-tidy][misc-redundant-expression] Fix crash on CXXFoldExpr

2020-06-01 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. This fix has expanded from preventing the crash to adding support for `CXXFoldExpr` to misc-redundant-expression. Maybe rename the revision to explain that better. If that is the case then you may as well add a test case showing a redundant fold expression

[PATCH] D80536: [clang-tidy][modernize-loop-convert] Make loop var type human readable

2020-05-26 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I'm having trouble with the reproduction of this - https://godbolt.org/z/tsMfcj. Aside from that this needs some test cases to demonstrate the patch is indeed working Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-05-26 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:32 + return; +if (std::string(Info->getNameStart()) != Check.MacroName) + return; ``` if (Info->getName() !=

[PATCH] D69000: [clang-tidy] new check: modernize-deprecated-iterator-base

2020-05-27 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. First of all I'd definitely wait until a patch to include ASTMatcher support for CXXBaseSpecifier lands before progressing with this. Secondly this issue should be warned on by clang when compiling with c++17 with some stdlib support, libc++ should mark iterator as

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

2020-05-20 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D80202#2046266 , @klimek wrote: > Given the extra complexity I'd like to see that it matters - bound nodes tend > to be small. I put that in the description, but this is where i need help. whats the best way to benchmark

[PATCH] D80531: [clang-tidy]: Added modernize-replace-disallow-copy-and-assign-macro

2020-06-02 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM with 2 small nits, but I'd still give a few days see if anyone else wants to weight in. Comment at:

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281756. njames93 added a comment. Rebase from parent and address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84814/new/ https://reviews.llvm.org/D84814 Files:

[PATCH] D84812: [clang-tidy][NFC] Added convienence methods for getting optional options

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281752. njames93 added a comment. Revert to using warning for logging parsing errors Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84812/new/ https://reviews.llvm.org/D84812 Files:

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-08-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:33 +bool isExitFunction(StringRef FnName) { + return FnName == "_Exit" || FnName == "exit" || FnName == "quick_exit"; +} whisperity wrote: > aaron.ballman wrote:

[PATCH] D85028: [clangd] Support new/deleta operator in TargetFinder.

2020-08-01 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/FindTarget.cpp:466-467 + } + void VisitCXXDeleteExpr(const CXXDeleteExpr *CNE) { +Outer.add(CNE->getOperatorDelete(), Flags); + } nit: Repository: rG LLVM Github

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

2020-08-01 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 282379. njames93 marked 4 inline comments as done. njames93 added a comment. - Address comments. - Added test case for when this behaviour is disabled. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84814/new/

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-08-01 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D84898#2184931 , @vabridgers wrote: > I believe all review comments have been address, except for the discussion on > implementing this in the CFE or as a tidy check. Could try firing off an RFC to cfe-dev see if other

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

2020-08-01 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4888c9ce97d8: [clang-tidy] readability-identifier-naming checks configs for included files (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

2020-08-01 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 282380. njames93 added a comment. Fix unpunctuated comment and simplify second check command. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84814/new/ https://reviews.llvm.org/D84814 Files:

[PATCH] D84898: [clang-tidy] Add new checker for complex conditions with no meaning

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. For the record `X < Y < Z ` does have a mathematical meaning, Y is constrained between X and Z. However in the context of `C` the expression isnt parsed like that. If someone writes this they likely wanted `(X < Y) && (Y < Z)` For this specific check as you pointed out

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:14047 +static bool isComparisonOpSamePrecedence(BinaryOperatorKind Opc) { + switch (Opc) { Quuxplusone wrote: > Same precedence as what? > I think this should just be called

[PATCH] D82089: [clang-tidy] modernize-loop-convert reverse iteration support

2020-08-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 282757. njames93 added a comment. - Rebased trunk - Cleaned up test cases - Added support for specifying to include as system include. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82089/new/

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

2020-08-06 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Also with a name like compare op parenthesis. It sounds like this would consider `==` and `!=` Comment at: clang/lib/Sema/SemaExpr.cpp:14034 + << FixItHint::CreateInsertion(LHSExpr->getEndLoc(), ") && ") + <<

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

2020-08-11 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:210 GetClangTidyOptions(*TFS.view(/*CWD=*/llvm::None), File); + if (!Opts.ClangTidyOpts.Checks) { +// If the user hasn't configured clang-tidy checks at all, including

[PATCH] D84926: [clang-tidy][NFC] Use StringMap for ClangTidyCheckFactories::FacoryMap

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281862. njames93 added a comment. Fix build errors Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84926/new/ https://reviews.llvm.org/D84926 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp

[PATCH] D84924: [clang-tidy][WIP] Added command line option `fix-notes`

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. This is very much a work in progress Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84924/new/ https://reviews.llvm.org/D84924 ___ cfe-commits mailing list

[PATCH] D84868: [clang-tidy] Use StringMap for ClangTidyOptions::OptionsMap

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG45a720a86432: [clang-tidy] Use StringMap for ClangTidyOptions::OptionsMap (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84868/new/

[PATCH] D84926: [clang-tidy][NFC] Use StringMap for ClangTidyCheckFactories::FacoryMap

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D84926 Files:

[PATCH] D84902: [clang-tidy] Fix ODR violation in unittests.

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. LGTM, Thanks that bug was eating away at me for a good few days. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84902/new/ https://reviews.llvm.org/D84902

[PATCH] D84924: [clang-tidy] Added command line option `fix-notes`

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: alexfh, aaron.ballman, gribozavr2, Eugene.Zelenko, hokein. Herald added subscribers: cfe-commits, arphaman, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Added an option to control whether to apply

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

2020-07-31 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 282172. njames93 added a comment. Added Option GetConfigPerFile to control this behaviour. Ensure the check is enabled in the header files configuration before using any configuration found. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

2020-08-13 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:120 + // clangd doesn't replay those when using a preamble. + "-llvm-header-guard"); + static const std::string CrashingChecks =

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

2020-08-13 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:103 +Context->getOptions().CheckOptions) +.get("UseAssignment", 0) != 0) {} +

[PATCH] D81923: [clang-tidy] Add modernize-use-ranges check.

2020-08-04 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 282890. njames93 added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81923/new/ https://reviews.llvm.org/D81923 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

[PATCH] D85666: [clang-tidy] IncludeInserter: allow <> in header name

2020-08-11 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Should a line be added to the release notes to explain this behaviour: 'Checks that specify files to include now support wrapping the include in angle brackets to create a system include'? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D85697: [clang-tidy] Add cppcoreguidelines-prefer-scoped-enums-over-unscoped

2020-08-11 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Is 'over-unscoped' really needed in the name, would just 'prefer-scoped-enum' be better, WDYT? For the case of macro, you can check if the location is a macro using `SourceLocation::isMacroID()`. For this to work you would also need to check every single usage of the

[PATCH] D85666: [clang-tidy] IncludeInserter: allow <> in header name

2020-08-11 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp:98-103 + assert(SourceMgr && "SourceMgr shouldn't be null; did you remember to call " + "registerPreprocessor()?"); + bool IsAngled =

[PATCH] D85621: [clang] Allow DynTypedNode to store a TemplateArgumentLoc

2020-08-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D85621#2205856 , @nridge wrote: > In D85621#2205803 , @hokein wrote: > >> Please run `clang/docs/tools/dump_ast_matchers.py` script to update the >> `LibASTMatchersReference.html`

[PATCH] D84812: [clang-tidy][NFC] Added convienence methods for getting optional options

2020-07-31 Thread Nathan James via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1fd2049e38da: [clang-tidy][NFC] Added convienence methods for getting optional options (authored by njames93). Changed prior to commit:

[PATCH] D82815: [clang-tidy] Sanity checks in ClangTidyTest header.

2020-06-29 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added a reviewer: aaron.ballman. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Motivated by a suspicously failing build, but also good to have anyway in general. Repository: rG LLVM Github Monorepo

[PATCH] D82825: [clang-tidy] Added alias llvm-else-after-return.

2020-06-29 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. This is supposed to have a parent revision D82824 but for some reason phab won't let me add it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82825/new/ https://reviews.llvm.org/D82825

[PATCH] D82825: [clang-tidy] Added alias llvm-else-after-return.

2020-06-29 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, gribozavr2. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 added a comment. This is supposed to have a parent revision D82824 but for some