[PATCH] D77791: [ASTMatchers] Add support for dynamic matching of ofKind narrowing matcher

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbf968e28ee82: [ASTMatchers] Add support for dynamic matching of ofKind narrowing matcher (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77503: [ASTMatchers] Fixed CastKind being parsed incorrectly for dynamic matchers

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGdb71354e4ff1: [ASTMatchers] Fixed CastKind being parsed incorrectly for dynamic matchers (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77499: [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 255258. njames93 added a comment. - Remove format artefacts Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77499/new/ https://reviews.llvm.org/D77499 Files:

[PATCH] D77534: [clangd] DefineOutline: removes static token from static CXXMethodDecl

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: sammccall, kadircet, hokein. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. njames93 added a project: clang-tools-extra. Removes the `static` token when defining

[PATCH] D75911: [clang-tidy] Added hasAnyListedName matcher

2020-03-13 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/Matchers.h:53-55 +ast_matchers::internal::Matcher +hasAnyListedName(std::vector Names); + alexfh wrote: > We could change all checks to use

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

2020-03-15 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, klimek. Herald added a project: clang. Herald added a subscriber: cfe-commits. Extend hasReturnValue to match the last statement in a gnu statement expression if the last statement is an expression. Repository: rG LLVM

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

2020-03-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D76196#1923524 , @aaron.ballman wrote: > I don't think this is a natural fit for the functionality. A statement > expression doesn't have a return value so much as it is a value expression > that can contain multiple

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

2020-03-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D76196#1924393 , @aaron.ballman wrote: > I'm still not understanding your use case, so it's a bit hard for me to tell > whether this approach is good or not. Do you have a situation where you > cannot match on the

[PATCH] D76196: [ASTMatchers] Extend hasReturnValue to GNU StmtExpr

2020-03-16 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 250569. njames93 added a comment. - Rename matcher to hasFinalExpr - Don't review it yet Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76196/new/ https://reviews.llvm.org/D76196 Files:

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Also fix the test case, premerge found a failure Comment at: clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp:23 + + for (const Stmt *Child : TheStmt->children()) { +if (const auto *TheDeclRefExpr = dyn_cast(Child))

[PATCH] D77199: [clang-tidy] Address false positive in modernize-use-default-member-init

2020-04-01 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. Fixes incorrect warning emitted by "modernize-use-default-member-init" (new to 10.0.0)

[PATCH] D76606: [clang-tidy] Change checks that take enum configurations to use a new access method.

2020-04-04 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 255027. njames93 added a comment. Herald added subscribers: arphaman, kbarton, nemanjai. - Change checks that take enum configurations to use a new access method. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-04-04 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I added support for bools. The previous integer parsing behaviour is still there, however now it also responds to `true` or `false`. This won't parse `True|TRUE|False|FALSE` etc as I wanted it to be in line with `.clang-format` configuration files for handling of bool.

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-04-04 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 255023. njames93 added a comment. - Extended support for boolean types Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77085/new/ https://reviews.llvm.org/D77085 Files:

[PATCH] D77499: [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments

2020-04-05 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added a reviewer: klimek. Herald added subscribers: cfe-commits, mgorny. Herald added a reviewer: jdoerfert. Herald added a project: clang. njames93 edited the summary of this revision. Herald added a subscriber: dexonsmith. This adds support for giving

[PATCH] D77503: [ASTMatchers] Fixed CastKind being parsed incorrectly for dynamic matchers

2020-04-05 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. Requires hasCastKind arguments to have `CK_` prefixed to bring it in line with the documentation and other matchers that take enumerations. Repository: rG

[PATCH] D77493: [clang-tidy] Add do-not-refer-atomic-twice check

2020-04-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In my mind this check is definitely in the realm of the static analyser, clang-tidy just isn't designed for this. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:137 `cppcoreguidelines-avoid-goto `_, +

[PATCH] D76761: [clang-tidy] Fix crash in readability-redundant-string-cstr

2020-03-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D76761#1941476 , @gribozavr2 wrote: > Right -- what I meant is a more detailed description of why, for example, > `tryGetCallExprAncestorForCxxConstructExpr` can't find the `CallExpr` in this > case -- is it not there, or

[PATCH] D76761: [clang-tidy] Fix crash in readability-redundant-string-cstr

2020-03-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 252646. njames93 added a comment. - Fixed by replacing cumbersome code with the arguably proper solution Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76761/new/ https://reviews.llvm.org/D76761 Files:

[PATCH] D77199: [clang-tidy] Address false positive in modernize-use-default-member-init

2020-04-03 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG2c7ea1c4c5f7: [clang-tidy] Address false positive in modernize-use-default-member-init (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77199: [clang-tidy] Address false positive in modernize-use-default-member-init

2020-04-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 254870. njames93 added a comment. - Added test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77199/new/ https://reviews.llvm.org/D77199 Files:

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-04-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 254890. njames93 added a comment. - Address nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77085/new/ https://reviews.llvm.org/D77085 Files: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-04-03 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 8 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:25 + llvm::raw_svector_ostream Output(Buffer); + Output << "warning: Option not found '" << OptionName << "'\n"; + return

[PATCH] D76761: [clang-tidy] Fix crash in readability-redundant-string-cstr

2020-03-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D76761#1941070 , @gribozavr2 wrote: > Could you provide more information about why these null checks are needed in > this case? Well the fact it was crashing without those null checks was the reason they were added. I'm

[PATCH] D76496: [clang-tidy] StringFindStartswith should ignore 3-arg find()

2020-03-27 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D76496#1947059 , @niko wrote: > In D76496#1935127 , @njames93 wrote: > > > I'm not hugely familiar with the abseil library, but from what I can see > > `absl::StartsWith` takes

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

2020-03-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710 +return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName, +M1.Message) < + std::tie(M2.FilePath, M2.FileOffset,

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

2020-03-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I'm not entirely sure this is where the fix needs to be for this. The test case code is whacky as hell, but from what I can see clang should always emit a `BinaryOperator` for dependent type operators. No idea why it is spewing out a `CXXOperatorCallExpr` instead.

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

2020-03-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:708-710 +return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName, +M1.Message) < + std::tie(M2.FilePath, M2.FileOffset,

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

2020-03-29 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D76990#1948516 , @ztamas wrote: > I agree, it seems suspicious that a BinaryOperator matcher does not work in > this case. However, I'm working on this level of the code, I'm looking at the > matchers like an API, what I'm

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-03-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 253751. njames93 added a comment. - Fix assertions failing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77085/new/ https://reviews.llvm.org/D77085 Files: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

[PATCH] D76761: [clang-tidy] Fix crash in readability-redundant-string-cstr

2020-03-31 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3807079d705f: [clang-tidy] Fix crash in readability-redundant-string-cstr (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76761/new/

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-03-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, xazax.hun. Herald added a project: clang. njames93 added a comment. njames93 added a project: clang-tools-extra. This pretty much nulls out (clang-tidy) Warn on

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-03-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. This pretty much nulls out (clang-tidy) Warn on invalid "case" configurations for readability-identifier-naming I have moved that into a follow up patch that can be submitted now if you'd like. Repository: rG LLVM Github Monorepo

[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

2020-04-02 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. This check needs a store Options override method as it reading options from a configuration. Check other checks to see how this is done. Comment at: clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.h:35 +private: + std::string

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-04-02 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 254552. njames93 added a comment. - Small refactor Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77085/new/ https://reviews.llvm.org/D77085 Files: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

[PATCH] D76744: [clang-tidy] Add check to ensure llvm-libc implementations are defined in correct namespace.

2020-03-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. This check should only worry about the first declaration of the function, any redecls or the definition could appear outside the namespace like this: namespace blah{ void foo(); } void blah::foo(); void blah::foo(){} There are some missing test cases I'd

[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

2020-03-25 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6538b4393dc3: [clang-apply-replacements] No longer deduplucates replacements from the same TU (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76761: [clang-tidy] Fix crash in readability-redundant-string-cstr

2020-03-25 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76761 Files: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp

[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

2020-03-26 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. If you want to make it a general check, you should consider making the default module options set the correct namespace RequiredNamespace ClangTidyOptions getModuleOptions() override { ClangTidyOptions Options;

[PATCH] D77534: [clangd] DefineOutline: removes static token from static CXXMethodDecl

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 255298. njames93 added a comment. - Moved keyword removing logic into a lambda. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77534/new/ https://reviews.llvm.org/D77534 Files:

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

[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

[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

[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

[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

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

[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

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

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

[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)), +

[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] 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] 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 ) {} + aaron.ballman wrote: > I'd appreciate some

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

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

[PATCH] D75220: [clang-tidy] RenamerClangTidy now correctly renames `using namespace` decls

2020-03-18 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/D75220/new/ https://reviews.llvm.org/D75220 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D75220: [clang-tidy] RenamerClangTidy now correctly renames `using namespace` decls

2020-03-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 251190. njames93 added a comment. - Fixed comment nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75220/new/ https://reviews.llvm.org/D75220 Files:

[PATCH] D75220: [clang-tidy] RenamerClangTidy now correctly renames `using namespace` decls

2020-03-18 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1365ab4b63b7: [clang-tidy] RenamerClangTidy now correctly renames `using namespace` decls (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-03-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 251231. njames93 added a comment. - rebase and small nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73052/new/ https://reviews.llvm.org/D73052 Files:

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-18 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9bb5685b2161: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue… (authored by tetsuo-cpp, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75184: [clang-tidy] Optional inheritance of file configs from parent directories 

2020-03-20 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D75184#1932764 , @alexfh wrote: > There's one more thing to consider: just by looking at the name of a local > option we don't know whether it will be read using `get()` or > `getLocalOrGlobal()`. By removing local options

[PATCH] D76496: [clang-tidy] StringFindStartswith should ignore 3-arg find()

2020-03-21 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I'm not hugely familiar with the abseil library, but from what I can see `absl::StartsWith` takes `absl::string_view` as args. Therefore surely it makes sense to handle the 3rd arg for `str::find` by explicitly constructing the `absl::string_view` from the pointer and

[PATCH] D76545: [clang-tidy] Add a new check group 'experimental-'

2020-03-21 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I'm not sold on this being necessary as there is nothing against simply putting the check in the right module to begin with, then changing the registerCheck call to `experimenal-` as well as the docs. Could even do some fancy trickery with the python scripts to add

[PATCH] D76541: [clang-tidy][NFC] Add missing check group docs and order entries

2020-03-21 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Forgive me if I'm wrong, but these kinds of changes typically don't require a review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76541/new/ https://reviews.llvm.org/D76541

[PATCH] D76549: [clang-tidy] Fix RenamerClangTidy handling qualified TypeLocs

2020-03-21 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. Previously if a type was accessed with a qualifier, RenamerClangTidy wouldn't rename the TypeLoc, this patch addresses

[PATCH] D76549: [clang-tidy] Fix RenamerClangTidy handling qualified TypeLocs

2020-03-22 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 251910. njames93 added a comment. - Address `auto` and add test cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76549/new/ https://reviews.llvm.org/D76549 Files:

[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

2020-03-22 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D76054#1935533 , @ymandel wrote: > Thanks for expanding the description, including the helpful example. I'm not > sure, though, that this is the "right" behavior, at least not always. Worse, > I'm not sure there is a single

[PATCH] D76606: [clang-tidy] Warn on invalid "case" configurations for readability-identifier-naming

2020-03-23 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I'm not too sure how many other checks are like this, but I feel that this functionality could maybe be brought out of this check and into the Options to let more checks that take enum like configurations to use it. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

2020-03-23 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 252017. njames93 marked 5 inline comments as done. njames93 added a comment. - Address reviewer comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76054/new/ https://reviews.llvm.org/D76054 Files:

[PATCH] D76606: [clang-tidy] Warn on invalid "case" configurations for readability-identifier-naming

2020-03-23 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/readability/IdentifierNamingCheck.cpp:154 + llvm::errs() << "warning: Invalid case style '" << CaseValue + << "' for

[PATCH] D76606: [clang-tidy] Warn on invalid "case" configurations for readability-identifier-naming

2020-03-23 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. njames93 marked 2 inline comments as done. njames93 added a project: clang-tools-extra. njames93 added a comment. I'm not too sure how

[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

2020-03-23 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/test/clang-apply-replacements/identical2.cpp:3 +// RUN: grep -Ev "// *[A-Z-]+:" %S/Inputs/identical2/identical2.cpp > %T/Inputs/identical2/identical2.cpp +// RUN: sed "s#\$(path)#%/T/Inputs/identical2#"

[PATCH] D76549: [clang-tidy] Fix RenamerClangTidy handling qualified TypeLocs

2020-03-23 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7693a9b93140: [clang-tidy] Fix RenamerClangTidy handling qualified TypeLocs (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76549/new/

[PATCH] D73541: [Clang] Added method getTokenLocations to StringLiteral

2020-03-23 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/D73541/new/ https://reviews.llvm.org/D73541 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249121. njames93 added a comment. - Removed template in favour of specific overloads Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75714/new/ https://reviews.llvm.org/D75714 Files:

[PATCH] D75184: [clang-tidy] Optional inheritance of file configs from parent directories 

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. How are local and global options handled. For example in root .clang-tidy: CheckOptions: - key: some-check.GlobalOption value: '1' and in a subfolder .clang-tidy: CheckOptions: - key: GlobalOption value:

[PATCH] D76606: [clang-tidy] Warn on invalid "case" configurations for readability-identifier-naming

2020-03-24 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 3 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:149-151 +auto CaseOptionName = (Name + "Case").str(); +auto CaseValue = Options.get(CaseOptionName, ""); +auto

[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

[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] D80202: [ASTMatchers] Performance optimization for memoization

2020-05-20 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Running most of the clang tidy checks on the clang-tidy folder yields these results =BeforePatch=== RUN1: 4045.17user 83.93system 11:28.80elapsed 599%CPU (0avgtext+0avgdata 534024maxresident)k

[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 ) { + Options.store(Opts, "StringLikeClasses", I don't

[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:72 + binaryOperator(hasOperatorName("=="), + hasEitherOperand(ignoringParenImpCasts(StringNpos)), +

[PATCH] D80371: [clang-tidy] Fix potential assert in use-noexcept check

2020-05-21 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. Fix a potential assert in use-noexcept check if there is an issue getting the `TypeSourceInfo` as well as a small clean

[PATCH] D80371: [clang-tidy] Fix potential assert in use-noexcept check

2020-05-22 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Figured out the actual cause of this bug, `getExceptionSpecRange()` returns a null range if the function has an unknown return type undefined_type throws() throw(); This is the tidy output (where assertions are disabled) warning: dynamic exception specification

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

2020-05-27 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, xazax.hun. Herald added a project: clang. Fixes readability-identifier-naming option MacroDefinitionCase should ignore macros passed as parameters.

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

2020-05-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.cpp:40-44 + // FIXME(tdl-g): These options are being parsed redundantly with the + // constructor because TransformerClangTidyCheck forces us to provide MakeRule + //

[PATCH] D80697: [clang-tidy] Reworked TransformerClangTidyCheck to simplify usage of Options

2020-05-28 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. Changed `TransformerClangTidyCheck` to have a virtual method generate the rule. This has the advantages of making handling

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

2020-05-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp:550 + +#if __cplusplus > 201703L /* C++2a and later */ + bernhardmgruber wrote: > How do you want to handle these tests which require

[PATCH] D80371: [clang-tidy] Fix potential assert in use-noexcept check

2020-05-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 265923. njames93 added a comment. - Added test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80371/new/ https://reviews.llvm.org/D80371 Files:

[PATCH] D80371: [clang-tidy] Fix potential assert in use-noexcept check

2020-05-25 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4c5818dd8cd9: [clang-tidy] Fix potential assert in use-noexcept check (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D80371?vs=265923=265924#toc Repository: rG LLVM Github

[PATCH] D80371: [clang-tidy] Fix potential assert in use-noexcept check

2020-05-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D80371#2052069 , @aaron.ballman wrote: > LGTM, but please add a test case for the changes. As this fix is preventing a crash in error causing code I can't include a specific test case as the clang-tidy tests will fail if

[PATCH] D80371: [clang-tidy] Fix potential assert in use-noexcept check

2020-05-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 265899. njames93 added a comment. - Isolated exact cause of the assert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80371/new/ https://reviews.llvm.org/D80371 Files:

[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 a comment. In D80531#2053969 , @kwk wrote: > @Eugene.Zelenko thank you for the review. I've fixed all places that you've > mentioned. And have a question about one thing in particular. See inline. > > Do you by any chance know why

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

2020-05-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Few changes and nits then LGTM Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp:25 + const SourceManager ) + : Check(Check), PP(PP), SM(SM) {} + nit: You don't need to store a

[PATCH] D80697: [clang-tidy] Reworked TransformerClangTidyCheck to simplify usage of Options

2020-05-28 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 266957. njames93 added a comment. - Renamed makeRule to buildRule to avoid ambiguity with tooling::makeRule Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80697/new/ https://reviews.llvm.org/D80697 Files:

[PATCH] D80697: [clang-tidy] Reworked TransformerClangTidyCheck to simplify usage of Options

2020-05-29 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 267161. njames93 marked 3 inline comments as done. njames93 added a comment. - Add back old constructors, one being deprecated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80697/new/

[PATCH] D80697: [clang-tidy] Reworked TransformerClangTidyCheck to simplify usage of Options

2020-05-29 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. There are a few reasons for using the virtual method: - It keeps everything contained in the one class. - In the case where options are needed it avoid extra work in the derived class. - It's consistent with how all base class checks handle the specifics of derived

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

2020-05-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. It may be worth verifying that the fix-its are identical too, multiple versions of a check could be running with differing options resulting in different fix-its generated, in that case we should let clang-tidy disable any conflicting fixes for us. Side note would it

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

2020-05-30 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. LGMT, just a few minor nits though. Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:15-16 +#include "clang/Frontend/CompilerInstance.h" +#include +#include + Fairly

[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#2064857 , @Daniel599 wrote: > I have made the needed changes in order to detect a conflict in duplicated > fix-it of aliased checkers and also added a test for it. > Now I`ll try to work on combining aliased errors

<    1   2   3   4   5   6   7   8   9   >