[PATCH] D90531: [clangd][WIP] Start implementing clang-tidy options into clangd config

njames93 created this revision. njames93 added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, mgorny. Herald added a project: clang. njames93 requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. First step of implementin

[PATCH] D90531: [clangd][WIP] Start implementing clang-tidy options into clangd config

njames93 added inline comments. Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:236-237 +StringRef Type(IsPositive ? "Add" : "Remove"); +// Don't support negating here, its handled if the item is in the Add or +// Remove list. +if (Str[0] == '-') {

[PATCH] D90535: [clang-tidy] Allow -warnings-as-errors to be specified from run_clang_tidy.py

njames93 added a comment. Just a general drive by comment, there have been quite a few patches recently that add arguments that just get forwarded straight to clang-tidy, would it not be a whole lot simpler if we could just automatically forward arguments straight to clang-tidy. clang-tidy itse

[PATCH] D90531: [clangd][WIP] Start implementing clang-tidy options into clangd config

njames93 updated this revision to Diff 302141. njames93 added a comment. Added unittests for reading and compiling config and fixed the bugs causing said unittests to fail. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90531/new/ https://reviews.l

[PATCH] D90552: [clangd] Set the User option for clang-tidy to mimick its behaviour

njames93 created this revision. njames93 added a reviewer: sammccall. 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. Probably not essential as afaik o

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

njames93 created this revision. njames93 added a reviewer: sammccall. 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. If an enum has different names fo

[PATCH] D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files

njames93 added a comment. In D61989#2314544 , @collinjackson wrote: > Are there any updates on this change? I see there's a comment that hasn't > been addressed yet. Is that the only roadblock stopping this from getting > merged? Tbh clang-tidy already

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

njames93 created this revision. njames93 added a reviewer: rsmith. Herald added subscribers: cfe-commits, martong. Herald added a reviewer: shafik. Herald added a project: clang. njames93 requested review of this revision. Adds support for NamespaceDecl to inform if its part of a nested namespace.

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

njames93 updated this revision to Diff 302191. njames93 added a comment. Herald added subscribers: llvm-commits, dexonsmith. Herald added a project: LLVM. Fix bug where prepare would return true erroneously when enum contains duplicated constants Repository: rG LLVM Github Monorepo CHANGES S

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

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:130 m(ObjcIvar) \ +m(HungarianNotation) \ Is this line needed? Comment at: clang-tools-extra/clang-tidy/readability/Iden

[PATCH] D90531: [clangd][WIP] Start implementing clang-tidy options into clangd config

njames93 updated this revision to Diff 302431. njames93 added a comment. Rename Tidy to ClangTidy in Fragment Extract CheckOptions logic into a new class DynamicDictParser, this should help if config is extended to load other dictionaries where keys are unknown strings.e Repository: rG LLVM

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

njames93 updated this revision to Diff 302437. njames93 added a comment. Added JsonNodeDumper and argument comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90568/new/ https://reviews.llvm.org/D90568 Files: clang/include/clang/AST/Decl.h

[PATCH] D90303: [ASTMatchers] Made isExpandedFromMacro Polymorphic

