[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

[PATCH] D82824: [clang-tidy] Added option to readability-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. Added a 'RefactorConditionVariables' option to control how the check handles condition variables Repository: rG LLVM Gi

[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 https://reviews.ll

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 274353. njames93 added a comment. Address doc backticks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82824/new/ https://reviews.llvm.org/D82824 Files: clang-tools-extra/clang-tidy/readability/ElseAfterRet

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

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 274359. njames93 added a comment. Address doc backticks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82825/new/ https://reviews.llvm.org/D82825 Files: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp

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

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/llvm-else-after-return.rst:3 +.. meta:: + :http-equiv=refresh: 5;URL=readability-else-after-return.html + aaron.ballman wrote: > Hmm

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst:74 + + When `true`, The check will attempt to refactor a variable defined inside + the condition of the ``if`` st

[PATCH] D82626: [CodeComplete] Tweak completion for else.

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 274469. njames93 added a comment. Address comments and add tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82626/new/ https://reviews.llvm.org/D82626 Files: clang/include/clang/Sema/Sema.h clang/lib/

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 274473. njames93 added a comment. Renamed new option to WarnOnConditionVariables Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82824/new/ https://reviews.llvm.org/D82824 Files: clang-tools-extra/clang-tidy/

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

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 274476. njames93 added a comment. Moved docs into target check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82825/new/ https://reviews.llvm.org/D82825 Files: clang-tools-extra/clang-tidy/llvm/LLVMTidyModu

[PATCH] D82706: [ASTMatchers] Enhanced support for matchers taking Regex arguments

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 274493. njames93 marked 2 inline comments as done. njames93 added a comment. Fix missing return statement Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82706/new/ https://reviews.llvm.org/D82706 Files: clan

[PATCH] D82706: [ASTMatchers] Enhanced support for matchers taking Regex arguments

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 7 inline comments as done. njames93 added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1952 +std::shared_ptr createAndVerifyRegex(StringRef Regex, + llvm::Regex::RegexFlags F

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

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG833273a81250: [clang-tidy] Sanity checks in ClangTidyTest header. (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82815/new/ https://r

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

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D82825#2122789 , @aaron.ballman wrote: > Other than the documentation and settling on the option name in the parent > revision, I think this LGTM (the changes will be mechanical and can be done > without further review once

[PATCH] D82626: [CodeComplete] Tweak completion for else.

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8ba4867c2700: [CodeComplete] Tweak completion for else. (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82626/new/ https://reviews.llv

[PATCH] D82720: [clang-tidy] performance-faster-string-find string-view

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG2efba0e8122a: [clang-tidy] performance-faster-string-find string-view (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D82720?vs=274200&id=274512#toc Repository: rG LLVM Gith

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

2020-06-30 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/readability/ElseAfterReturnCheck.cpp:193 if (checkConditionVarUsageInElse(If) != nullptr) { +if (!WarnOnConditionVariables) + return;

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

2020-06-30 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/readability/ElseAfterReturnCheck.cpp:193 if (checkConditionVarUsageInElse(If) != nullptr) { +if (!WarnOnConditionVariables) + return;

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, gribozavr2, hokein. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. Herald added a project: clang. Handle insertion fix-its when removing incompatible errors by introducting a new EventType `ET_I

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added a comment. Herald added a subscriber: wuzish. I've not added any specific unit tests for this, Don't think there would be much gained from them. The init-variables and isolate-declaration checks do a good enough job of demonstrating the f

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

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D82825#2123186 , @aaron.ballman wrote: > I can see arguments either way on this. Personally, I would not want the > check to diagnose this code because that would encourage people to widen the > scope and lifetime of `X` or

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa06a5ed97800: [clang-tidy] Added option to readability-else-after-return (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D82824?vs=274473&id=274569#toc Repository: rG LLVM G

[PATCH] D82706: [ASTMatchers] Enhanced support for matchers taking Regex arguments

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 274591. njames93 added a comment. Added best guess support for parsing RegexFlags Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82706/new/ https://reviews.llvm.org/D82706 Files: clang/docs/LibASTMatchersRef

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added subscribers: Eugene.Zelenko, njames93. njames93 added a comment. Please override `isLanguageVersionSupported` to restrict this check to just running on ObjectiveC translation units. Comment at: clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp:33 + Re

[PATCH] D82924: [clang-tidy] fix cppcoreguidelines-init-variables with catch variables

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, gribozavr2. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. Herald added a project: clang. Ignore catch statement var decls. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D8292

[PATCH] D82924: [clang-tidy] fix cppcoreguidelines-init-variables with catch variables

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 274655. njames93 added a comment. New line at end of file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82924/new/ https://reviews.llvm.org/D82924 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/InitV

[PATCH] D82924: [clang-tidy] fix cppcoreguidelines-init-variables with catch variables

2020-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 274654. njames93 added a comment. Herald added a subscriber: wuzish. Remove unneeded include Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82924/new/ https://reviews.llvm.org/D82924 Files: clang-tools-extra

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

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:109 SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes), -TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) { +

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

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

[PATCH] D66564: [clang-tidy] new altera struct pack align check

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D66564#2185335 , @ffrankies wrote: > Is there a way around this? And if not, do we ignore it? Lastly, do I need to > do anything else since this check has been accepted? There is a "Close > Revision" action in the comments, b

[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] 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: https://re

[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] 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: clang-tools-extr

[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 https://reviews.llvm.o

[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 people

[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 Monor

[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] 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 `isRelationalO

[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/ https://reviews.l

[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 clang-tools-e

[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(), ") && ") + << FixItHint::CreateInsertion(LHSBO

[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` file.

[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 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 https://revie

[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 = Header.consume_front("<"

[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] 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 = aar

[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) {} + `Opt

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

2020-05-07 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/utils/RenamerClangTidyCheck.h:44 + virtual void storeCheckOptions(ClangTidyOptions::OptionMap &Opts) {} + aaron.ballman wrote: > I'd appreciate

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

2020-05-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:99 IO.mapOptional("InheritParentConfig", Options.InheritParentConfig); +IO.mapOptional("UseColor", Options.UseColor); } I'm not entirely sure this option bel

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

2020-05-08 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 262865. njames93 added a comment. - Address nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73052/new/ https://reviews.llvm.org/D73052 Files: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

2020-05-08 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 262920. njames93 marked 3 inline comments as done. njames93 added a comment. - Tweaked options to remove second virtual method Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73052/new/ https://reviews.llvm.org/

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

2020-05-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Could this not use the `-fcolor-diagnostics` and `fno-color-diagnostics` command line flags that clang uses for its diagnostics to keep everything the same? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79477/new/ https://reviews.llvm.org/D79477 __

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

2020-05-09 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG82ddae061b4b: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D79674: [clang-tidy] Better support for Override function in RenamerClangTidy based checks

2020-05-09 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2, alexfh. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Methods that override virtual methods will now get renamed if the initial virtual method has a name violation. Repository

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

2020-05-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D79477#2027978 , @hyd-dev wrote: > None of the clang-tidy command line options are prefixed with `-f`. > This command line option used to be `--color-diagnostics`, but I've followed > @hokein's advice to change it to `--use-c

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

2020-05-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D79477#2028609 , @hyd-dev wrote: > This option sets `DiagOpts->ShowColors` to `true`. As I known, it controls > **all** diagnostics reported by the `clang-tidy` program (by > `clang::tidy::(anonymous namespace)::ErrorReporter

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

2020-05-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D79477#2028618 , @hyd-dev wrote: > > Not a fan of this test case as it only demonstrates the color behaviour of > > the process running the check not the actual option itself > > What does "option itself" mean? What I mean t

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:15 - `abseil-duration-addition `_, "Yes" - `abseil-duration-comparison `_, "Yes" - `abseil-duration-conversion-cast `_, "Yes" - `abseil-duration-division `_, "Yes" - `abseil

[PATCH] D80054: [ASTMatchers] Added BinaryOperator hasOperands matcher

2020-05-15 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: klimek, aaron.ballman, gribozavr2, alexfh. Herald added a project: clang. Herald added a subscriber: cfe-commits. Adds a matcher called `hasOperands` for `BinaryOperator`'s when you need to match both sides but the order isn't important, u

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:71-72 + binaryOperator(hasOperatorName("=="), + hasEitherOperand(ignoringParenImpCasts(StringNpos)), + hasEi

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:1-2 +//===--- StringFindStrContainsCheck.cc - clang-tidy---*- C++ +//-*-===// +// Don't need the C++ specifier in a cpp file. ``` //===---

[PATCH] D80054: [ASTMatchers] Added BinaryOperator hasOperands matcher

2020-05-17 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG74bcb00e00f3: [ASTMatchers] Added BinaryOperator hasOperands matcher (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80054/new/ https:

[PATCH] D80025: [ASTMatcher] Correct memoization bug ignoring direction (descendants or ancestors)

2020-05-18 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. This needs rebasing against trunk Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80025/new/ https://reviews.llvm.org/D80025 ___ cfe-commits mailing list cfe-commits@lists.llvm.

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

2020-05-19 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added a reviewer: klimek. Herald added a project: clang. Herald added a subscriber: cfe-commits. A simple optimization to avoid unnecessary copies of BoundNodesTreeBuilders when querying the cache for memoized matchers. This hasn't been benchmarked as I'm

[PATCH] D80023: [clang-tidy] Add abseil-string-find-str-contains checker.

2020-05-19 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:101-102 + +void StringFindStrContainsCheck::storeOptions( +ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "StringLikeClasses", I don't

[PATCH] D132786: [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition

2022-08-27 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6bd98b4f2df0: [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D132786?vs=456102&id=456139#toc Reposito

[PATCH] D132795: [clang-tidy] Tweak diagnostics for bugprone-assign-in-if-condition

2022-08-27 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2, LegalizeAdulthood, dodohand. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a s

[PATCH] D132795: [clang-tidy] Tweak diagnostics for bugprone-assign-in-if-condition

2022-08-27 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 456144. njames93 added a comment. Remove unnecessary includes added. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132795/new/ https://reviews.llvm.org/D132795 Files: clang-tools-extra/clang-tidy/bugprone/A

[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

2022-08-27 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. LGTM, just with a small nit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132640/new/ https://reviews.llvm.org/D132640 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D132795: [clang-tidy] Tweak diagnostics for bugprone-assign-in-if-condition

2022-08-27 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. njames93 marked an inline comment as done. Closed by commit rG32d88239ae65: [clang-tidy] Tweak diagnostics for bugprone-assign-in-if-condition (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D132

[PATCH] D132804: [clang-tidy] Add option to ignore CRTP overrides in convert-member-functions-to-static

2022-08-27 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2, LegalizeAdulthood, JonasToth. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a

[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

2022-08-27 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp:138 +on(hasType(hasCanonicalType(hasDeclaration( +namedDecl(hasAnyName(ContainersWithPushBack))); Sorr

[PATCH] D130665: [clang-tidy] Fix false negative in readability-convert-member-functions-to-static

2022-08-27 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130665/new/ https://reviews.llvm.org/D130665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D132804: [clang-tidy] Add option to ignore CRTP overrides in convert-member-functions-to-static

2022-08-27 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 456170. njames93 marked an inline comment as done. njames93 added a comment. Fix documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132804/new/ https://reviews.llvm.org/D132804 Files: clang-tools-

[PATCH] D132786: [clang-tidy] Fix a false positive in bugprone-assignment-in-if-condition

2022-08-29 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D132786#3755390 , @dodohand wrote: > IMO you have just introduced a bug, not fixed one. > A lambda expression in an if statement condition clause is exactly the kind > of thing that this checker was designed to flag. > The BA

[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

2022-08-29 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D132640#3757277 , @corona10 wrote: > @njames93 Can we land this patch before it occurs conflicting with other > changes? Yes, sorry do you have commit access. If you don't what is your GitHub name and email that I should us

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-08-30 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/readability/UseEarlyExitsCheck.cpp:66 + if (needsParensAfterUnaryNegation(Condition)) { +Diag << FixItHint::CreateInsertion(Condition->getBeginLoc(), "!(") +

[PATCH] D132998: [clang-tidy] Restrict use-equals-default to c++11-or-later

2022-08-31 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. There's going to be contention with this. The modernize module is designed for converting pre c++11 codebases to c++11, can't really do that if the check is disabled on those older codebases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

2022-08-31 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG495d984e14bd: [clang-tidy] Fix modernize-use-emplace to support alias cases (authored by corona10, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-09-01 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 457199. njames93 marked 3 inline comments as done. njames93 added a comment. Fix documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130181/new/ https://reviews.llvm.org/D130181 Files: clang-tools-

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-09-01 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst:63 +void Process(bool A, bool B) { + if (A && B) { +// Long processing. JonasToth wrote: > njames93 wrote: > > JonasToth wrote:

[PATCH] D133102: [clang-tidy] Extend simplify-boolean-expr check

2022-09-01 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, JonasToth, LegalizeAdulthood, alexfh. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subsc

[PATCH] D133102: [clang-tidy] Extend simplify-boolean-expr check

2022-09-01 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 457228. njames93 added a comment. Remove unnecessary includes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133102/new/ https://reviews.llvm.org/D133102 Files: clang-tools-extra/clang-tidy/readability/Simpl

[PATCH] D132550: Changes to code ownership in clang and clang-tidy

2022-09-02 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132550/new/ https://reviews.llvm.org/D132550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-09-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D130181#3769083 , @JonasToth wrote: > ... Your concerns aren't actually that important. Because the transformations only work on for binary operators, and not CXXOperatorCallExpr, it would always never do any special logic,

[PATCH] D133371: [clang-tidy][WIP] create API for extracting option types.

2022-09-06 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, LegalizeAdulthood, gribozavr2, JonasToth. Herald added subscribers: carlosgalvezp, mgrang, kbarton, xazax.hun, nemanjai. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-t

[PATCH] D133371: [clang-tidy][WIP] create API for extracting option types.

2022-09-06 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 458285. njames93 added a comment. Fix failing test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133371/new/ https://reviews.llvm.org/D133371 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp clang-t

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

2022-09-14 Thread Nathan James via Phabricator via cfe-commits
njames93 requested changes to this revision. njames93 added a comment. This revision now requires changes to proceed. Just address that test issue then it'll be good Comment at: clang-tools-extra/test/clang-tidy/checkers/misc/redundant-expression.cpp:1 -// RUN: %check_clang_ti

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

2022-09-14 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. What's with the .keep files. Will lit or sphinx fail if the cuda directory doesn't exist, if not I'd be happier removing them(they'd get removed once the first cuda check lands anyway). Comment at: clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp

[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

2022-06-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-unknown-expr.cpp:1 +// RUN: %check_clang_tidy -fix-errors %s bugprone-branch-clone %t + is -fix-errors necessary here, given we aren't verifying any fixi

[PATCH] D128402: [clang-tidy] Don't treat invalid branches as identical

2022-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D128402#3621002 , @ishaangandhi wrote: > Once you see it, can you either confirm `-fix-errors` was correct originally, > or instruct me on how to fix this test failure? `-expect-clang-tidy-error` is the technically correct

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

2022-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 441351. njames93 added a comment. Herald added subscribers: ormris, mgorny. Emit documentation links for the version of clangd built. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128379/new/ https://reviews.l

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

2022-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 441352. njames93 added a comment. Fix lit cfg typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128379/new/ https://reviews.llvm.org/D128379 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-ext

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

2022-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 requested review of this revision. njames93 added a comment. Any issues with this now for getting correct version? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128379/new/ https://reviews.llvm.org/D128379

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

2022-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D128379#3622286 , @sammccall wrote: > In D128379#3622128 , @sammccall > wrote: > >> Hmm, this version looks complicated to me. >> And also fragile: downstream we have CLANG_VERSION_ST

[PATCH] D128379: [clangd] Change the url for clang-tidy check documentation

2022-06-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 441499. njames93 added a comment. Changed to only use the major version. If this still seems too flaky, happy to go back to just linking to the in progress release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D127807: [clang-tidy] Properly forward clang-tidy output when running tests

2022-07-01 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. I'd say land this, but keep a close eye on the build bots as it may need reverting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127807/new/ https://reviews.llvm.org/D127807

<    1   2   3   4   5   6   7   8   9   10   >