[PATCH] D81917: [clang-tidy] For `run-clang-tidy.py` escape the paths that are used for analysis.

2020-06-28 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. Looks good Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81917/new/ https://reviews.llvm.org/D81917

[PATCH] D82626: [CodeComplete] Tweak completion for else.

2020-06-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I'm still unsure what is the best behaviour here. Would suggesting both patterns, but sort them based on what the then branch uses be best Example with: if (...) { // Statements } Suggestions: - else { // Statements } - else if (...) { // Statements } -

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

2020-06-28 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: klimek, aaron.ballman. Herald added subscribers: llvm-commits, cfe-commits, hiraditya. Herald added projects: clang, LLVM. Added new Macros `AST(_POLYMORPHIC)_MATCHER_REGEX(_OVERLOAD)` that define a matchers that take a regular expression

[PATCH] D82707: [clang][docs] Remove untracked files from formatted status

2020-06-28 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: krasimir, JakeMerdichAMD, sammccall, curdeius, bollu, alexshap, jdoerfert, DavidTruby, sscalpone, MyDeveloperDay. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently on

[PATCH] D82661: [clang-tidy] Remove unnecessary includes throughout clang-tidy header files

2020-06-26 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh. Herald added subscribers: cfe-commits, lebedev.ri, arphaman, dexonsmith, steven_wu, kbarton, hiraditya, xazax.hun, nemanjai. Herald added a reviewer: lebedev.ri. Herald added a project: clang. Plus replacing a few

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

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

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

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

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

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

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

2020-06-16 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: sammccall, kadircet, hokein. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Adds a command line flag `clang-tidy-config` for specifing configuration of checks,

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

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

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

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

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

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

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

