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

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

[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 `_, + `cppcoreguidelines-avoid-non-

[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] 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: clang/include/clang/ASTMatchers/Dynamic/Diagnostic

[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] 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: clang-tools-extra/clangd/refac

[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 255387. njames93 marked 4 inline comments as done. njames93 added a comment. - Added test cases and addressed nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77499/new/ https://reviews.llvm.org/D77499 Fil

[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 added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:16-17 +if (Item.equals_lower(Search)) { + if (Item.equals(Search)) +return Item.str(); + MaxEditDistance = 1; aaron.ballman wrote: > Is this case possi

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

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG427c1dc4f424: [ASTMatchers] Matchers that take enumerations args provide hints with invalid… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

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

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 255440. njames93 added a comment. - Added support for giving suggestions on invalid inputs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77503/new/ https://reviews.llvm.org/D77503 Files: clang/lib/ASTMatche

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

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 9 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:294 + if (isa(FD) && cast(FD)->isStatic()) +DeleteKeyword(tok::kw_static, {FD->getBeginLoc(), FD->getLocation()});

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

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. As no FixIts are made the `IncludeInserter` can be removed. I'm struggling to see the value of this check though. If it was reworked to check for loop in the middle of a function it would have a lot more value. bool all_of = true; for (auto X : V) { if (!X) {

[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 255472. njames93 marked an inline comment as done. njames93 added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77534/new/ https://reviews.llvm.org/D77534 Files: clang-tools-ex

[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 255473. njames93 added a comment. - Fix issue with last revision Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77534/new/ https://reviews.llvm.org/D77534 Files: clang-tools-extra/clangd/refactor/tweaks/Defi

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

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG353a9883680e: [clangd] DefineOutline: removes static token from static CXXMethodDecl (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77

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

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfcf7cc268fe4: [clang-tidy] Added support for validating configuration options (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D77085?vs=255023&id=255764#toc Repository: rG L

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

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9dff9ecdd113: [clang-tidy] Change checks that take enum configurations to use a new access… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

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

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D77085#1967807 , @nemanjai wrote: > A recent commit has taken down a whole bunch of bots. The build error > messages all seem to point to code in this patch. If this is indeed the > cause, please revert. I was aware and hop

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

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added a subscriber: jdoerfert. njames93 added a comment. In D77085#1967883 , @nemanjai wrote: > Awesome, thanks. Certainly fixes the compile time failures in my local build. > There is still a link-time failure (undefined reference) with my share

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

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D77572#1967956 , @mgehre wrote: > In D77572#1965143 , @njames93 wrote: > > > I'm struggling to see the value of this check though. If it was reworked to > > check for loop in the middle

[PATCH] D77680: [clang-tidy] misc-unused-parameters: Don't remove parameter from lambda

2020-04-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Outright disabling for lambdas probably isn't the ideal solution, however it's at least a good stepping stone til a better solution can be found. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77680/new/ https://reviews.ll

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

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 256237. njames93 added a comment. Fixed up documentation for CastKind matcher. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77503/new/ https://reviews.llvm.org/D77503 Files: clang/docs/LibASTMatchersRefere

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

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: klimek, aaron.ballman. Herald added a reviewer: jdoerfert. Herald added a project: clang. Herald added a subscriber: cfe-commits. Adds support for using the ofKind in clang-query and other dynamic matcher use cases Repository: rG LLVM

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

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:102-106 + static constexpr llvm::StringRef Allowed[] = { + "UETT_SizeOf", "UETT_AlignOf", + "UETT_VecStep", "UETT_OpenMPRequiredSimdAlign", + "UETT_Preferr

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

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 256262. njames93 marked 4 inline comments as done. njames93 added a comment. Added test cases and address nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77791/new/ https://reviews.llvm.org/D77791 Files:

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

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 256275. njames93 added a comment. Added unit test for matcher Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77503/new/ https://reviews.llvm.org/D77503 Files: clang/docs/LibASTMatchersReference.html clang/

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

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:102-106 + static constexpr llvm::StringRef Allowed[] = { + "UETT_SizeOf", "UETT_AlignOf", + "UETT_VecStep", "UETT_Ope

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

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

[PATCH] D77831: [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

2020-04-09 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. This was done with a script that looks for calls to Options.get(GlobalOrLocal) that take an integer for

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

2020-04-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Right now it would be a great candidate for this - http://lists.llvm.org/pipermail/cfe-dev/2020-March/064980.html, however in its current state I wouldn't say its ready to get the green light right now. No point worrying about the fix-its yet, but when it is time, could

[PATCH] D77831: [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

2020-04-12 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 256842. njames93 added a comment. Added a few more cases by hand Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77831/new/ https://reviews.llvm.org/D77831 Files: clang-tools-extra/clang-tidy/bugprone/Argumen

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

2020-04-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D76496#1949851 , @niko wrote: > Correct me if I'm wrong, but that seems to be in violation of IWYU? Maybe I'm > misreading this, or is the idea that higher-lever tooling (e.g. IWYU fixer) > is supposed to address that? It i

[PATCH] D77831: [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

2020-04-12 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG672207c319a0: [clang-tidy] Convert config options that are bools to use the bool overload of… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

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

2020-04-13 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 257151. njames93 added a comment. Clean up code and remove unrelated changes 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/bugp

[PATCH] D78139: [clang-tidy] modernize-use-using: Fix broken fixit with 'template' keyword

2020-04-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/AST/NestedNameSpecifier.cpp:314 +InnerPolicy); +} else if (const auto *SpecType = + dyn_cast(T)) { Can this be renamed as it shadows the `SpecType` variabl

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

2020-04-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D75184#1984565 , @DmitryPolukhin wrote: > I didn't notice the issue because of massive breakage with diff landed just > be bore mine Sorry about that :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

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

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D77572#2071045 , @RKSimon wrote: > @mgehre Please can you take at the remaining buildbot failures here : > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/68662 Pushed a fix for that too.

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

2020-06-03 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:39 +"__builtin_classify_type", +// "__builtin_va_start", +// "__builtin_stdarg_start", aaron.

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

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 268256. njames93 added a comment. - Detect va_list type VarDecls to warn on Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80887/new/ https://reviews.llvm.org/D80887 Files: clang-tools-extra/clang-tidy/cppco

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

2020-06-04 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 268378. njames93 added a comment. Hopefully fixed windows buildn Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80887/new/ https://reviews.llvm.org/D80887 Files: clang-tools-extra/clang-tidy/cppcoreguideline

[PATCH] D80896: [clang-tidy][misc-redundant-expression] Support for CXXFoldExpr

2020-06-04 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-redundant-expression.cpp:31 +struct Bar2 { + static_assert((... && (sizeof(Values) > 0)) || (... && (sizeof(Values) > 0))); + // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: both sides of

[PATCH] D80490: [clang-tidy] Check for rule of five and zero.

2020-06-04 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. It would take more complex matching and diagnostics logic, but it would be so much nicer if the diagnostics were grouped. So instead of having a diagnostic about class `X` defining a copy construct and another about it defining a copy assignment operator, It could just

[PATCH] D81150: Use libClangTesting in the unittest for AST matchers

2020-06-04 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersTest.h:62 +inline ArrayRef langCxx11OrLater() { + static std::vector Result = {Lang_CXX11, Lang_CXX14, Lang_CXX17, + Lang_CXX20}; Bit

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

2020-06-04 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe21c3f223a35: [clang-tidy] ignore builtin varargs from pro-type-vararg-check (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80887/new/

[PATCH] D81180: AST Matchers test: use arrays instead of vectors

2020-06-04 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. cheers LGTM, probably didn't need a review this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81180/new/ https://reviews.llvm.org/D8118

[PATCH] D80301: [yaml][clang-tidy] Fix new line YAML serialization

2020-06-04 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: llvm/lib/Support/YAMLTraits.cpp:894 + std::string &Val) { + Val.clear(); + size_t CurrentPos = 0; If you want to do the same here... ``` SmallVector Lines; Scalar.split(Li

[PATCH] D80896: [clang-tidy][misc-redundant-expression] Support for CXXFoldExpr

2020-06-05 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. LGTM, nothing else to add. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80896/new/ https://reviews.llvm.org/D80896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

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

2020-06-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I decided to do some more stringent benchmarking, just focusing on the matching, not the boilerplate of running clang-tidy. =BeforePatch=== Matcher Timings: 116.0756 user 29.1397 system 145.2154 pr

[PATCH] D80896: [clang-tidy][misc-redundant-expression] Support for CXXFoldExpr

2020-06-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. How are you landing these changes, because the commit message isn't lining up with the PR name? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80896/new/ https://reviews.llvm.org/D80896 _

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

2020-06-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I feel the refactoring of Aliasing should be in its own PR, with this being a child of it. Comment at: clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp:38 + ifStmt(hasCondition(anyOf( + declRefExpr(hasDeclaration(varD

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

2020-06-06 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:777 + "or make sure they are both configured the same. " + "Aliased checkers: '{0}', '{1}'", + Exist

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

2020-06-06 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, lebedev.ri, gribozavr2. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Ignore template instantiations in the matchers, Addresses readability-simplify-boolean-expr false-positive for

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

2020-06-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/readability/SimplifyBooleanExprCheck.cpp:425 Finder->addMatcher( - ifStmt(isExpansionInMainFile(), hasCondition(cxxBoolLiteral(equals(Value))

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

2020-06-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/readability/SimplifyBooleanExprCheck.cpp:425 Finder->addMatcher( - ifStmt(isExpansionInMainFile(), hasCondition(cxxBoolLiteral(equals(Value))

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

2020-06-07 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 269076. njames93 added a comment. Added back isExpansionInMainFile check Should put a follow up to query removing this check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81336/new/ https://reviews.llvm.org/

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

2020-06-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. These test cases don't show it, from what i can see this will erroneously match: if (onFire) { tryPutFireOut(); } else { if (isHot && onFire) { scream(); } } It appears that this will change the second if to: if (isHot) { scream(); } R

[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present

2020-06-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D79912#2081402 , @Tridacnid wrote: > Awesome. I don't have commit access, https://llvm.org/docs/Phabricator.html > says I just need to ask here and someone will pick it up. Let me know if > there's anything else I need to do.

[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present

2020-06-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D79912#2082337 , @Tridacnid wrote: > tridac...@gmail.com > > https://github.com/Tridacnid > > Thanks! I've commited it on your behalf, w

[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present

2020-06-09 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGce5fecb7d0a1: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit… (authored by Tridacnid, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present

2020-06-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D79912#2083477 , @Tridacnid wrote: > Bug report has been closed. I'm seeing some build failures in my inbox but > eyeballing them doesn't make me think this change is related. What is the > expectation for me in this scenario

[PATCH] D81396: [clang-tidy] New util `Aliasing` factored out from `bugprone-infinite-loop`

2020-06-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/Aliasing.cpp:17 + +/// Return whether `S` is a reference to the declaration of `Var`. +static bool isAccessForVar(const Stmt *S, const VarDecl *Var) { Ditto `\p `. C

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

2020-06-10 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: klimek, aaron.ballman, jkorous. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. Adds a matcher called `hasDirectBase` for matching the `CXXBaseSpecifier` of a class that directly derives from another class

[PATCH] D81396: [clang-tidy] New util `Aliasing` factored out from `bugprone-infinite-loop`

2020-06-10 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. LGTM, but with one more nit Comment at: clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp:16 using namespace clang::ast_matchers; +using clang::tidy::utils::hasPtrOrReferenceInFunc; This fu

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

2020-06-10 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2888-2890 +/// class Foo; +/// class Bar : Foo {}; +/// class Baz : Bar {}; aaron.ballman wrote: > It seems like these aren'

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

2020-06-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 27. njames93 added a comment. - Added back `hasType` overload for `CXXBaseSpecifier` - Added `hasClassTemplate` and `hasClassOrClassTemplate` matcher for `CXXBaseSpecifier` - Added `hasTemplatedDecl` for `ClassTemplateDecl` Repository: rG LLVM Github

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

2020-06-10 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 2 inline comments as done. njames93 added a comment. In D81552#2086420 , @jkorous wrote: > @njames93 `hasDirectBase` seems like a useful matcher to me! OTOH I am not > totally convinced about `hasType` -> `hasClass`. Anyway, don't you want

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

2020-06-11 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3537 AST_POLYMORPHIC_MATCHER_P_OVERLOAD( -hasType, -AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl, -

[PATCH] D15528: Teach clang-tidy how to -Werror checks.

2020-06-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Herald added a subscriber: arphaman. Comment at: clang-tidy/tool/ClangTidyMain.cpp:362 + << Plural << "\n"; +return WErrorCount; + } Was there any specific reason for returning the error count instead of retur

[PATCH] D81785: [clangd] Fix readability-else-after-return 'Adding a note without main diagnostic' crash

2020-06-13 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: sammccall, hokein. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. njames93 added a comment. The actual fix in `ElseAfterReturnCheck.cpp` is needed. How

[PATCH] D81785: [clangd] Fix readability-else-after-return 'Adding a note without main diagnostic' crash

2020-06-13 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. The actual fix in `ElseAfterReturnCheck.cpp` is needed. However clangd's handling of Remarks is a little suspicious. Remarks are supposed to be different to notes in the sense they are designed to be stand alone, unlike notes which depend on a another diagnostic. see A

[PATCH] D81785: [clangd] Fix readability-else-after-return 'Adding a note without main diagnostic' crash

2020-06-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:191 // scope, we can pull the decl out of the if statement. - DiagnosticBuilder Diag = - diag(ElseLoc, WarningMessage, clang::DiagnosticIDs::Level::

[PATCH] D81785: [clangd] Fix readability-else-after-return 'Adding a note without main diagnostic' crash

2020-06-15 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 270759. njames93 marked an inline comment as done. njames93 added a comment. - Remove clangd test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81785/new/ https://reviews.llvm.org/D81785 Files: clang-t

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

2020-06-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. As an afterthought, Would you think it a good idea to extend this logic to the `ConfigOptionsProvider` That way you can use something along the lines of: clang-tidy --config='{InheritParentConfig: true, CheckOptions: []}' clang-tidy would then go to read the .clang-t

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

2020-06-16 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, gribozavr2. Herald added subscribers: cfe-commits, mgrang, xazax.hun, mgorny. Herald added a project: clang. Flags and replaces calls to standard library algorithms that could be converted to use the `std::ranges` al

[PATCH] D81785: [clangd] Fix readability-else-after-return 'Adding a note without main diagnostic' crash

2020-06-16 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG740575dc232b: [clangd] Fix readability-else-after-return 'Adding a note without main… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

2020-06-16 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 271047. njames93 added a comment. Removed `isExpansionInMainFile` check as per original authors comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81336/new/ https://reviews.llvm.org/D81336 Files: clang-

[PATCH] D81932: [clang-tidy] Improved accuracy of check list updater script

2020-06-16 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2, Eugene.Zelenko. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. - Added a file `FixItHint` comments to Check files to for the script to mark those checks as offering fix-its when

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

2020-06-16 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/modernize/UseRangesCheck.cpp:77 + MatchCallTo((ID + "Begin").str(), namedDecl().bind(Range), + hasAnyName("::std::begi

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

2020-06-16 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe1ba7241c3ef: [clang-tidy] simplify-bool-expr ignores template instantiations (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81336/new

[PATCH] D81949: [clang-tidy] Extend InheritParentConfig to CommandLineConfig

2020-06-16 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: DmitryPolukhin, alexfh, gribozavr2, klimek, hokein, aaron.ballman. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Extend the `InheritParentConfig` support introduced in D75184

[PATCH] D81953: [clang-tidy] warnings-as-error no longer exits with ErrorCount

2020-06-16 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: alexfh, jroelofs, aaron.ballman. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. When using `-warnings-as-errors`, If there are any warnings promoted to errors, clang-tidy exits with the number of warnings.

[PATCH] D81953: [clang-tidy] warnings-as-error no longer exits with ErrorCount

2020-06-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D81953#2096388 , @aaron.ballman wrote: > LGTM unless @jroelofs has a reason why the code was originally written that > way, but can you add test coverage for it? How would you suggest I add test coverage for this, afaik llv

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

2020-06-16 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 271191. njames93 marked 4 inline comments as done. njames93 added a comment. Tweaked documentation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81923/new/ https://reviews.llvm.org/D81923 Files: clang-tools

[PATCH] D81975: [clangd] Add command line option for ClangTidyConfig

2020-06-16 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. Adds a command line flag `clang-tidy-config` for specifing configuration of checks, i

[PATCH] D82004: [clang-tidy][NFC] Remove the double look-up on IncludeInserter

2020-06-17 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: klimek, gribozavr2, aaron.ballman, alexfh. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. Refactor out the double lookup in `IncludeInserter` when trying to get the `IncludeSorter` for a specified `FileID`

[PATCH] D81975: [clangd] Add command line option for ClangTidyConfig

2020-06-17 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Gotta say it's not hugely pressing. The reason for it is clangd lets you specify some checks to run but it doesn't let you specify the options for those checks. Effectively forcing each project to require a .clang-tidy configuration file if you want to use checks where

[PATCH] D81953: [clang-tidy] warnings-as-error no longer exits with ErrorCount

2020-06-17 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGccd127008aa2: [clang-tidy] warnings-as-error no longer exits with ErrorCount (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81953/new/

[PATCH] D82004: [clang-tidy][NFC] Remove the double look-up on IncludeInserter

2020-06-17 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG08c83ed75752: [clang-tidy][NFC] Remove the double look-up on IncludeInserter (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D82004?vs=271334&id=271432#toc Repository: rG LL

[PATCH] D82059: [clang-tidy] RenamerClangTidy group redecls into 1 warning.

2020-06-17 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, jbcoe, gribozavr2. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. This changes the behavious of `RenamerClangTidyCheck` based checks by grouping declarations of the same thing into 1

[PATCH] D82059: [clang-tidy] RenamerClangTidy group redecls into 1 warning.

2020-06-17 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 271509. njames93 added a comment. New line... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82059/new/ https://reviews.llvm.org/D82059 Files: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp c

[PATCH] D82002: [clangd] Drop FS usage in ClangTidyOpts

2020-06-18 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:495-498 +bool HasChecks = false; +for (const auto &Source : Sources) + HasChecks |= Source.first.Checks.hasValue(); +if (!HasChecks) `if (llvm::none_of(Sourc

[PATCH] D82002: [clangd] Drop FS usage in ClangTidyOpts

2020-06-18 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I'm of the idea that rather than having `ClangdTidyOptionsProvider` inherit form `tidy::ClangTidyOptionsProvider`, just have it as its own class. We don't need the interface offered by clang tidy here. It would solve the `must be threadsafe` comment issue as well as re

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

2020-07-01 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG669494e9c06c: [clang-tidy] fix cppcoreguidelines-init-variables with catch variables (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82

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

2020-07-01 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:125 +CheckFactories.registerCheck( +"bugprone-no-escape"); CheckFactories.registerCheck( ellis wrote: > Not sure if we want to lint this line

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

2020-07-01 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Just my 2 cents, but would it not be wise to introduce a test case that runs the 2 aforementioned checks together demonstrating how they interact with each other. This will also force compliance if either check is updated down the line. CHANGES SINCE LAST ACTION htt

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

2020-07-01 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 274846. njames93 added a comment. Small refactor based on comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82706/new/ https://reviews.llvm.org/D82706 Files: clang/docs/LibASTMatchersReference.html c

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

2020-07-01 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 5 inline comments as done. njames93 added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:124 +llvm::Optional getRegexFlag(llvm::StringRef Flag) { + for (auto &StringFlag : RegexMap) { +if (Flag == StringFlag.first)

[PATCH] D82807: [clang-tidy] Allows the prevailing include header guard in Flang ...

2020-07-01 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Would you be able to add some unit tests for this in a follow up. `clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp` is where they live. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82807/new/ https://reviews.llv

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