[PATCH] D124650: [clang-tidy] Simplify boolean expressions by DeMorgan's theorem

njames93 added a comment. I've done my own implementation, but its definitely over engineered, WDYT? D124806 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124650/new/ https://reviews.llvm.org/D124650 ___ cf

[PATCH] D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice.

njames93 marked 4 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:51 ClangTidyContext *Context) -: NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions), +: NamePrefix((CheckName + ".").str(

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

njames93 created this revision. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 updated this revision to Diff 426524. njames93 added a comment. njames93 updated this revision to Diff 426976. njames93 edited the summary of this revision. njames93 added revie

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

njames93 updated this revision to Diff 427038. njames93 added a comment. Enhanced support for parens, both adding them in when needed as well as removing some un-needed parens. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124806/new/ https://revi

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

njames93 updated this revision to Diff 427082. njames93 added a comment. Reimplement matchers as an ASTVisitor instead. This massively simplifies the tracking of nested cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124806/new/ https://review

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

njames93 updated this revision to Diff 427087. njames93 added a comment. Added release notes. Fix build issues with previous push. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124806/new/ https://reviews.llvm.org/D124806 Files: clang-tools-extr

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

njames93 created this revision. njames93 added reviewers: alexfh, klimek, aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. R

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

njames93 updated this revision to Diff 427418. njames93 added a comment. Remove redundant traversal of CompoundStmt in a replace method. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125026/new/ https://reviews.llvm.org/D125026 Files: clang-tool

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

njames93 updated this revision to Diff 427430. njames93 added a comment. Transform final matcher, Benchmark -> 0.82s. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125026/new/ https://reviews.llvm.org/D125026 Files: clang-tools-extra/clang-tidy/

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

njames93 updated this revision to Diff 427436. njames93 added a comment. Remove SimplifyBoolExprMatchers header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125026/new/ https://reviews.llvm.org/D125026 Files: clang-tools-extra/clang-tidy/readab

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

njames93 updated this revision to Diff 427529. njames93 added a comment. Remove unnecessary tests in ReadabilityTidyModule now that the SimplifyBooleanExprMatchers header has been removed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125026/new/

[PATCH] D124320: [clang-tidy] Add createChecks method that also checks for LangaugeOptions

This revision was automatically updated to reflect the committed changes. Closed by commit rGdd87aceb514d: [clang-tidy][NFC] Add createChecks method that also checks for LangaugeOptions (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D129706: [NFCI][clang-tidy] Reimplement GlobList without relying on Regex.

njames93 created this revision. njames93 added reviewers: alexfh, aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Regex is

[PATCH] D129706: [NFCI][clang-tidy] Reimplement GlobList without relying on Regex.

njames93 updated this revision to Diff 57. njames93 added a comment. Fix comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129706/new/ https://reviews.llvm.org/D129706 Files: clang-tools-extra/clang-tidy/GlobList.cpp clang-tools-extr

[PATCH] D129706: [NFCI][clang-tidy] Reimplement GlobList without relying on Regex.

njames93 updated this revision to Diff 444786. njames93 added a comment. Added functionality for Checking equivalence to a StringRef without having to build the Glob. These features are ground work for upcoming patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D129706: [NFCI][clang-tidy] Reimplement GlobList without relying on Regex.

njames93 updated this revision to Diff 444820. njames93 added a comment. More code cleanup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129706/new/ https://reviews.llvm.org/D129706 Files: clang-tools-extra/clang-tidy/GlobList.cpp clang-tools

[PATCH] D129953: [PATCH] [clang-tidy] NFC: add preposition "of" to code annotation of ElseAfterReturnCheck

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. While you're here could you also put full stops( or periods 🙃) at the end of the sentences. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D129953: [PATCH] [clang-tidy] NFC: add preposition "of" to code annotation of ElseAfterReturnCheck

njames93 added a comment. In D129953#3657925 , @zhouyizhou wrote: > In D129953#3657920 , @njames93 > wrote: > >> While you're here could you also put full stops( or periods 🙃) at the end of >> the sentences. >

[PATCH] D129706: [NFCI][clang-tidy] Reimplement GlobList without relying on Regex.

njames93 updated this revision to Diff 445464. njames93 added a comment. Enable escaping the leading `-` at the start of a glob to match strings starting with a `-`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129706/new/ https://reviews.llvm.or

[PATCH] D129706: [NFCI][clang-tidy] Reimplement GlobList without relying on Regex.

njames93 updated this revision to Diff 445469. njames93 added a comment. Ensure escaped negatives are printed correctly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129706/new/ https://reviews.llvm.org/D129706 Files: clang-tools-extra/clang-ti

[PATCH] D130005: [clang-tidy] Prettify GlobList before serializing Options

njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, LegalizeAdulthood. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added subscribers: cfe-commits, aheejin. Herald added a project: cl

[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

njames93 added a comment. Can you add test cases where there is a struct with an anonymous union and another field that does need an initializer, just to verify warning and fixit behaviour. struct S3 { S3() {} int A; union { int B; int C = 0; // Possibly a case where t

[PATCH] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

njames93 added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp:1 +#include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h" #include "ClangTidyTest.h" kwk wrote: > @LegalizeAdulthood I'm doing standalone builds

[PATCH] D130026: [clang-tidy] Remove unnecessary code from ReadabilityModuleTest

njames93 created this revision. njames93 added reviewers: LegalizeAdulthood, aaron.ballman, kwk, serge-sans-paille. Herald added subscribers: xazax.hun, mgorny. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscr

[PATCH] D130026: [clang-tidy] Remove unnecessary code from ReadabilityModuleTest

This revision was automatically updated to reflect the committed changes. Closed by commit rG6357f1c1aa76: [clang-tidy] Remove unnecessary code from ReadabilityModuleTest (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130026/ne

[PATCH] D129706: [NFCI][clang-tidy] Reimplement GlobList without relying on Regex.

njames93 updated this revision to Diff 446097. njames93 added a comment. Improve handling of consecutive wild cards. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129706/new/ https://reviews.llvm.org/D129706 Files: clang-tools-extra/clang-tidy/G

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

njames93 created this revision. njames93 added reviewers: alexfh, aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun, mgorny. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. A

[PATCH] D130130: [clang-tidy] Add an IgnoreMacros option to readability-avoid-const-params-in-decls

njames93 added a comment. Please can you upload your diff with full context(or use arcanist which does it for you). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130130/new/ https://reviews.llvm.org/D130130 ___

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

njames93 updated this revision to Diff 446197. njames93 added a comment. Add option to wrap early exit in braces. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130181/new/ https://reviews.llvm.org/D130181 Files: clang-tools-extra/clang-tidy/read

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp:217 + LineCountThreshold(Options.get("LineCountThreshold", 10U)), + WrapInBraces(Options.get("WrapInBraces", false)) {} + I have an idea that we

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

njames93 updated this revision to Diff 446219. njames93 added a comment. Added logic to infer WrapInBraces option if unspecified. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130181/new/ https://reviews.llvm.org/D130181 Files: clang-tools-extra

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

njames93 updated this revision to Diff 446255. njames93 added a comment. Fix pre-build failing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130181/new/ https://reviews.llvm.org/D130181 Files: clang-tools-extra/clang-tidy/readability/CMakeLists

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

njames93 accepted this revision. njames93 added a comment. LGTM with those changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54943/new/ https://reviews.llvm.org/D54943 ___ cfe-commits mailing list c

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

njames93 closed this revision. njames93 added a comment. In D120185#3668029 , @mgorny wrote: > This change broke standalone build of clang. Please fix. I've pushed rG251b5b864183e868ffc86522e320f91ab3c5a771

[PATCH] D129953: [PATCH] [clang-tidy] NFC: add preposition "of" to code annotation of ElseAfterReturnCheck

This revision was automatically updated to reflect the committed changes. Closed by commit rG934d60382673: [clang-tidy][NFC] Add preposition "of" to code annotation of… (authored by zhouyizhou, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

njames93 updated this revision to Diff 447236. njames93 added a comment. Add option `SplitConjunctions` to alter fix-it for `if` statements with `&&` in the condition. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130181/new/ https://reviews.llvm.

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

njames93 updated this revision to Diff 447246. njames93 added a comment. NL Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130181/new/ https://reviews.llvm.org/D130181 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-e

[PATCH] D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice.

This revision was automatically updated to reflect the committed changes. njames93 marked 4 inline comments as done. Closed by commit rG12cb540529e4: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would… (authored by njames93). Repository: rG LLVM Github Monorepo CHA

[PATCH] D125026: [clang-tidy][NFC] Reimplement most of SimplifyBooleanExpr with RecursiveASTVisitors

njames93 updated this revision to Diff 428041. njames93 added a comment. Clean up some code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125026/new/ https://reviews.llvm.org/D125026 Files: clang-tools-extra/clang-tidy/readability/SimplifyBoole

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

njames93 updated this revision to Diff 428069. njames93 added a comment. Tweak fix to not remove parens if it will result in a LogicalOpParentheses warning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124806/new/ https://reviews.llvm.org/D124806

[PATCH] D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice.

njames93 added a comment. In D124341#3502659 , @uabelho wrote: > Hi, > > I noticed that this patch isn't NFC. Whoops, good catch. I left in some debugging code, fixed in rGa308a55720249749

[PATCH] D125026: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors

njames93 updated this revision to Diff 429659. njames93 added a comment. Tweak logic for detecting nested `if`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125026/new/ https://reviews.llvm.org/D125026 Files: clang-tools-extra/clang-tidy/readab

[PATCH] D125026: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors

njames93 updated this revision to Diff 429660. njames93 added a comment. Nit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125026/new/ https://reviews.llvm.org/D125026 Files: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp

[PATCH] D125026: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors

njames93 added a comment. Since latest update speed up gone from 0.87s to 0.04s for SemaCodeComplete. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125026/new/ https://reviews.llvm.org/D125026 ___ cfe-co

[PATCH] D125026: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors

njames93 added a comment. So those times were a little unfair as now we don't use ParentMapContext, however the cost of building that is included in the running time for the check. In a use case where you have other enabled checks with do make use of that, the cost for building would be moved to

[PATCH] D125026: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors

This revision was automatically updated to reflect the committed changes. Closed by commit rG6f8726191960: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

njames93 updated this revision to Diff 429715. njames93 added a comment. Rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124806/new/ https://reviews.llvm.org/D124806 Files: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

njames93 updated this revision to Diff 429907. njames93 added a comment. Extend fix-its to also support removing parens where possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124806/new/ https://reviews.llvm.org/D124806 Files: clang-tools

[PATCH] D125026: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors

njames93 added a comment. In D125026#3519831 , @LegalizeAdulthood wrote: > Just for my own edification, how did you know/suspect that a pure visitor > would be faster than matchers? Mainly cause the fact we are creating 2 matcher expressions that diffe

[PATCH] D125874: [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init statement or condition variable

njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, LegalizeAdulthood. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-

[PATCH] D125877: [clang-tidy] Fix readability-simplify-boolean-expr crash with implicit cast in return.

njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, LegalizeAdulthood. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-

[PATCH] D125877: [clang-tidy] Fix readability-simplify-boolean-expr crash with implicit cast in return.

This revision was automatically updated to reflect the committed changes. Closed by commit rG4739176fd304: [clang-tidy] Fix readability-simplify-boolean-expr crash with implicit cast in… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

njames93 added a comment. In D124806#3523109 , @LegalizeAdulthood wrote: > LGTM! Thanks for taking my idea and implementing it much better than what I > had `:)`. Cheers. Can I ask why you feel that this should be behind an option? Repository: rG

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

njames93 updated this revision to Diff 430472. njames93 added a comment. Remove unnecessary parens in documentation as check will remove the parens when possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124806/new/ https://reviews.llvm.org/D

[PATCH] D125874: [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init statement or condition variable

njames93 updated this revision to Diff 430473. njames93 added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125874/new/ https://reviews.llvm.org/D125874 Files: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.

[PATCH] D125885: [clang-tidy] bugprone-argument-comment: Ignore calls to user-defined literals

njames93 added a comment. These tests aren't that great. I'd advise adding a test that just checks using the userDefinedLiteral in `bugprone-argument-comment-literals.cpp`, That check file will add comments to parameters that don't have them. That way you aren't testing against distinguishing be

[PATCH] D125874: [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init statement or condition variable

This revision was automatically updated to reflect the committed changes. Closed by commit rG35660247dd9c: [clang-tidy] Fix readability-simplify-boolean-expr when Ifs have an init… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D125949: [clang-tidy] modernize-avoid-bind: Fix crash when method name is not a simple identifier

njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind.cpp:367 +// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer a lambda to std::bind +// CHECK-FIXES: auto EEE = [d] { d->operator()(1, 2); } + Fixi

[PATCH] D125771: [clang-tidy] Add a useful note about -std=c++11-or-later

njames93 added a comment. Tbh the whole testing infrastructure for clang-tidy is a mess. When I have wanted to verify fixes and diagnostics for header files, I find manually invoking clang-tidy is better than using the check_clang_tidy script. I don't have the time right now, but a verify mode l

[PATCH] D126097: [clang-tidy] Adds the NSDateFormatter checker to clang-tidy

njames93 added a comment. In D126097#3528787 , @NoQ wrote: > Looks awesome! > >> add_new_check.py > > I'm surprised it wasn't executable already, do we want to keep it? It keeps getting reset, likely due to Windows users committing changes. It should be

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

This revision was automatically updated to reflect the committed changes. Closed by commit rGaf77b1d99016: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D126162: [clang-tidy] Extend SimplifyBooleanExpr demorgan support.

njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, LegalizeAdulthood. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. njames93 requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

njames93 added a comment. Pretty much good to go, just a few nits with the tests. Can you add CHECK-FIXES directives for all warnings if there should be a fixit. If there shouldn't be one could you either add a comment saying there shouldn't be one, or put a CHECK-FIXES-NOT directive. Repositor

[PATCH] D126077: Fix stack crash in classIsDerivedFrom triggered by clang-tidy

njames93 edited reviewers, added: aaron.ballman, njames93, alexfh, LegalizeAdulthood; removed: bixia, aartbik. njames93 added a comment. Do you know of any other instances where this issue could surface? Please can you add a line to the `Improved Checks` section in `clang-tools-extra/docs/Relea

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

njames93 added a comment. Please upload diff with full context. Can you add a note about this in the release notes. As a side point I'm not sure this change really follows what the rule is trying to before. The rule is about not using std::memset to reinitialise objects that aren't trivial. Hav

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

njames93 added a comment. This diff looks like it's rooted on the clang-tools-extra directory which is why the pre-merge bot is failing to build. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126186/new/ https://reviews.llvm.org/D126186 ___

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 added a comment. Sorry I didn't make it obvious with the test cases. The fix will never extend the lifetime of variables either in the condition or declared in the else block. It will only apply if the if is the last statement in its scope. Though I should check back through the scope

[PATCH] D71980: Fix ClangTidy Bug - 44229, 33203 and 32204

njames93 updated this revision to Diff 235525. njames93 added a comment. Forgot to clang-format... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71980/new/ https://reviews.llvm.org/D71980 Files: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp clang-tools-extra/clang-tidy

[PATCH] D71980: Fix ClangTidy Bug - 44229, 33203 and 32204

njames93 created this revision. njames93 added reviewers: clang-tools-extra, cfe-commits. fixes readability-misleading-indentation broken for if constexpr , readability-braces-around-statements broken for if constexpr

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 added a comment. Adding checks to see if there are any declarations already in the ifs contained scope is really starting to bloat the code while trying to cover every edge case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 _

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone, ReadabilityBracesAroundStatements and ReadabilityMisleadingIndentation

njames93 updated this revision to Diff 235570. njames93 retitled this revision from "[clang-tidy] Fix bug - 44229, 33203 and 32204" to "[clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone, ReadabilityBracesAroundStatements and ReadabilityMisl

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 updated this revision to Diff 235573. njames93 added a comment. Fixed a few more edge cases with test cases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 Files: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp cla

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

njames93 updated this revision to Diff 235645. njames93 retitled this revision from "[clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone, ReadabilityBracesAroundStatements and ReadabilityMisleadingIndentation" to "[clang-tidy] Disable Checks

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 updated this revision to Diff 235661. njames93 added a comment. Fixed a few of the test cases failing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 Files: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp clang-too

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

njames93 marked an inline comment as done. njames93 added a comment. In D71980#1799437 , @xazax.hun wrote: > Thanks for working on this, please see one of my concerns inline. > > If you are trying to fix this problem, alternatively, you could also fix it

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp:62 Finder->addMatcher( - ifStmt(stmt().bind("if"), + ifStmt(unless(allOf(isConstexpr(), isInTemplateInstantiation())), + stmt().bind("if"),

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

njames93 added a comment. I have a version with a new DiscaredStmt working, but it's currently lacking test cases, docs and it was pretty much made as a copy paste job from NullStmt so I'm not going to hurry up about trying to polish that one. The new lines and test cases patch I'll get done pr

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 updated this revision to Diff 235752. njames93 added a comment. hopefully adhered to all conventions :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 Files: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp clang-t

[PATCH] D71980: [clang-tidy] Disable Checks on If constexpr statements in template Instantiations for BugproneBranchClone and ReadabilityBracesAroundStatements

njames93 updated this revision to Diff 235755. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71980/new/ https://reviews.llvm.org/D71980 Files: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp clang-

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 added a comment. I'm in two minds about issuing a warning when scope restrictions prevent a fix. Do you think creating an option to enable or disable emitting warnings for cases where the scope prevents a fix would be a good idea? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 added a comment. In D71846#1800381 , @aaron.ballman wrote: > In D71846#1800344 , @njames93 wrote: > > > I'm in two minds about issuing a warning when scope restrictions prevent a > > fix. Do you think cre

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 added a comment. In D71846#1800411 , @aaron.ballman wrote: > In D71846#1800401 , @njames93 wrote: > > > In D71846#1800381 , @aaron.ballman > > wrote: > > > > > In

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 added a comment. In D71846#1800481 , @aaron.ballman wrote: > In D71846#1800480 , @njames93 wrote: > > > Definitely default to diagnosing everything, that's how my current > > implementation looks right no

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 updated this revision to Diff 235775. njames93 added a comment. Added option to disable warning when an automatic fix can't be applied due to scope restrictions of variables, default option is to show all warnings CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https:

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 marked an inline comment as done. njames93 added a comment. In D71846#1800503 , @aaron.ballman wrote: > The new option LGTM but one of the tests can be updated to have a less > complex RUN line. that was just put in when i was initially testin

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 updated this revision to Diff 235788. njames93 added a comment. removed the unnecessary explicit setting of WarnOnUnfixable value for test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 Files: clang-tools-extra/clang-tidy/readab

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 updated this revision to Diff 235791. njames93 added a comment. small reformat CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 Files: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp clang-tools-extra/clang-tidy/rea

[PATCH] D71846: [ASTMatchers] Fix for https://bugs.llvm.org/show_bug.cgi?id=44364

njames93 added a comment. Yes please, I'm still very new to the llvm contributing system so I would have no idea what to do next. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 ___ cfe-commits mail

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

njames93 created this revision. njames93 added a reviewer: aaron.ballman. njames93 added projects: clang, clang-tools-extra. Herald added a subscriber: xazax.hun. Fixes (clang-tidy) readability-identifier-naming misses fixing member variables in destructor

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

njames93 updated this revision to Diff 236012. njames93 added a comment. More edge cases handled to do with field decls and initializers CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72121/new/ https://reviews.llvm.org/D72121 Files: clang-tools-extra/clang-tidy/readability/Identifier

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

njames93 added a comment. Ahh clang format script must have run, I'll undo that. I've not tried running it against the whole of llvm as I don't know what the easy way to do that is, especially given that llvm disregards it's standard naming convention for old projects and stl like containers. T

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

njames93 updated this revision to Diff 236157. njames93 added a comment. addressed issues. Running the clang-tidy on the whole of lib clang just resulted in errors when compiling due to its dependancy on llvm, but I kind of knew that was going to happen CHANGES SINCE LAST ACTION https://revi

[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

njames93 added a comment. side note when creating this, the add_new_check.py file hasn't been updated in relation to this commit https://github.com/llvm/llvm-project/commit/d2c9c9157b0528436d3b9f5e22c872f0ee6509a2. This results in a malformed rst file. In this patch I have just manually set it

[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

njames93 created this revision. njames93 added a reviewer: aaron.ballman. njames93 added projects: clang, clang-tools-extra. Herald added subscribers: xazax.hun, whisperity, mgorny. njames93 added a comment. side note when creating this, the add_new_check.py file hasn't been updated in relation t

[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

njames93 added a comment. I'll address those issues tonight. As for the ** case, I'll. Hadn't even thought of that, I'll try and sort that out Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72217/new/ https://reviews.llvm.org/D72217

[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

njames93 updated this revision to Diff 236255. njames93 marked 13 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72217/new/ https://reviews.llvm.org/D72217 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/

[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

njames93 added a comment. fixed some of the code guidelines issues. Will tackle some of the more pressing issues like ensuring correctness CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72217/new/ https://reviews.llvm.org/D72217 ___ cfe-com

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp:137 +} +}; // namespace CtorInits aaron.ballman wrote: > I think this is one mor

[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

njames93 updated this revision to Diff 236486. njames93 added a comment. I have tried to make the code more robust against macro decls. Also target the actual type specifier rather than everything before the name when doing the replacements. This should leave any other qualifiers or attributes i

[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

njames93 marked 7 inline comments as done. njames93 added a comment. What do you think about volatile qualifiers. Personally I don't think its important to qualify an volatile on a pointer that is volatile, but i should adhere to the decl being declared as volatile volatile auto X = getPointe

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