[PATCH] D117090: [clang-tidy] Fix `readability-non-const-parameter` for parameter referenced by an lvalue

2022-02-24 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Before landing make sure this is rebased against trunk, just so the pre-merge bot can run and verify there are no issues. It looks like this patch is based off a few commits that don't appear on trunk. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117090/new/

[PATCH] D120555: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments without name-like identifier

2022-02-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D120555#3345664 , @whisperity wrote: > Thanks! I'll re-run the tests on top of **14.0** and do the backport too, > soon. 🙂 If you backport, the release notes change on trunk should then be reverted. Repository: rG LLVM

[PATCH] D120814: [clang-tidy] Add check to perfer pre-increment over post-increment

2022-03-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. See D72553 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120814/new/ https://reviews.llvm.org/D120814 ___ cfe-commits mailing list cfe-commits

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

2022-03-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 412772. njames93 added a comment. Herald added a project: All. Moved tests into ASTMatchersInternal, makes sense in here and removes the dirty clang-tidy plugin hack. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

2022-03-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 412793. njames93 added a comment. Update test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120185/new/ https://reviews.llvm.org/D120185 Files: clang-tools-extra/docs/ReleaseNotes.rst clang/lib/ASTMa

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

2022-03-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:789 + else +OS << " "; + D->getSourceRange().print(OS, aaron.ballman wrote: > Should this be `" : "` instead? Good catch Comment

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

2022-03-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:796 +MV.ActiveASTContext->getSourceManager()); +} else if (const auto *T = Item.second.get()) { + OS << T->getTypeClassName() << "Type : "; --

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

2022-03-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. How does this handle cases with multiple assignments in one statement, and can tests be added to demonstrate the behaviour. Foo += Bar <<= 5; Int Baz = 5; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119599/new/ htt

[PATCH] D101054: [AST] Sort introspection results without instantiating other data

2021-04-23 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. This I suppose is a good fast way to sort these. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101054/new/ https://reviews.llvm.org/D101054

[PATCH] D101049: [AST] Add DeclarationNameInfo to node introspection

2021-04-23 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/include/clang/Tooling/NodeIntrospection.h:29 class CXXBaseSpecifier; +class DeclarationNameInfo; Please fix this lint warning. Comment at: clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py:

[PATCH] D93325: Add srcloc output to clang-query

2021-04-23 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-query/Query.cpp:106-108 + if (Scout->first == CommonEntry) { +++Iter; + } nit: Elide braces. Comment at: clang-tools-extra/clang-query/Query.cpp:122 + +Te

[PATCH] D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available.

2021-04-27 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: steveire, aaron.ballman. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Checks if introspection support is available set output kind parser. If it isn't present

[PATCH] D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available.

2021-04-27 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 340835. njames93 added a comment. Cleanup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101365/new/ https://reviews.llvm.org/D101365 Files: clang-tools-extra/clang-query/QueryParser.cpp Index: clang-tool

[PATCH] D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available.

2021-04-27 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 340902. njames93 added a comment. Fix up tests(probably, see what premerge says) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101365/new/ https://reviews.llvm.org/D101365 Files: clang-tools-extra/clang-que

[PATCH] D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available.

2021-04-27 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 341021. njames93 added a comment. Nit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101365/new/ https://reviews.llvm.org/D101365 Files: clang-tools-extra/clang-query/Query.h clang-tools-extra/clang-query

[PATCH] D101275: [clangd] Hide inlay hints capability behind a command-line flag

2021-04-27 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:312 + desc("Enable preview of InlayHints feature"), init(false), + Hidden}; + Does it make sense to hide this flag? This is a feature

[PATCH] D101259: [clang-tidy] Fix cppcoreguidelines-pro-type-vararg false positives with __builtin_ms_va_list

2021-04-27 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:84 + // In these cases we need to remove all typedefs one by one to check this. + const auto VaListKind = Context.getTargetInfo().getBuiltinVaListKind(); + using