2020-06-07 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:425 Finder->addMatcher( - ifStmt(isExpansionInMainFile(),

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

2020-06-07 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:425 Finder->addMatcher( - ifStmt(isExpansionInMainFile(),

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

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

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

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

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

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

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

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

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

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

[PATCH] D81075: [ASTMatchers] Fix crash with HasName and HasAnyName

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 268142. njames93 added a comment. Fix clang-format error. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81075/new/ https://reviews.llvm.org/D81075 Files: clang/include/clang/AST/PrettyPrinter.h

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

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 268116. njames93 added a comment. Included more builtins. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80887/new/ https://reviews.llvm.org/D80887 Files:

[PATCH] D80877: [clang-tidy] Added MacroDefiniton docs for readability-identifier-naming

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG65fa0a9f7f3e: [clang-tidy] Added MacroDefiniton docs for readability-identifier-naming (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80536: [clang-tidy][modernize-loop-convert] Make loop var type human readable

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80536/new/ https://reviews.llvm.org/D80536

[PATCH] D81075: [ASTMatchers] Fix crash with HasName and HasAnyName

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: klimek, sbenza. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes crashes caused by matching fully qualified RecordDecls inside Function bodies. See https://bugs.llvm.org/show_bug.cgi?id=43639. I'm still unsure

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

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. LGTM, but I wouldn't say no to a test case using `(Limit) -= 1`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79912/new/ https://reviews.llvm.org/D79912 ___ cfe-commits mailing

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:40 +// "__builtin_va_start", +// "__builtin_stdarg_start", +"__builtin_assume_aligned", // Documented as variadic to support overloading

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D80514#2070843 , @bernhardmgruber wrote: > Ping. > > Furthermore: how can I schedule another CI build with the newest version of > the diff? When I go to the failed build from Harbormaster and I click > restart, it rebuilds

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

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 268149. njames93 marked 3 inline comments as done. njames93 added a comment. Add va_start. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80887/new/ https://reviews.llvm.org/D80887 Files:

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

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

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

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:39 +"__builtin_classify_type", +// "__builtin_va_start", +// "__builtin_stdarg_start",

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

2020-06-03 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D80514#2071056 , @bernhardmgruber wrote: > Reuploaded diff in an attempt to trigger a CI build. It's not working, Still no big issue, when its ready to land just the checks locally CHANGES SINCE LAST ACTION

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

2020-06-04 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. cheers LGTM, probably didn't need a review this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81180/new/

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

2020-07-16 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 278398. njames93 added a comment. Updated release notes for version bump. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82089/new/ https://reviews.llvm.org/D82089 Files:

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D83717#2152762 , @gamesh411 wrote: > In D83717#2150977 , @njames93 wrote: > > > Alternatively you could do something like this, though it would be a pain > >

[PATCH] D83680: [clang-tidy] Refactor IncludeInserter

2020-07-17 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 278807. njames93 marked 5 inline comments as done. njames93 added a comment. Address reviewer comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83680/new/ https://reviews.llvm.org/D83680 Files:

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

2020-07-17 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D83717#2155103 , @gamesh411 wrote: > My thoughts exactly! I also thought about anchor-points as a feature in > file-check, as that would immensely increase the readability of the test-code > in such cases. I've put in an

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

2020-07-28 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281194. njames93 added a comment. Fix new lines in test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82898/new/ https://reviews.llvm.org/D82898 Files:

[PATCH] D84453: [clang-tidy] Suppress one unittest on macOS.

2020-07-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D84453#2177665 , @NoQ wrote: > In D84453#2171548 , @njames93 wrote: > >> I was having issues with this test case under macos in D82188 >> . >> It would

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

2020-07-28 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281200. njames93 added a comment. Replace if/else logic with switches Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82898/new/ https://reviews.llvm.org/D82898 Files:

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

2020-07-28 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281418. njames93 marked 2 inline comments as done. njames93 added a comment. Remove unreachable after switch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82898/new/ https://reviews.llvm.org/D82898 Files:

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

2020-07-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:629 Priority = std::make_tuple(Begin, Type, -End, -ErrorSize, ErrorId); - else +return; + case ET_Insert: aaron.ballman wrote: >

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

2020-07-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:116-121 + unsigned Priority = 0; for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(), E =

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

2020-07-28 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281439. njames93 added a comment. Add missing new lines in test files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84814/new/ https://reviews.llvm.org/D84814 Files:

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

2020-07-28 Thread Nathan James via Phabricator via cfe-commits
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. Herald added a subscriber: aheejin. When checking for the style of a decl that

[PATCH] D84812: [clang-tidy][NFC] Added convienence methods for getting optional options

2020-07-28 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Herald added a subscriber: aheejin. These methods abstract away Error handling when

[PATCH] D84812: [clang-tidy][NFC] Added convienence methods for getting optional options

2020-07-28 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281433. njames93 added a comment. Fix wrong patch diff Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84812/new/ https://reviews.llvm.org/D84812 Files: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281494. njames93 added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84814/new/ https://reviews.llvm.org/D84814 Files:

[PATCH] D84831: [clang-tidy] Fix RedundantStringCStrCheck with r values

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
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. The previous fix for this, https://reviews.llvm.org/D76761, Passed test cases

[PATCH] D84812: [clang-tidy][NFC] Added convienence methods for getting optional options

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:224 + else +consumeError(ValueOr.takeError()); + return llvm::None; gribozavr2 wrote: > Is this specialization defined

[PATCH] D84812: [clang-tidy][NFC] Added convienence methods for getting optional options

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281492. njames93 added a comment. Rename logOptionParsingError Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84812/new/ https://reviews.llvm.org/D84812 Files:

[PATCH] D84831: [clang-tidy] Fix RedundantStringCStrCheck with r values

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb99630e43261: [clang-tidy] Fix RedundantStringCStrCheck with r values (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84831/new/

[PATCH] D84868: [clang-tidy] Use StringMap for ClangTidyOptions::OptionsMap

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Ordering of options isn't important so an `llvm::StringMap` is a much better container

[PATCH] D84868: [clang-tidy] Use StringMap for ClangTidyOptions::OptionsMap

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 281639. njames93 added a comment. Missing call to getValue() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84868/new/ https://reviews.llvm.org/D84868 Files: clang-tools-extra/clang-tidy/ClangTidy.cpp

[PATCH] D84850: [clang-tidy] Fix module options being registered with different priorities

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added a reviewer: DmitryPolukhin. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. njames93 requested review of this revision. Not a bug that is ever likely to materialise, but still worth fixing Repository: rG LLVM

[PATCH] D84850: [clang-tidy] Fix module options being registered with different priorities

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG62beb7c6f4f2: [clang-tidy] Fix module options being registered with different priorities (authored by njames93). Repository: rG LLVM Github

[PATCH] D84831: [clang-tidy] Fix RedundantStringCStrCheck with r values

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D84831#2181336 , @gribozavr2 wrote: >> Passed test cases but failed in the real world as std::string has a non >> trivial destructor so creates a CXXBindTemporaryExpr. > > An idea for a future change: move the std::string

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

2020-07-29 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbbc2ddecbd34: [clang-tidy] Handled insertion only fixits when determining conflicts. (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D83680: [clang-tidy] Refactor IncludeInserter

2020-07-27 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG13c9bbc28ef9: [clang-tidy] Refactor IncludeInserter (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83680/new/

[PATCH] D84453: [clang-tidy] Suppress one unittest under ASan.

2020-07-24 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I was having issues with this test case under macos in D82188 . It would fail for seemingly no apparent reason until I disable a test in a different translation unit. This made me think there is a subtle bug in the linker used on macos.

[PATCH] D83759: [clangd] Port lit tests to Windows

2020-07-21 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D83759#2163622 , @ArcsinX wrote: > What do you think of this patch? I'm not sure if Windows is important OS for > developers. Windows is most certainly an important OS for developers and something the whole llvm project

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

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:109 SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes), -TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) { +

[PATCH] D84926: [clang-tidy][NFC] Use StringMap for ClangTidyCheckFactories::FacoryMap

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGc23ae3f18ee3: [clang-tidy][NFC] Use StringMap for ClangTidyCheckFactories::FacoryMap (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D66564: [clang-tidy] new altera struct pack align check

2020-07-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D66564#2185335 , @ffrankies wrote: > Is there a way around this? And if not, do we ignore it? Lastly, do I need to > do anything else since this check has been accepted? There is a "Close > Revision" action in the comments,

[PATCH] D86176: [clang-tidy] readability-simplify-boolean-expr detects negated literals

2020-08-18 Thread Nathan James via Phabricator via cfe-commits
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. Adds support for detecting cases like `if (!true) ...`. Addresses

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

2020-08-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Can you make sure you upload diffs with full context (-U9). Or using arcanist it will be done automatically. Make sure the diff is up to date with trunk Remove any changes that aren't related to this patch, they just make this look noisy. Repository: rCTE

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

2020-08-28 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:428 case IdentifierNamingCheck::CT_HungarianNotation: { const NamedDecl *pNamedDecl = dyn_cast(pDecl); aaron.ballman wrote: > I feel like I

[PATCH] D86176: [clang-tidy] readability-simplify-boolean-expr detects negated literals

2020-08-22 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGdf5335a36d3d: [clang-tidy] readability-simplify-boolean-expr detects negated literals (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86176: [clang-tidy] readability-simplify-boolean-expr detects negated literals

2020-08-21 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:66 + if (const auto *Negated = Result.Nodes.getNodeAs(Id)) { +if (Negated->getOpcode() == UO_LNot && +isa(Negated->getSubExpr()))

[PATCH] D86176: [clang-tidy] readability-simplify-boolean-expr detects negated literals

2020-08-20 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:66 + if (const auto *Negated = Result.Nodes.getNodeAs(Id)) { +if (Negated->getOpcode() == UO_LNot && +isa(Negated->getSubExpr()))

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

2020-08-30 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. As Hungarian notation enforces prefixes as well as casing styles it would be wise to warn on cases where the style is Hungarian but a prefix has also been set. Another issue is this current set up will let you define any decls style as hungarian, this doesn't make

[PATCH] D87683: [clang-tidy] Crash fix for bugprone-misplaced-pointer-arithmetic-in-alloc

2020-09-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Please fix the test case first, can't call `operator new(unsigned long, void*)` with an argument of type `void*` The other failures the pre merge bot detected can safely be disregarded Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87627: [clang-tidy] Fix crash in modernize-use-noexcept on uninstantiated throw class

2020-09-14 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Please fix the clang-format fail too. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-use-noexcept-opt.cpp:23 +// Shouldn't crash due to llvm_unreachable in canThrow() on EST_Uninstantiated +template class c { void *operator

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

2020-09-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D86671#2259443 , @dougpuob wrote: > In D86671#2259364 , @njames93 wrote: > >> Did you upload this incorrectly again, context is missing and seems to be a >> relative diff from a

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

2020-09-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Did you upload this incorrectly again, context is missing and seems to be a relative diff from a previous version of this patch? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86671/new/ https://reviews.llvm.org/D86671

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

2020-09-25 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: sammccall, kadircet, hokein. Herald added subscribers: cfe-commits, usaxena95, arphaman. Herald added a project: clang. njames93 requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Extend the Trivial setter

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

2020-09-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 294357. njames93 marked 2 inline comments as done. njames93 added a comment. Updated function doc Fix potential assertion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88297/new/

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

2020-09-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:461 + if (auto *CE = llvm::dyn_cast(RHS->IgnoreCasts())) { +// Make sure we get the version of move with 1 arg, the other is for moving +// ranges. sammccall wrote: > nit:

<    3   4   5   6   7   8   9   >