njames93 added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:313 + AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc), + std::string, MacroName) { // Verifies that the statement' beginning and end

[PATCH] D90531: [clangd][WIP] Start implementing clang-tidy options into clangd config

njames93 updated this revision to Diff 302552. njames93 added a comment. Removed support for using clang-tidy that will happen in a follow up as its a more involved change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90531/new/ https://reviews.l

[PATCH] D90682: [clangd][NFC] Make Located::operator->() use pointer sematics

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

[PATCH] D90303: [ASTMatchers] Made isExpandedFromMacro Polymorphic

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb091af790f19: [ASTMatchers] Made isExpandedFromMacro Polymorphic (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D90303?

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG163325086c35: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule (authored by h-joo, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D102322: [clang-tidy] Add '-target' CLI option to override target triple

njames93 added a comment. This change doesn't seem to accomplish anything. We can already set the target via the compile command. This can be done in the compilation database directly or by using the `extra-arg` and `extra-arg-before` command line arguments. If you're not using a compilation dat

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG4c59ab34f7bd: [clang-tidy][NFC] Simplify a lot of bugprone-sizeof-expression matchers (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

njames93 added a comment. Whats the intended behaviour for derived classes and their destructors? Can test be added to demonstrate that behaviour? Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualBaseClassDestructorCheck.cpp:44 + "making it public and

[PATCH] D97567: [clang-tidy] performance-* checks: Also allow allow member expressions to be used in a const manner.

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM, but with a very minor nit. Comment at: clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp:49 declRefExpr(to(varDecl(equalsNode(&VarDecl.bind("declRe

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

njames93 added a comment. I think the best method is to In D86671#2750957 , @dougpuob wrote: > Hi @njames93: > Could you do me a favor? Because it is my first patch, something I'm not > sure. I'm confused about can I land this patch now? I read the "LLVM

[PATCH] D102337: [clang-tidy] Fix test that requires Windows platofrm

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102337/new/ https://reviews.llvm.org/D102337 ___ cfe-commits mailing list cfe-comm

[PATCH] D102369: [ASTMatchers][NFC] Remove runtime checks where compile time is sufficient

njames93 created this revision. njames93 added reviewers: klimek, aaron.ballman, alexfh. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Some polymorphic matchers use dyn_cast in cases where we know the type from the TemplateParam

[PATCH] D102369: [ASTMatchers][NFC] Remove runtime checks where compile time is sufficient

njames93 added a comment. In D102369#2756493 , @aaron.ballman wrote: > I'm not opposed, but I don't know that this is really an improvement. We've > gone from a pretty simple code pattern to needing to spell out the type twice > with a type_trait, and

[PATCH] D102576: [clang-tidy] cppcoreguidelines-avoid-do-while: a new check

njames93 added a comment. This is going to be very noisey for codebases that use a do...while macro: #define MACRO() do { Something(); With(); Multiple(); Statements(); } while (false) Maybe you should ignore do statements that are in macro expansions as well as potentially do statements wit

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

njames93 added a comment. Is it worth using regex to ignore some operators: `*::operator bool` to ignore all bool conversions etc. Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst:12-14 +This check implements `C.46

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

njames93 updated this revision to Diff 318315. njames93 added a comment. Split up the code a little more. Fix a few malformed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94942/new/ https://reviews.llvm.org/D94942 Files: clang-tools-e

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

njames93 added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ImplementAbstract.cpp:38-40 + // If we have any pure virtual methods declared in the root (The class + // this tweak was invoked on), assume the user probably doesn't want to + //

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

njames93 added a comment. In D94942#2515049 , @kadircet wrote: > Thanks this looks great, and something i've been longing for some time! But I > got a couple of questions about the how the interaction is designed (sorry if > I missed some high level disc

[PATCH] D95231: [clangd] Selection handles CXXBaseSpecifier

njames93 created this revision. njames93 added reviewers: kadircet, hokein, sammccall. Herald added subscribers: usaxena95, arphaman. njames93 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Selection now includes the

[PATCH] D95231: [clangd] Selection handles CXXBaseSpecifier

njames93 updated this revision to Diff 318523. njames93 added a comment. Update FindTargets to fix symbol renaming on a CXXBaseSpecifier. Add test to ensure rename wont trigger on a CXXBaseSpecifier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D952

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

njames93 updated this revision to Diff 318552. njames93 added a comment. If cursor is over one of the base specifiers, offer to implement only the pure methods from that base class. Depends on D95231 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

njames93 added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ImplementAbstract.cpp:354 + return "Implement pure virtual methods from '" + + FromBase->getType().getAsString() + "'"; +} Maybe this should be changed as it

[PATCH] D95270: [clangd][NFC] Simplify handing on methods with no params

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. Add bind methods handling the case

[PATCH] D95131: [CodeComplete] Add ranged for loops code pattern.

This revision was automatically updated to reflect the committed changes. Closed by commit rGd18c3c7b18e9: [CodeComplete] Add ranged for loops code pattern. (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95131/new/ https://rev

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

njames93 updated this revision to Diff 318678. njames93 edited the summary of this revision. njames93 added a comment. Fix failing tests. Updated message for tweak from a specified base class. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94942/new/

[PATCH] D95231: [clangd] Selection handles CXXBaseSpecifier

njames93 added inline comments. Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:272-283 + R"cpp( +struct Foo {}; +struct Bar : [[private Fo^o]] {}; + )cpp", + "CXXBaseSpecifier", + }, + { -

[PATCH] D95231: [clangd] Selection handles CXXBaseSpecifier

njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:496 } + bool TraverseCXXBaseSpecifier(const CXXBaseSpecifier &X) { +auto N = DynTypedNode::create(X); sammccall wrote: > can we n

[PATCH] D95231: [clangd] Selection handles CXXBaseSpecifier

njames93 updated this revision to Diff 318930. njames93 marked 2 inline comments as done. njames93 added a comment. Update behaviour to keep RecordTypeLoc seletion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95231/new/ https://reviews.llvm.org/D

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

njames93 updated this revision to Diff 318932. njames93 added a comment. Update getSelectedRecord to work now that BaseSpecifier may contain nested Nodes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94942/new/ https://reviews.llvm.org/D94942 Fi

[PATCH] D95231: [clangd] Selection handles CXXBaseSpecifier

njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:272-283 + R"cpp( +struct Foo {}; +struct Bar : [[private Fo^o]] {}; + )cpp", + "CXXBaseS

[PATCH] D95231: [clangd] Selection handles CXXBaseSpecifier

njames93 updated this revision to Diff 318933. njames93 marked an inline comment as done. njames93 added a comment. Remove FindTarget to instead add in a follow up patch. Side note, Think I'll wait til after tomorrow to land this, unsure if the changes will break anything elsewhere. Repository:

[PATCH] D95338: [clangd] FindTarget resolves base specifier

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. FindTarget on the virtual keyword

[PATCH] D95270: [clangd][NFC] Simplify handing on methods with no params

njames93 updated this revision to Diff 318942. njames93 marked an inline comment as done. njames93 added a comment. Remove unnecessary specialization. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95270/new/ https://reviews.llvm.org/D95270 Files:

[PATCH] D95270: [clangd][NFC] Simplify handing on methods with no params

njames93 added inline comments. Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:262 + template + void bind(const char *Method, +void (ClangdLSPServer::*Handler)(const NoParams &, kadircet wrote: > why do we need this one ? Good point, this

[PATCH] D95338: [clangd] FindTarget resolves base specifier

njames93 updated this revision to Diff 318968. njames93 added a comment. Remove unintended formatting change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95338/new/ https://reviews.llvm.org/D95338 Files: clang-tools-extra/clangd/FindTarget.cpp

[PATCH] D95362: [clangd] Parse Diagnostics block, and nest ClangTidy block under it.

njames93 added a comment. LG, Obviously we need to get this in before the branch or get it cherrypicked. Also documentation will need updating on https://github.com/llvm/clangd-www. Do you want to sort that out? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D95270: [clangd][NFC] Simplify handing on methods with no params

This revision was automatically updated to reflect the committed changes. Closed by commit rGf05b492aae4d: [clangd][NFC] Simplify handing on methods with no params (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95270/new/ http

[PATCH] D95385: Revert "[clangd][NFC] Simplify handing on methods with no params"

njames93 added a comment. Thanks for reverting this, That old full specialization allowed at class scope defect report always gets me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95385/new/ https://reviews.llvm.org/D95385 __

[PATCH] D95407: [clangd] NFC Silence some buildbot warnings

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. clang-tools-extra/clangd/unittest

[PATCH] D95231: [clangd] Selection handles CXXBaseSpecifier

This revision was automatically updated to reflect the committed changes. Closed by commit rGd92413a45e20: [clangd] Selection handles CXXBaseSpecifier (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95231/new/ https://reviews.l

[PATCH] D95338: [clangd] FindTarget resolves base specifier

This revision was automatically updated to reflect the committed changes. Closed by commit rG7730599c4164: [clangd] FindTarget resolves base specifier (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95338/new/ https://reviews.l

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

njames93 updated this revision to Diff 319898. njames93 added a comment. - Fix an issue where replacements could conflict with each other. - Add support for defining method stubs instead of just declaring them, possibly need a way to configure this behaviour. - Change tests to ignore whitespace b

[PATCH] D95653: [cte] [clang-tidy] Fix linking tests to LLVMTestingSupport

njames93 added a comment. I think I introduced this failure due to my abysmal knowledge of CMake and LLVM's library structure, So I'm definitely not qualified to say if this is a good fix, but seeing as it fixes the linker error I'll give it a tentative LG. CHANGES SINCE LAST ACTION https://

[PATCH] D95653: [clang-tidy] Fix linking tests to LLVMTestingSupport

njames93 added a comment. In D95653#2530163 , @mgorny wrote: > In D95653#2529776 , @njames93 wrote: > >> I think I introduced this failure due to my abysmal In D95653#2530163

[PATCH] D93800: [clangd][WIP] Add caching behaviour for clang-format config

njames93 updated this revision to Diff 320301. njames93 added a comment. Refactor alot of the implementation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93800/new/ https://reviews.llvm.org/D93800 Files: clang-tools-extra/clangd/CMakeLists.txt

[PATCH] D95714: clang-tidy: modernize-use-nullptr mistakenly fixes rewritten spaceship operator comparisons

njames93 added a comment. Thanks for looking at this, its been on my todo list for long time. Regarding the hasGrandparent idea, that could be recreated with `hasParent(expr(hasParent(cxxRewrittenBinaryOperator(` Though looking at the (trimmed) AST I'm not sure that's enough. Appears to have

[PATCH] D95726: clang-tidy: modernize-use-nullptr mistakenly fixes rewritten spaceship operator comparisons - hasGrandparent version

njames93 requested changes to this revision. njames93 added a comment. This revision now requires changes to proceed. Not a fan of this approach, as pointed out in D95714 the `hasGrandparent` functionality can be achieved by chaining `hasParent` calls. Reposit

[PATCH] D93800: [clangd] Add caching behaviour for clang-format config

njames93 updated this revision to Diff 320309. njames93 added a comment. Tests pass on my machine, rebasing on main and trigger a fresh rebuild. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93800/new/ https://reviews.llvm.org/D93800 Files: clan

[PATCH] D93800: [clangd] Add caching behaviour for clang-format config

njames93 updated this revision to Diff 320319. njames93 added a comment. Hopefully fix the build failing. ASAN said it was a use-after-scope issue which seemed suspicious. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93800/new/ https://reviews.ll

[PATCH] D93800: [clangd] Add caching behaviour for clang-format config

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

[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

njames93 added a comment. In D95714#2532516 , @poelmanc wrote: > @njames93 Thank you so much for the quick feedback, I made your suggested > changes and added a test that it properly converts `result = (a1 > (ptr == 0 > ? a1 : a2));` to `result = (a1 > (

[PATCH] D95771: [clang-tidy] fix modernize-loop-convert to retain needed array-like operator[]

njames93 edited reviewers, added: aaron.ballman, alexfh, njames93; removed: sammccall, hokein. njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM, Can you fix the formatting as well. Comment at: clang-tools-extra/t

[PATCH] D93844: [clang-format] Add possibility to be based on parent directory

njames93 added a comment. As a follow up it may be wise to pass a diag handler to parseConfiguration as when we parse it a second time, we probably want to disregard any warnings (like unknown key) detected as they will have been printed on the first pass. Comment at: clang/l

[PATCH] D95168: [clang-format] Add InsertBraces option

njames93 added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:17996 +format(ForSourceLong, Style)); +} + MyDeveloperDay wrote: > MyDeveloperDay wrote: > > MyDeveloperDay wrote: > > > are you testing do/while? > > whilst people dis

[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

njames93 added a comment. In D95714#2540626 , @poelmanc wrote: > Thanks for all the great feedback I received here. To give credit where > credit's due, this updated revision to UseNullptrCheck.cpp is now actually > 100% @steveire's //suggested// code. E

[PATCH] D96082: [clang-tidy] Add 'readability-useless-return-value' check

njames93 requested changes to this revision. njames93 added a comment. This revision now requires changes to proceed. This check, more specifically the fixes, seem quite dangerous. Changing function signatures is highly likely to cause compilation issues. For a starters this check doesn't appear

[PATCH] D96142: [clang-tidy] Simplify too-small loop variable check

njames93 added a comment. I'm not sure about this. The warning is good and addresses a real problem. I'm not sure what the infrastructure surrounding this is but in clang it would show a "stack trace" of instantiations that result in this warning. Maybe we should think of something similar in c

[PATCH] D93800: [clangd] Add caching behaviour for clang-format config

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

[PATCH] D96204: [clangd] Fix clang tidy provider when multiple config files exist in directory tree

njames93 created this revision. njames93 added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman, mgorny. njames93 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Currently Clang tidy prov

[PATCH] D96204: [clangd] Fix clang tidy provider when multiple config files exist in directory tree

njames93 updated this revision to Diff 321937. njames93 added a comment. Newline at eof. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96204/new/ https://reviews.llvm.org/D96204 Files: clang-tools-extra/clangd/TidyProvider.cpp clang-tools-extr

[PATCH] D96204: [clangd] Fix clang tidy provider when multiple config files exist in directory tree

njames93 added a comment. I have made an extra test case that compares the output of our provider to the output of a `clang::tidy::FileOptionsProvider`, This would ensure coherent behaviour between our implementation and clang-tidy implementation. However I'm not sure it should be included as an

[PATCH] D96209: [clang-tidy] Fix readability-avoid-const-params-in-decls removing const in template paramaters

njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh. Herald added subscribers: nullptr.cpp, xazax.hun. njames93 requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://bugs.llvm.org/show_bug.cgi?id=49063

[PATCH] D95771: [clang-tidy] fix modernize-loop-convert to retain needed array-like operator[]

njames93 added a comment. Herald added a subscriber: nullptr.cpp. In D95771#2540598 , @poelmanc wrote: > @njames93 Thanks for the review and for accepting this revision. I lack > llvm-project commit access so if it's good to go I would greatly appreciate

[PATCH] D96114: [ASTMatchers] Fix parent-child traversal between functions and parms

njames93 added a reviewer: klimek. njames93 added a comment. I have no issues here, but see what the others say as well. Also related post to cfe-dev - https://lists.llvm.org/pipermail/cfe-dev/2021-February/067629.html Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:148-1

[PATCH] D93800: [clangd] Add caching behaviour for clang-format config

njames93 updated this revision to Diff 321988. njames93 added a comment. Add unittests for provider. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93800/new/ https://reviews.llvm.org/D93800 Files: clang-tools-extra/clangd/CMakeLists.txt clang-

[PATCH] D95771: [clang-tidy] fix modernize-loop-convert to retain needed array-like operator[]

This revision was automatically updated to reflect the committed changes. Closed by commit rG5229edd66742: [clang-tidy] fix modernize-loop-convert to retain needed array-like operator[] (authored by poelmanc, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D95771: [clang-tidy] fix modernize-loop-convert to retain needed array-like operator[]

njames93 added a comment. In D95771#2547160 , @poelmanc wrote: > In D95771#2547102 , @njames93 wrote: > >> Should this be committed using `poelmanc `? > > That or `Conrad Poelman ` would be great, thanks. I committ

[PATCH] D92133: [clangd] Cache .clang-tidy files again.

njames93 added a comment. Unfortunately this broke the search order for clang-tidy files in root directories in favour or subdirectories. This wasn't caught as we didn't have tests for this. I've put a fix for this in D96204 with appropriate tests. Repository

[PATCH] D95771: [clang-tidy] fix modernize-loop-convert to retain needed array-like operator[]

njames93 added a comment. In D95771#2547467 , @poelmanc wrote: > Thanks for the commit, and thanks for the heads-up! I've now added the > address in my github account. Its updated now and the commit is linked to your account. Repository: rG LLVM Gith

[PATCH] D96245: [clangd] Propagate CodeActions in addition to Fixes for diagnostics

njames93 added a comment. Emitting fixes for a diagnostic in another file seems dangerous, what's the intended use case for this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96245/new/ https://reviews.llvm.org/D96245 ___

[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. Herald added a subscriber: nullptr.cpp. LGTM, Same as last time for the commit? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95714/new/ https://reviews.llvm.org/D95714 _

[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

njames93 added a comment. I'm unable to cleanly apply this patch, looks like something was a bit off when you created the diff. ➜ llvm-project git:(main) ✗ arc patch D95714 INFO Base commit is not in local repository; trying to fetch. Created and checked out b

[PATCH] D18961: Add a readability-deleted-default clang-tidy check.

njames93 added a comment. Herald added a subscriber: mgorny. Now that there is a stable warning in clang for this check that is enabled without specifying any warning flags, Is there reason to think about retiring this check? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D96286: [clangd] Change TidyProvider cache to use a linked list approach

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. I know linked lists are the work

[PATCH] D96286: [clangd] Change TidyProvider cache to use a linked list approach

njames93 added a comment. A similar approach could be used for the config provider, however It may not make as much sense there. The directories are traversed in the opposite order and we will always attempt to read from each directory. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D95714: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator comparisons

This revision was automatically updated to reflect the committed changes. Closed by commit rG98146c1f5d0c: [clang-tidy] fix modernize-use-nullptr false positive with spaceship operator… (authored by poelmanc, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D96281: [clang-tidy] Add options to flag individual core increments and to ignore macros to readability-function-cognitive-complexity check.

njames93 added a comment. How does this handle a macro where the argument has complex code. MACRO(if (...) {}); In this case the macro code is definitely increasing the complexity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96281/new/ https:

[PATCH] D96495: [clangd] Retire the cross-file-rename command-line flag.

njames93 added a comment. Those test failures look related. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96495/new/ https://reviews.llvm.org/D96495 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D96508: [clangd] Retire clang-tidy-checks flag.

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. In clangd-12 the ability to overr

[PATCH] D96508: [clangd] Retire clang-tidy-checks flag.

njames93 added a comment. Ping? It would be nice to get this fix in the release branch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96508/new/ https://reviews.llvm.org/D96508 ___ cfe-commits mailing li

[PATCH] D96508: [clangd] Retire clang-tidy-checks flag.

njames93 added a comment. In D96508#2559936 , @sammccall wrote: > I think we should have the config mechanism available for one release cycle > before removing the old one. > > So I think it's fine to retire the flag but we shouldn't pick this into the >

[PATCH] D96204: [clangd] Fix clang tidy provider when multiple config files exist in directory tree

njames93 added a comment. @sammccall I meant to ping this one sorry. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96204/new/ https://reviews.llvm.org/D96204 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D96138: [clang-tidy] Simplify delete null ptr check

njames93 added a comment. Seems that your test cases are failing on windows, I'm guessing this is a MSVC template compatibility issue. Probably need to add `-fno-delayed-template-parsing` to the clang driver args (after `-- --`). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D96139: [clang-tidy] Simplify inaccurate erase check

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LG with nit. Comment at: clang-tools-extra/clang-tidy/bugprone/InaccurateEraseCheck.cpp:25-27 + 1, anyOf(cxxMemberCallExpr(callee(cxxMethodDecl(hasName("end"

[PATCH] D96223: [clang-tidy] Simplify static assert check

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LG with a few nits. Comment at: clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp:49-51 + expr(anyOf(expr(anyOf(AssertExprRoot, +un

[PATCH] D96204: [clangd] Fix clang tidy provider when multiple config files exist in directory tree

This revision was automatically updated to reflect the committed changes. Closed by commit rGba3ea9c60f0f: [clangd] Fix clang tidy provider when multiple config files exist in directory… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D94554: [clangd] Add a Filesystem that overlays Dirty files.

njames93 updated this revision to Diff 323378. njames93 added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94554/new/ https://reviews.llvm.org/D94554 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp clang-tools-extra/clan

[PATCH] D95043: [clangd] Use Dirty Filesystem for cross file rename.

njames93 updated this revision to Diff 323379. njames93 added a comment. Rebase and fix up tests after changes to rename. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95043/new/ https://reviews.llvm.org/D95043 Files: clang-tools-extra/clangd/Cl

<    6   7   8   9   10   11   12   13   14   15   >