[PATCH] D91037: [clang-tidy][bugprone-use-after-mnove] Warn on std::move for consts

njames93 added a comment. Think this has an incorrect name, seems to have something to do with `bugprone-redundant-branch-condition` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91037/new/ https://reviews.llvm.org/D91037

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 303774. njames93 added a comment. - Fix compile error for gcc 5.3. - Add back logging configuration when creating CTContext. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D9

[PATCH] D90555: [clangd] Handle duplicate enum constants in PopulateSwitch tweak

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5918ef8b1aac: [clangd] Handle duplicate enum constants in PopulateSwitch tweak (authored by njames93). Changed prior to commit: https://reviews.ll

[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

njames93 created this revision. njames93 added reviewers: rsmith, aaron.ballman, klimek, alexfh. Herald added subscribers: lldb-commits, cfe-commits, dexonsmith. Herald added projects: clang, LLDB. njames93 requested review of this revision. Herald added a subscriber: JDevlieghere. Implementing a

[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

njames93 added a comment. In D91103#2384048 , @jingham wrote: > IIUC, the expression parser part of this change suppresses any Fixits that > are clang-tidy type rewrites, is that right? If so that is indeed the > correct behavior. But the fact that thi

[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

njames93 updated this revision to Diff 304126. njames93 added a comment. Add `isReformatFixit` method to `FixItHint` to better express intent. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91103/new/ https://reviews.llvm.org/D91103 Files: clang-

[PATCH] D91149: [clang-tidy] Fix formatting of else after return check

njames93 created this revision. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Simplifies the code in the fix-it generation. Addresses https://bugs.llvm.org/show_bug.cgi?id=47809 (hopefully) By not changing the whole els

[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

njames93 added a comment. An example of this in action is D91149 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91103/new/ https://reviews.llvm.org/D91103 ___ cfe-commits

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

njames93 updated this revision to Diff 304145. njames93 marked 2 inline comments as done. njames93 added a comment. Made `-fix-notes` imply `-fix` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84924/new/ https://reviews.llvm.org/D84924 Files: cl

[PATCH] D91033: [clang-tidy][NFC] Tweak GlobList to iterate backwards

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/GlobList.cpp:56 bool GlobList::contains(StringRef S) { - bool Contains = false; - for (const GlobListItem &Item : Items) { + for (const GlobListItem &Item : llvm::reverse(Items)) { if (Item.Regex.ma

[PATCH] D91033: [clang-tidy][NFC] Tweak GlobList to iterate backwards

This revision was automatically updated to reflect the committed changes. Closed by commit rGe076fee63dd3: [clang-tidy][NFC] Tweak GlobList to iterate backwards (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D91033?vs=303725&id=304166#toc Repository: rG LLVM Github

[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

njames93 updated this revision to Diff 304198. njames93 added a comment. Herald added subscribers: usaxena95, kadircet, arphaman. Teach clangd to ignore these fix-its. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91103/new/ https://reviews.llvm.or

[PATCH] D91184: [clang-tidy] Merge options inplace instead of copying

njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2, alexfh. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Changed `ClangTidyOptions::mergeWith` to operate on the instance instead of ret

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

njames93 added a comment. Taking a step back, rather than worrying about if its an `ExprWithCleanups`. Shouldn't we just get the condition removing all implicit nodes. const Expr* Cond = InnerIf->getCond()->ignoreImplicit(); This has to be simpler and will likely future proof this against any

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES: {{^\ *

[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

njames93 added inline comments. Comment at: clang-tools-extra/clangd/Diagnostics.cpp:636 +// Filter out any reformat fixits, we don't handle these. +// FIXME: Can we? +llvm::erase_if(FixIts, kadircet wrote: > in theory yes, as we have access to source

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:45 ifStmt( hasCondition(ignoringParenImpCasts(anyOf( declRefExpr(hasDeclaration(ImmutableVar)), Just noticed, as

[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

njames93 updated this revision to Diff 304477. njames93 marked 2 inline comments as done. njames93 added a comment. Removed the first loop for clangd diagnostic, turns out it didnt make the following code that much messier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

njames93 updated this revision to Diff 304574. njames93 marked an inline comment as done. njames93 added a comment. Address nit by replacing optional. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91103/new/ https://reviews.llvm.org/D91103 Files:

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 304780. njames93 marked 4 inline comments as done. njames93 added a comment. Reworked large chunks of this: - Renamed ClangdTidyProvider->TidyProvider. - Removed the obselete interface mirroring ClangTidyOptionsProvider, it wasn't needed. - Incorporated the

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 added a comment. Thanks for the comments, I agree it was a little too much. Likely due in part to how I first tried to mirror the interface of ClangTidyOptionsProvider. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:250 if (Preamble && Preamble->StatCache) -

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

njames93 added a reviewer: aaron.ballman. njames93 added a comment. Should this be a NamingStyle option instead. `{key: readability-identifier-naming.ParameterShortSizeThreshold, value: 2}` WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90282/n

[PATCH] D91341: [clangd][NFC] Don't store symbolmap on the heap

njames93 created this revision. njames93 added a reviewer: hokein. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman. Herald added a project: clang. njames93 requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Storing this on the heap doesn't mak

[PATCH] D91351: [tooling] Implement determinsitic ordering of CompilationDatabasePlugins

njames93 created this revision. njames93 added reviewers: klimek, nridge. Herald added subscribers: cfe-commits, usaxena95, kadircet, mgrang. Herald added a project: clang. njames93 requested review of this revision. Herald added a subscriber: ilya-biryukov. Right now plugins appear to be registed

[PATCH] D91184: [clang-tidy] Merge options inplace instead of copying

This revision was automatically updated to reflect the committed changes. Closed by commit rG06db8f984f1a: [clang-tidy] Merge options inplace instead of copying (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D91184?vs=304237&id=304892#toc Repository: rG LLVM Github

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 304940. njames93 added a comment. - Small tweaks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/c

[PATCH] D91351: [tooling] Implement determinsitic ordering of CompilationDatabasePlugins

njames93 added inline comments. Comment at: clang/lib/Tooling/CompilationDatabase.cpp:73 CompilationDatabasePluginRegistry::entries()) { +Plugins.emplace_back(Database.getName(), Database.instantiate()); + } nridge wrote: > A side effect of this chan

[PATCH] D84293: Add an assertion in SmallVector::push_back()

njames93 added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:10811-10813 +Ops.reserve(Ops.size() + 1); Ops.push_back(Ops[0]); Ops.erase(Ops.begin()); Probably a little off topic, but shouldn't this be refactored as a rotate. Has the

[PATCH] D91351: [tooling] Implement determinsitic ordering of CompilationDatabasePlugins

njames93 added a comment. I'm hesitant to land this right away, want to see what happens with D91454 and its related mailing list . In my mind thats a much nicer way to fix this issue. Reposi

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Consider this code: if (Cond) { #ifdef X_SUPPORTED X(); #else return; #endi

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

njames93 updated this revision to Diff 305373. njames93 added a comment. Fix assertions failing with included files. Ran this over the entire clang directory, which has a lot of macro (ab)use, and no crashes or otherwise unexpected behaviour. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

njames93 added a comment. Just a general drive by point, is it wise to add a new tidy module to clang-tidy called `threads` (or similar). We have got a few other checks related to multi-threaded code in the pipeline (D77493 , D75229

[PATCH] D91602: [clang-tidy] Make clang-format and include-order-check coherent

njames93 added a comment. Thanks for fixing this, been bugging me for a while. Comment at: clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp:1 // RUN: %check_clang_tidy %s llvm-include-order %t -- -- -isystem %S/Inputs/Headers Would it be wi

[PATCH] D90944: [clang-tidy] implement misc-mt-unsafe

njames93 added a comment. Also can this be ran through clang-tidy, feeling a few naming violations are in here. If you use arc to upload your patches you'll get lint warnings about clang-tidy and clang-format. Comment at: clang-tools-extra/clang-tidy/misc/MtUnsafeCheck.cpp:27

[PATCH] D90944: [clang-tidy] implement concurrent-mt-unsafe

njames93 added a comment. In D90944#2399759 , @segoon wrote: > - move plugin to `concurrent` group While this is definitely a step in the right direction, it should be a separate patch to introduce the new module. After making that, set this to be a chil

[PATCH] D91602: [clang-tidy] Make clang-format and include-order-check coherent

njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/llvm-include-order.cpp:1 // RUN: %check_clang_tidy %s llvm-include-order %t -- -- -isystem %S/Inputs/Headers kadircet wrote: > njames93 wrote: > > Would it be wise to speci

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

njames93 marked 8 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp:313 +#endif +} aaron.ballman wrote: > We should probably add some tests for more pathological case

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

njames93 updated this revision to Diff 305897. 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/D91485/new/ https://reviews.llvm.org/D91485 Files: clang-tools-ext

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

njames93 updated this revision to Diff 305898. njames93 added a comment. Whoops missed one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91485/new/ https://reviews.llvm.org/D91485 Files: clang-tools-extra/clang-tidy/readability/ElseAfterReturnC

[PATCH] D91656: [clang-tidy] add concurrent module

njames93 added a comment. You have messed up the revisions, this should be the parent, D90944 should be the child. Thats why the build bot can't apply the patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91656/

[PATCH] D91694: [clangd] Remove the trailing "." in add-using message.

njames93 added a comment. If you are doing this may as well cover `DefineOutline` and `RemoveUsingNamespace` - that one can do away with formatv too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91694/new/ https://reviews.llvm.org/D91694 ___

[PATCH] D91656: [clang-tidy] add concurrency module

njames93 added a comment. Now that you have added the concurrency module as a new patch, all that code needs removing from this diff. If you generate the diff from that patch it should work. You should only be left with the code needed to add this check to that module. CHANGES SINCE LAST ACTI

[PATCH] D91656: [clang-tidy] add concurrency module

njames93 added a comment. Can you remove the code related to adding the mt-unsafe-posix check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91656/new/ https://reviews.llvm.org/D91656 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

njames93 updated this revision to Diff 306143. njames93 added a comment. Added test cases to ensure clang-tidy doesn't crash. Expanded auto out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91485/new/ https://reviews.llvm.org/D91485 Files: clan

[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/linuxkernel/MacroTrailingSemiCheck.cpp:15 + +#include + Any reason for including the c standard header? Comment at: clang-tools-extra/clang-tidy/linuxkernel/MacroTraili

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG617e8e5ee3bb: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches… (authored by njames93). Cha

[PATCH] D90568: [clang] Add [is|set]Nested methods to NamespaceDecl

njames93 added a comment. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90568/new/ https://reviews.llvm.org/D90568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D91656: [clang-tidy] add concurrency module

njames93 added a comment. LG, I have no further comments, though see what other say about this first. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91656/new/ https://reviews.llvm.org/D91656 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp:25 "linuxkernel-must-check-errs"); CheckFactories.registerCheck("linuxkernel-switch-semi"); } This check hasn't landed, can you please

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

njames93 added a comment. In D90282#2405615 , @aaron.ballman wrote: > I don't have a strong opinion on that approach. It would be a more flexible > approach and that's appealing, but it also means writing regular expressions > which is a bit unappealing

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

njames93 updated this revision to Diff 294895. njames93 added a comment. Address reviewer comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82089/new/ https://reviews.llvm.org/D82089 Files: clang-tools-extra/clang-tidy/modernize/LoopConvert

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

njames93 updated this revision to Diff 294897. njames93 marked an inline comment as done. njames93 added a comment. IsReverse= Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82089/new/ https://reviews.llvm.org/D82089 Files: clang-tools-extra/clan

[PATCH] D84176: [CMake][CTE] Add "check-clang-extra-..." targets to test only a particular Clang extra tool

njames93 added a comment. Just a follow up on this point, dependencies are a little messed up here. Running any of the check-clang-extra<...> results in all clang-tools-extra targets being built. Not ideal if you just want to test a small change to one tools and potentially having to rebuild al

[PATCH] D88297: [clangd] Trivial setter support when moving items to fields

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG01a30fa6787d: [clangd] Trivial setter support when moving items to fields (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SIN

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

njames93 added a comment. Not strictly relevant here, but this does open up the idea of enforcing the style where an enum constant is prefixed by the initials of the enum name. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-nota

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

njames93 added a comment. Can you please upload this diff with full context `-U99` Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:50 + +AST_MATCHER(ImplicitCastExpr, isArrayToPointerDecay) { + return Node.getCastKind() ==

[PATCH] D89065: [clang] Tweaked fixit for static assert with no message

njames93 created this revision. njames93 added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. njames93 requested review of this revision. If a static assert has a message as the right side of an and condition, suggest a fix it of replacing the '&&' to '

[PATCH] D89065: [clang] Tweaked fixit for static assert with no message

njames93 added a comment. I don't see any support for verifying fix-its in the test cases so unsure how i could go about testing this change. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:18 #include "clang/Basic/CharInfo.h" +#include "clang/Basic/DiagnosticParse.h" #include

[PATCH] D89065: [clang] Tweaked fixit for static assert with no message

njames93 updated this revision to Diff 297072. njames93 added a comment. Remove unnecessary changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89065/new/ https://reviews.llvm.org/D89065 Files: clang/lib/Parse/ParseDeclCXX.cpp Index: clang/l

[PATCH] D89065: [clang] Tweaked fixit for static assert with no message

njames93 planned changes to this revision. njames93 added a comment. Just noticed this doesn't quite work as expected, will update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89065/new/ https://reviews.llvm.org/D89065 ___

[PATCH] D89194: [clang-tidy] Fix crash in readability-function-cognitive-complexity on weak refs

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:504 Finder->addMatcher( - functionDecl( - allOf(isDefinition(), unless(anyOf(isDefaulted(), isDeleted(), -

[PATCH] D89065: [clang] Tweaked fixit for static assert with no message

njames93 updated this revision to Diff 297434. njames93 added a comment. FixIt now properly generated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89065/new/ https://reviews.llvm.org/D89065 Files: clang/lib/Parse/ParseDeclCXX.cpp Index: clang

[PATCH] D89407: [clang-tidy] Add scoped enum constants to identifier naming check

njames93 created this revision. njames93 added reviewers: alexfh, JonasToth, aaron.ballman, gribozavr2. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Added option `ScopedEnumConstant(Prefix|Case|Suffix)` to readability

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

njames93 added a comment. Is this diff been created incorrectly again? Taking a step back, Is Hungarian notation really a case style, Seems to me its mainly about the prefix and a user may want `DWORD dwUPPER_CASE`, Right now there is no way of adopting that. Maybe extend the options for hungar

[PATCH] D89407: [clang-tidy] Add scoped enum constants to identifier naming check

njames93 updated this revision to Diff 298200. njames93 added a comment. Small tweak to the docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89407/new/ https://reviews.llvm.org/D89407 Files: clang-tools-extra/clang-tidy/readability/IdentifierN

[PATCH] D89407: [clang-tidy] Add scoped enum constants to identifier naming check

njames93 updated this revision to Diff 298349. njames93 added a comment. Removed runtime check in release mode that EnumConstant DeclContext is a EnumDecl Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89407/new/ https://reviews.llvm.org/D89407 Fi

[PATCH] D89407: [clang-tidy] Add scoped enum constants to identifier naming check

njames93 added a comment. In D89407#2333560 , @Eugene.Zelenko wrote: > I think will be good idea to add test for suffix/prefix too. Not strictly necessary, there are tests that make sure prefixes and suffixes are applied correctly. Then there are tests

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8a548bc203cf: [clang-tidy] modernize-loop-convert reverse iteration support (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES S

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

njames93 updated this revision to Diff 298616. njames93 added a comment. Updated release notes to reflect change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79674/new/ https://reviews.llvm.org/D79674 Files: clang-tools-extra/clang-tidy/utils/R

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:127-128 getFileStyleFromOptions(const ClangTidyCheck::OptionsView &Options) { SmallVector, 0> Styles( SK_Count); SmallString<64> StyleString;

[PATCH] D91885: [clang-tidy] Add support for diagnostics with no location

njames93 created this revision. njames93 added reviewers: alexfh, aaron.ballman. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. Herald added a project: clang. njames93 requested review of this revision. Add methods for emitting diagnostics with no location as well as a specia

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 306794. njames93 marked an inline comment as done. njames93 added a comment. Reworked everything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 306796. njames93 marked 7 inline comments as done. njames93 added a comment. Messed up branches, fixed that Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: cl

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.h:20 +/// The base class of all clang-tidy config providers for clangd. +class TidyProvider { +public: sammccall wrote: > we're using inhe

[PATCH] D91908: [clang-tidy] Use compiled regex for AllowedRegexp in macro usage check

njames93 added a comment. While I'm a fan of this change, I think this is the wrong way to do it. Leave the check the same, but build the regex in the pp callback constructor. That should only get called once for the lifetime of the check. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 306850. njames93 added a comment. Fix ChecksToDisable to actually disable the checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, gribozavr2. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Addresses https://bugs.llvm.org/show_bug.cgi?id=48230. Handle the case when the

[PATCH] D91925: [clangd][NFC] Small tweak to combined provider

njames93 created this revision. njames93 added reviewers: sammccall, Bigcheese. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman. Herald added a project: clang. njames93 requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. This should address the

[PATCH] D91885: [clang-tidy] Add support for diagnostics with no location

njames93 updated this revision to Diff 306884. njames93 added a comment. Fix unittest failing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91885/new/ https://reviews.llvm.org/D91885 Files: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp clan

[PATCH] D90531: [clangd] Add clang-tidy options to config

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG20b69af7c9c8: [clangd] Add clang-tidy options to config (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D91908: [clang-tidy] Use compiled regex for AllowedRegexp in macro usage check

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. Because this is basically NFC and we already have tests for the AllowedRegexp option, there is no need to introduce new tests here. So long as those current tests pass with this change, thi

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:134-137 +if (!IgnoredRegexp.isValid()) { + llvm::errs() << "Invalid IgnoredRegexp regular expression: " + << IgnoredRegexpStr; +} ---

[PATCH] D91930: [clangd] Implement textDocument/codeLens

njames93 added a comment. Please upload the diff with full context and fix the failing unit tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91930/new/ https://reviews.llvm.org/D91930 ___ cfe-commits

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525 +return "; cannot be fixed because '" + Fixup + + "' is not a valid identifier"; gribozavr2 wrote: > I don't think we need to tell that to

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525 +return "; cannot be fixed because '" + Fixup + + "' is not a valid identifier"; gribozavr2 wrote: > njames93 wrote: > > gribozavr2 wrote:

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

njames93 accepted this revision. njames93 added a comment. LGTM, before you push this, can you edit the summary and title as its no longer about just the length of the name. Otherwise the commit message will likely lead to confusion on whats being added. CHANGES SINCE LAST ACTION https://rev

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

This revision was automatically updated to reflect the committed changes. Closed by commit rG9f3edc323a88: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid… (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D91915?vs=306854&id=307156#toc R

[PATCH] D90282: [clang-tidy] Support IgnoredRegexp configuration to selectively suppress identifier naming checks

njames93 added a comment. Just one last small nit. // You can ignore this, nothing to do with this patch. All these changes in the documentation tell me we should probably restructure the documentation for this check. Right now we have NxM lines of documentation for each combination of style ki

[PATCH] D91908: [clang-tidy] Use compiled regex for AllowedRegexp in macro usage check

This revision was automatically updated to reflect the committed changes. Closed by commit rG269ef315d1be: [clang-tidy] Use compiled regex for AllowedRegexp in macro usage check (authored by smhc, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D91908: [clang-tidy] Use compiled regex for AllowedRegexp in macro usage check

njames93 added a comment. In D91908#2410240 , @smhc wrote: > Thank you - I don't have commit access so am unable to merge it myself. The > clang-tidy tests are passing fine. Just checking, Has this landed correctly, showing you are the author? I'm assum

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307213. njames93 added a comment. Fix assertion in provideDefaultChecks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.tx

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307229. njames93 added a comment. Added PathRef to TidyProvider, this handles finding clang-tidy files when the compile command directory isn't the project root. Use elog for reporting errors instead of `llvm::errs` - It was an artefact from the code in Cla

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307349. njames93 added a comment. Small tweaks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/cla

[PATCH] D88172: [clangd] Extract common file-caching logic from ConfigProvider.

njames93 added a comment. In D88172#2414249 , @sammccall wrote: > @kbobyrev ping... I think we do actually want to land this, for use with > `.clang-tidy` files after D91029 Yes, I was looking at copying the config cache

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 marked 12 inline comments as done and an inline comment as not done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:241 +/// Empty clang tidy provider, using this as a provider will disable clang-tidy. +static void emptyTidyProvider(t

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307416. njames93 marked 5 inline comments as done. njames93 added a comment. Address (most of the) comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: c

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 marked 6 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:303 E.instantiate()->addCheckFactories(CTFactories); -CTContext.emplace(std::make_unique( -tidy::ClangTidyGlobalOptions(), Inputs.Opts.

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 updated this revision to Diff 307478. njames93 added a comment. Getting closer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91029/new/ https://reviews.llvm.org/D91029 Files: clang-tools-extra/clangd/CMakeLists.txt clang-tools-extra/

[PATCH] D91029: [clangd] Implement clang-tidy options from config

njames93 added inline comments. Comment at: clang-tools-extra/clangd/TidyProvider.cpp:194 +for (auto &Option : llvm::reverse(OptionStack)) + Opts.mergeWith(Option, ++Order); + }; sammccall wrote: > njames93 wrote: > > sammccall wrote: > > > This order b

[PATCH] D90282: [clang-tidy] Support IgnoredRegexp configuration to selectively suppress identifier naming checks

This revision was automatically updated to reflect the committed changes. Closed by commit rG9c4df9eecb6c: [clang-tidy] Support IgnoredRegexp configuration to selectively suppress… (authored by smhc, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

<    4   5   6   7   8   9   10   11   12   13   >