[PATCH] D101365: [clang-query] Add check to prevent setting srcloc when no introspection is available.

2021-04-28 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG858a9583e1fe: [clang-query] Add check to prevent setting srcloc when no introspection is… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D97955: [clang-tidy] Refactor loop-convert to bring most of the checking into matchers

2021-04-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Herald added a project: clang-tools-extra. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97955/new/ https://reviews.llvm.org/D97955 ___ cfe-commits mailing list cfe-commit

[PATCH] D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy

2021-04-28 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 341167. njames93 added a comment. Rebase and fix up new checks added and changes to tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97121/new/ https://reviews.llvm.org/D97121 Files: clang-tools-extra/c

[PATCH] D101461: [clangd][NFC] Reserve storage when creating semantic token encoding.

2021-04-28 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: sammccall, kadircet. Herald added subscribers: usaxena95, arphaman. njames93 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Repository: rG LLVM

[PATCH] D101461: [clangd][NFC] Reserve storage when creating semantic token encoding.

2021-04-28 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc3846bcfe1cc: [clangd][NFC] Reserve storage when creating semantic token encoding. (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1014

[PATCH] D97955: [clang-tidy] Refactor loop-convert to bring most of the checking into matchers

2021-04-28 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 341350. njames93 added a comment. Remove unnecessary test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97955/new/ https://reviews.llvm.org/D97955 Files: clang-tools-extra/clang-tidy/modernize/LoopCon

[PATCH] D101611: [clangd][NFC] Remove unnecessary string captures in lambdas.

2021-04-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman. njames93 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Due to a somewhat ann

[PATCH] D101614: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers

2021-04-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: steveire, aaron.ballman. Herald added a subscriber: xazax.hun. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. There should be a follow up to this for changing t

[PATCH] D101617: [clang-tidy] Tweak diag ranges for bugprone-sizeof-expression

2021-04-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: alexfh, aaron.ballman. Herald added a subscriber: xazax.hun. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Provide more useful warning locations and diagnostic

[PATCH] D101611: [clangd][NFC] Remove unnecessary string captures in lambdas.

2021-04-30 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG681503708594: [clangd][NFC] Remove unnecessary string captures in lambdas. (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101611/new/

[PATCH] D101624: [clang-tidy] Make performance-inefficient-vector-operation work on members

2021-04-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, gribozavr2. Herald added a subscriber: xazax.hun. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Fixes https://llvm.org/PR50157 Adds sup

[PATCH] D101617: [clang-tidy] Tweak diag ranges for bugprone-sizeof-expression

2021-04-30 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 341883. njames93 added a comment. Herald added subscribers: usaxena95, kadircet, arphaman. Fix clangd test cases failing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101617/new/ https://reviews.llvm.org/D101

[PATCH] D101628: [Format] Don't sort includes if DisableFormat is true

2021-04-30 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added a reviewer: MyDeveloperDay. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://llvm.org/PR35099. I'm not sure if this decision was intentional but its definitely confusing

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2021-05-01 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Sorry, I thought this landed months ago. I'll take a proper look again next week. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:158 +/// name \p LocalName shoud not present. +void diagnoseInvalidConfigOption(StringRef LocalName) co

[PATCH] D101614: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers

2021-05-01 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 342130. njames93 added a comment. Update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101614/new/ https://reviews.llvm.org/D101614 Files: clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

2021-05-02 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:124-126 DiagnosticBuilder configurationDiag(StringRef Description, DiagnosticIDs::Level Level = DiagnosticIDs::Warning); You don't need a ClangTid

[PATCH] D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule

2021-05-02 Thread Nathan James via Phabricator via cfe-commits
njames93 added reviewers: aaron.ballman, njames93. njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp:4 +// RUN: -config="{CheckOptions: \ +// RUN: [{key: cppcoreguidelines-pro-type-membe

[PATCH] D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value

2021-05-02 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added a reviewer: aaron.ballman. Herald added subscribers: kbarton, xazax.hun, nemanjai. njames93 requested review of this revision. Herald added subscribers: cfe-commits, aheejin. Herald added a project: clang-tools-extra. Change instances where options wh

[PATCH] D99646: [clang-tidy] misc-avoid-std-io-outside-main: a new check

2021-05-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/AvoidStdIoOutsideMainCheck.cpp:22 + Finder->addMatcher( + declRefExpr(to(varDecl(hasAnyName("cin", "wcin", "cout", "wcout", "cerr", +"wcerr"), -

[PATCH] D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value

2021-05-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D101721#2733125 , @aaron.ballman wrote: > LGTM! One thing I'd like to be sure of though -- do we still have at least > one test that's showing you can use false/0 and true/1/nonzero > interchangeably? If not, we should prob

[PATCH] D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value

2021-05-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D101721#2733173 , @aaron.ballman wrote: > In D101721#2733169 , @njames93 > wrote: > >> However in a few years once we can be confident most users are using >> clang-tidy-11 or newer

[PATCH] D99543: [clang-tidy] Allow opt-in or out of some commonly occuring patterns in NarrowingConversionsCheck.

2021-05-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-ignoreconversionfromtypes-option.cpp:12 + +typedef long long size_t; + This is breaking tests on windows, It seems like its pre-defined

[PATCH] D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value

2021-05-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D101721#2733279 , @aaron.ballman wrote: > That makes sense to me. Should we file a bug to suggest adding the > deprecation warning in Clang 14(?) and planned removal in Clang 16(?) so that > we don't lose track of this? (I

[PATCH] D101628: [Format] Don't sort includes if DisableFormat is true

2021-05-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D101628#2734501 , @curdeius wrote: > Is it testable? I guess it would be, gimme a few to have a look at format unittests and I'll rustle up a quick test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D96215: [clang-tidy] Aliasing: Add support for lambda captures.

2021-05-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D96215#2550227 , @NoQ wrote: > Umm, looks like we're both missing the elephant in the room: passing a > variable into a function by reference. > > int &hidden_reference(int &x) { > return x; > } > > void test_hidd

[PATCH] D101628: [Format] Don't sort includes if DisableFormat is true

2021-05-04 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 342778. njames93 added a comment. Add test case demonstrating new behvaiour. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101628/new/ https://reviews.llvm.org/D101628 Files: clang/lib/Format/Format.cpp c

[PATCH] D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value

2021-05-04 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe1c729c56829: [clang-tidy][NFC] Update tests and Default options to use boolean value (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D101628: [Format] Don't sort includes if DisableFormat is true

2021-05-04 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG61dc0f2b593d: [Format] Don't sort includes if DisableFormat is true (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101628/new/ https:

[PATCH] D101259: [clang-tidy] Fix cppcoreguidelines-pro-type-vararg false positives with __builtin_ms_va_list

2021-05-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp:7 + +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t + TWeaver wrote: > Is the missing FileCheck call here on purpos

[PATCH] D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule

2021-05-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Whats the expected behaviour of sugar types. Can tests be added that demonstrate the behaviour. I'd argue these cases shouldn't be warned on if `IgnoreArrays` is enabled and If that behaviour isn't observed currently it should be addressed. typedef int TypedefArray[4

[PATCH] D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule

2021-05-09 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM, thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101239/new/ https://reviews.llvm.org/D101239 ___ cfe-commits mailing list

[PATCH] D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule

2021-05-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D101239#2746675 , @h-joo wrote: > In D101239#2746644 , @njames93 > wrote: > >> LGTM, thanks. > > @njames93, I appreciate your time for the review. May I ask one more thing? > I do n

[PATCH] D115425: [clangd] Generate ConfigFragment/YAML/docs from one tablegen source

2022-01-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. For the website, we use tags to specify the clangd version that the option was first supported in. I'd suggest that we also add a `Version` field to the `Field` class. This could also cause problem down the line if we ever wanted to remove a config option. Repository:

[PATCH] D115249: Clang-Tidy implicit bool conversion check use upercase suffixes

2022-01-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Something stylistic like this should likely be configurable. Lowercase literals are a part of the language and there is likely a codebase out there that explicitly prefers them Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

2022-01-08 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: alexfh, aaron.ballman. Herald added subscribers: carlosgalvezp, kristof.beyls, xazax.hun, mgorny. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Adds a check th

[PATCH] D116824: [clang-tidy] Fix RenamerClangTidyChecks suggesting invalid macro identifiers

2022-01-08 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM, Thanks. Don't know how I missed this one last time round. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116824/new/ https://reviews.ll

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

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

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

2022-01-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.cpp:63 +hasLHS(ToDecl), +hasRHS(integerLiteral(equals(1)), +

[PATCH] D114823: Filter string_view from the nullptr diagnosis of bugprone-string-constructor to prevent duplicate warnings with bugprone-stringview-nullptr

2022-01-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I'm in 2 minds about this. This diagnostic is a good fit for this warning and shouldn't be removed. Likewise duplicate diagnostics from different checks is annoying. Maybe a better path forward would be to suppress the diagnostic if the `bugprone-stringview-nullptr` ch

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

2022-01-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Shouldn't the constructors that need `std::move` also ensure the `` header is also included, or at least transitively included. We don't want to generate constructors that wont compile. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D117129: [clang-tidy] Extract Class IncluderClangTidyCheck

2022-01-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D117129#3238441 , @LegalizeAdulthood wrote: > Another option would be to migrate this to `ClangTidyCheck` itself > and add a flag/option passed to the ClangTidyCheck c'tor to opt-in > to the feature? I feel that this directi

[PATCH] D117185: [clangd] Ignore cvr-qualifiers in selection.

2022-01-13 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:194 + return Tok.kind() == tok::comment || Tok.kind() == tok::semi || + Tok.kind() == tok::kw_const || Tok.kind() == tok::kw_volatile || + Tok.kind() == tok::kw_restrict; -

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 400266. njames93 added a comment. Add support for array access `Arr[I * JExtent + J]` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116875/new/ https://reviews.llvm.org/D116875 Files: clang-tools-extra/clan

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. How does this check play with the `modernize-make-shared` check? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117306/new/ https://reviews.llvm.org/D117306 ___ cfe-commits maili

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 400267. njames93 added a comment. Herald added a project: clang-tools-extra. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95046/new/ https://reviews.llvm.org/D95046 Files: clang-tools-extra/clangd/C

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. @sammccall What else is needed for this to go in? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95046/new/ https://reviews.llvm.org/D95046 ___ cfe-commits mailing list cfe-commi

[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Herald added a subscriber: carlosgalvezp. Sorry its been so long, How does this handle cases where the variable contains resources. Like a vector being used in a loop where you don't want to release its memory between each iteration. void foo(){ std::vector Buffe

[PATCH] D117205: [clang-tidy] Support custom fix hint for cppcoreguidelines-pro-bounds-constant-array-index

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:26 +utils::IncludeSorter::IS_LLVM)), + FixHint(Options.get("FixHint", "gsl::at()")) {}

[PATCH] D117129: [clang-tidy] Extract Class IncluderClangTidyCheck

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D117129#3239514 , @LegalizeAdulthood wrote: > In D117129#3239143 , @njames93 > wrote: > >> My proposed idea was putting the `IncludeInserter` instance >> inside the `ClangTidyContext

[PATCH] D117129: [clang-tidy] Extract Class IncluderClangTidyCheck

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D117129#3246610 , @LegalizeAdulthood wrote: > In D117129#3246403 , @njames93 > wrote: > >> > > I pushed the functionality down into ClangTidyCheck > instead of introducing an interm

[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

2022-01-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:203 + return CopyRef; +return Fail; + } If C is default constructible, would it be nice to skip here instead of failing? C

[PATCH] D117460: [clang-tidy][NFC] Reduce map lookups in IncludeSorter

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

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

2022-01-17 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D95046#3247793 , @sammccall wrote: > Sorry about letting this sit. I think this is good to go, with a naming tweak. > > For the 14 cycle, it should be hidden: we may want to tweak the interactions > with `didSave` reparsing et

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

2022-01-17 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 400473. njames93 added a comment. Update name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95046/new/ https://reviews.llvm.org/D95046 Files: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/cl

[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

2022-01-17 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D117306#3247417 , @LegalizeAdulthood wrote: > In D117306#3245915 , @njames93 > wrote: > >> How does this check play with the `modernize-make-shared` check? > > Wouldn't it be orthogo

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

2022-01-17 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 400479. njames93 added a comment. Nit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95046/new/ https://reviews.llvm.org/D95046 Files: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/clangd/Cla

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

2022-01-17 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 rG8b88ff08038c: [clangd] Add option to use dirty file contents when building preambles. (authored by njames93). Repository: rG LLVM Github Monorepo

[PATCH] D117409: [clang-tidy] Move IncludeInserter into ClangTidyContext

2022-01-17 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. Herald added subscribers: carlosgalvezp, usaxena95, kadircet, arphaman, kbarton, xazax.hun, mgorny, nemanjai. Eugene.Zelenko added reviewers: aaron.ballman, LegalizeAdulthood. Eugene.Zelenko added a project: clang-tools-extra. njames93 updated this revision to Diff

[PATCH] D117460: [clang-tidy][NFC] Reduce map lookups in IncludeSorter

2022-01-17 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D117460#3248613 , @alexfh wrote: > I wonder what motivated the patch. Is this a performance optimization? If so, > do you have any measurements? I was doing some work on IncludeInserter and just thought, this seems unnecess

[PATCH] D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy

2021-12-26 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 396219. njames93 added a comment. Herald added a subscriber: carlosgalvezp. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97121/new/ https://reviews.llvm.org/D97121 Files: clang-tools-extra/clang-tid

[PATCH] D97653: [clang-tidy] Fix RenamerClangTidy checks breaking lambda captures.

2021-03-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Herald added a project: clang-tools-extra. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97653/new/ https://reviews.llvm.org/D97653 ___ cfe-commits mailing list cfe-commit

[PATCH] D97121: [clang-tidy] Add a Standalone diagnostics mode to clang-tidy

2021-03-16 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 331023. njames93 added a comment. Turns out we don't need to worry about InsertedHeaders at all in IncludeInserter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97121/new/ https://reviews.llvm.org/D97121 Fi

[PATCH] D98692: [clang-tidy] Add cppcoreguidelines-comparison-operator

2021-03-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp:20-21 +void ComparisonOperatorCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(functionDecl(hasAnyOverloadedOperatorName("==", "!=", "<", +

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

2021-03-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Again LGTM, but see what alex says. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:135-137 + for (const FileByteRange &FBR : Error.Message.Ranges) { +Diag << getRange(FBR); + } nit: Elide braces. ===

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

2021-03-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D98635#2629827 , @whisperity wrote: > @aaron.ballman Could you please help me understand this pre-merge buildbot? > It says that Debian failed, with the following message: > > [15/1038] Running lint check for sanitizer sourc

[PATCH] D98745: [clang] Add fixit for Wreorder-ctor

2021-03-16 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: rsmith, sammccall, aaron.ballman. Herald added a subscriber: mgrang. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Create fix-it hints to fix the order of constructors. To

[PATCH] D98692: [clang-tidy] Add cppcoreguidelines-comparison-operator

2021-03-17 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. While we're here, I'm not sure if there is a cppcoreguideline for it, but parameters to comparison functions should be passed by const ref usually, or by value if they are cheap to copy. Maybe there's precedent to extend this to flag parameters that take non const refe

[PATCH] D98692: [clang-tidy] Add cppcoreguidelines-comparison-operator

2021-03-17 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D98692#2631504 , @nullptr.cpp wrote: > The relevant rule is F.16: For "in" parameters, pass cheaply-copied types by > value and others by reference to const >

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

2021-03-17 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: klimek, aaron.ballman, steveire. Herald added a subscriber: pengfei. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Changing matchers to use non-const members and adding r-

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

2021-03-17 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 331340. njames93 added a comment. Fix PP guards erroneously using #ifdef Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98792/new/ https://reviews.llvm.org/D98792 Files: clang/include/clang/ASTMatchers/ASTMa

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

2021-03-17 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1564 public: - PolymorphicMatcher(const ParamTypes &... Params) : Params(Params...) {} + PolymorphicMatcher(const ParamTypes &...Params) : Params(Params...) {}

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

2021-03-17 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 331353. njames93 added a comment. Fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98792/new/ https://reviews.llvm.org/D98792 Files: clang/include/clang/ASTMatchers/ASTMatchersInternal.h clang

[PATCH] D98792: [ASTMatchers][NFC] Use move semantics when passing matchers around.

2021-03-17 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG48ab9674b21b: [ASTMatchers][NFC] Use move semantics when passing matchers around. (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98792

[PATCH] D98745: [clang] Add fixit for Wreorder-ctor

2021-03-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 331581. njames93 added a comment. Fix formatting issues Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98745/new/ https://reviews.llvm.org/D98745 Files: clang/test/SemaCXX/constructor-initializer.cpp clang

[PATCH] D98745: [clang] Add fixit for Wreorder-ctor

2021-03-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 331582. njames93 added a comment. Arc got confused Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98745/new/ https://reviews.llvm.org/D98745 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/li

[PATCH] D98745: [clang] Add fixit for Wreorder-ctor

2021-03-18 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 4 inline comments as done. njames93 added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5242 + if (Previous->isAnyMemberInitializer()) +Diag << 0 << Previous->getAnyMember()->getDeclName(); + else aaron.ballman wrote: > This w

[PATCH] D98745: [clang] Add fixit for Wreorder-ctor

2021-03-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 331635. njames93 marked an inline comment as done. njames93 added a comment. Address nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98745/new/ https://reviews.llvm.org/D98745 Files: clang/include/clang

[PATCH] D98827: [AST] Ensure that an empty json file is generated if compile errors

2021-03-18 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Are there any tests that ensure something is always outputted? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98827/new/ https://reviews.llvm.org/D98827 ___ cfe-commits mailing l

[PATCH] D98338: [clang-tidy] Fix bugprone-terminating-continue when continue appears inside a switch

2021-03-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 331713. njames93 marked an inline comment as done. njames93 added a comment. Add test case for do inside switch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98338/new/ https://reviews.llvm.org/D98338 Files:

[PATCH] D98338: [clang-tidy] Fix bugprone-terminating-continue when continue appears inside a switch

2021-03-20 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4dd92d61dbc4: [clang-tidy] Fix bugprone-terminating-continue when continue appears inside a… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

2021-03-21 Thread Nathan James via Phabricator via cfe-commits
njames93 requested changes to this revision. njames93 added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3812-3821 AST_POLYMORPHIC_MATCHER_P_OVERLOAD( hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(

[PATCH] D99106: [ASTMatchers][NFC] Use SmallVector when building variadic matcher descriptor

2021-03-22 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, klimek, steveire. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Saves having to manually deallocate storage and keeps InnerArgs will have good cache locali

[PATCH] D98275: [clang-tidy] Fix mpi checks when running multiple TUs per clang-tidy process

2021-03-22 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Herald added a project: clang-tools-extra. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98275/new/ https://reviews.llvm.org/D98275 ___ cfe-commits mailing list cfe-commit

<    15   16   17   18   19   20   21   >