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

2019-12-26 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 235384. njames93 edited the summary of this revision. njames93 added a comment. Think all the test cases are working, but they look like they could do with a clean up from someone who may know the clang-tidy-checks system better than myself CHANGES SINCE

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

2019-12-26 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Should also point out that clang-format now has a better time reformatting as i expanded the replacements range to include the entire else block rather than the start and end braces CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/

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

2019-12-24 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Thanks for the pointers, I'll amend those changes and update the patch, thanks for pointing it out for me. Probably be after boxing day though Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/

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

2019-12-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 235301. njames93 edited the summary of this revision. njames93 added a comment. fixed a spelling mistake in unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 Files:

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

2019-12-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 235300. njames93 marked an inline comment as done. njames93 added a comment. Renamed 'hasInitStorage' to 'hasInitStatement'. Added support for range for loops in 'hasInitStatement'. 'hasInitStatement' now has a match predicate on the init statement.

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

2019-12-29 Thread Nathan James via Phabricator via cfe-commits
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

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

2019-12-29 Thread Nathan James via Phabricator via cfe-commits
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

2019-12-30 Thread Nathan James via Phabricator via cfe-commits
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

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

2019-12-30 Thread Nathan James via Phabricator via cfe-commits
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

2019-12-30 Thread Nathan James via Phabricator via cfe-commits
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

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

2020-01-05 Thread Nathan James via Phabricator via cfe-commits
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

2020-01-04 Thread Nathan James via Phabricator via cfe-commits
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

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

2020-01-04 Thread Nathan James via Phabricator via cfe-commits
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

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

2020-01-05 Thread Nathan James via Phabricator via cfe-commits
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 ___

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

2020-01-05 Thread Nathan James via Phabricator via cfe-commits
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

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

2020-01-06 Thread Nathan James via Phabricator via cfe-commits
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

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

2020-01-06 Thread Nathan James via Phabricator via cfe-commits
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

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

2020-01-07 Thread Nathan James via Phabricator via cfe-commits
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 =

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

2020-01-07 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added a comment. Would you say this behaviour is better? void baz() { auto MyFunctionPtr = getPtrFunction(); // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto MyFunctionPtr' can be declared as 'auto *MyFunctionPtr' //

[PATCH] D72333: [clang-tidy] Disable match on `if constexpr` statements in template instantiation for `readability-misleading-indentation` check.

2020-01-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I could never seem to get the tests to handle differently in templated and non templated code. Maybe i was just being stupid. Think the root cause is that the check detects if the if column appears on a different column to either the else or the closing brace if its

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

2020-01-07 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 236615. njames93 added a comment. adhere to coding guidelines, added check for local volatile qualifiers, disregard pointer addition on function pointer types CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72217/new/

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

2020-01-07 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 236617. njames93 added a comment. Added docs for const and volatile qualifiers CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72217/new/ https://reviews.llvm.org/D72217 Files: clang-tools-extra/clang-tidy/readability/CMakeLists.txt

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

2019-12-28 Thread Nathan James via Phabricator via cfe-commits
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] D72217: [clang-tidy] Added readability-qualified-auto check

2020-01-08 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 236780. njames93 marked 3 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/llvm/LLVMTidyModule.cpp

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

2020-01-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D72217#1809212 , @Eugene.Zelenko wrote: > By the word, will be interesting to see results of this check run on LLVM > code. Probably results should be split on modules. So ran it on clang and clang-tidy just crashed, gonna

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

2020-01-07 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp:183 + + auto LambdaTest = [] { return 0; }; + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto LambdaTest' can

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

2020-01-07 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 236723. njames93 marked an inline comment as done. njames93 added a comment. Added an alias into the llvm module, LMK if I have done that incorrectly CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72217/new/ https://reviews.llvm.org/D72217 Files:

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

2020-01-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D72217#1809685 , @njames93 wrote: > In D72217#1809212 , @Eugene.Zelenko > wrote: > > > By the word, will be interesting to see results of this check run on LLVM > > code. Probably

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D75714#1913230 , @aaron.ballman wrote: > Is there any code we can cleanup as a result of adding these overloads? I > would have expected to see some code changes justifying each additional > overload, which would also give

[PATCH] D75841: [ASTMatchers] Add hasAnyOverloadedOperatorName matcher

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG77eec38626cb: [ASTMatchers] Add hasAnyOverloadedOperatorName matcher (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75841/new/

[PATCH] D76037: [clang] tooling replacements are escaped when exporting to YAML

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249875. njames93 added a comment. - Fix broken dependencies Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76037/new/ https://reviews.llvm.org/D76037 Files:

[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Test cases will follow, just time constrained for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76054/new/ https://reviews.llvm.org/D76054 ___ cfe-commits mailing list

[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2, AlexanderLanin. Herald added a project: clang. Herald added a subscriber: cfe-commits. njames93 added a comment. Test cases will follow, just time constrained for now. clang-apply-replacements currently

[PATCH] D76037: [clang] tooling replacements are escaped when exporting to YAML

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249860. njames93 added a comment. - Fix test case not being ran Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76037/new/ https://reviews.llvm.org/D76037 Files:

[PATCH] D76037: [clang] tooling replacements are escaped when exporting to YAML

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang/unittests/Tooling/ReplacementsYamlTest.cpp:68 + ASSERT_EQ(Doc.Replacements.size(), NewDoc.Replacements.size()); + if (Doc.Replacements.size() == NewDoc.Replacements.size()) { +for

[PATCH] D76037: [clang] tooling replacements are escaped when exporting to YAML

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249857. njames93 added a comment. - Small tidy of code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76037/new/ https://reviews.llvm.org/D76037 Files: clang/include/clang/Tooling/ReplacementsYaml.h

[PATCH] D76037: [clang] tooling replacements are escaped when exporting to YAML

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249856. njames93 added a comment. Herald added a subscriber: mgorny. - Moved escape/unescape impl to source file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76037/new/ https://reviews.llvm.org/D76037

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-11 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 For the record `check-clang-tools` is sufficient for testing all clang tidy checks CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75901/new/ https://reviews.llvm.org/D75901

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Please add a test case for this Comment at: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp:76-77 } else { +const ASTContext *ASTCtx = Result.Context; +const LangOptions = ASTCtx->getLangOpts(); static const

[PATCH] D75911: [clang-tidy] Added hasAnyListedName matcher

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2, alexfh, Eugene.Zelenko. Herald added subscribers: cfe-commits, kbarton, xazax.hun, mgorny, nemanjai. Herald added a project: clang. Adds a utils matcher called `hasAnyListedName` to alleviate all the hackery

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249307. njames93 added a comment. Herald added subscribers: arphaman, kbarton, nemanjai. - Add usages, removed optional on arrayrefs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75714/new/

[PATCH] D75569: [clang-tidy] New check for methods marked __attribute__((unavailable)) that do not override a method from a superclass.

2020-03-06 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Can objc instance methods be redeclared, the AST suggests so, if so you should probably only warm on the first occurrence but create fix its for each. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75569/new/

[PATCH] D75800: [ASTMatchers] adds isComparisonOperator to BinaryOperator and CXXOperatorCallExpr

2020-03-08 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfc3c80c38643: [ASTMatchers] adds isComparisonOperator to BinaryOperator and… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75800: [ASTMatchers] adds isComparisonOperator to BinaryOperator and CXXOperatorCallExpr

2020-03-07 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75800 Files:

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:97 + const auto CompareOperator = + expr(binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")), +

[PATCH] D75739: [clangd][vscode] Enable dot-to-arrow fixes in clangd completion.

2020-03-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. How does this handle cases where an object overloads the `->` operator or worse still overloads the operator and has conflicting names when accessed with `. ` or `->`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:74 +void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) { + const auto IntegerType = qualType(allOf(isInteger(), unless(isAnyCharacter()), +

[PATCH] D75800: [ASTMatchers] adds isComparisonOperator to BinaryOperator and CXXOperatorCallExpr

2020-03-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/misc/RedundantExpressionCheck.cpp:570 const auto RelationalExpr = ignoringParenImpCasts(binaryOperator( - isComparisonOperator(), expr().bind(Id), +

[PATCH] D75803: [clang-tidy] [NFC] Remove unnecessary matchers

2020-03-07 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. Herald added a project: clang. njames93 added a parent revision: D75800: [ASTMatchers] adds isComparisonOperator to BinaryOperator and CXXOperatorCallExpr. Herald added a subscriber: wuzish.

[PATCH] D75803: [clang-tidy] [NFC] Remove unnecessary matchers

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG223a43ee8d89: [clang-tidy] [NFC] Remove unnecessary matchers (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D75803?vs=248918=249040#toc Repository: rG LLVM Github Monorepo

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:44-46 bool parseFileExtensions(StringRef AllFileExtensions, - FileExtensionsSet , char Delimiter); + FileExtensionsSet , +

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. LGTM Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h:44-46 bool parseFileExtensions(StringRef AllFileExtensions, - FileExtensionsSet , char Delimiter); +

[PATCH] D75749: [clang-tidy] extend bugprone-signed-char-misuse check.

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM However, how does this handle cases when the type is written as `char`. They can be signed/unsigned based on what platform is being targeted. But on a platform where `char` is

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D75714#1912326 , @lebedev.ri wrote: > What's the use case here? Mainly syntactic sugar, If you have a function that maybe returns a `FixItHint` or `SourceRange` you can pass it directly to the `DiagnosticBuilder`.

[PATCH] D75841: [ASTMatchers] Add hasAnyOverloadedOperatorName matcher

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added a reviewer: aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75841 Files: clang/docs/LibASTMatchersReference.html

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-09 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang/include/clang/Basic/Diagnostic.h:1318 + const llvm::Optional ) { + if (Opt) { +DB << *Opt; Should this be disabled on

[PATCH] D76037: [clang] tooling replacements are escaped when exporting to YAML

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/Tooling/ReplacementsYaml.cpp:22 +static constexpr Escapes EscapeChars[] = { +{'\n', 'n'}, {'\r', 'r'}, {'\t', 't'}, {'\\', '\\'}}; + AlexanderLanin wrote: > Just so I have asked ;-) > Escaping every \

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D75901#1916119 , @tetsuo-cpp wrote: > @njames93 @MaskRay > Thanks for helping me with testing. I'll remember this for next time. > > I also saw this pattern of retrieving `LangOptions` from an `ASTContext` in > some other

[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 250054. njames93 added a comment. - Added test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76054/new/ https://reviews.llvm.org/D76054 Files:

[PATCH] D76037: [clang] tooling replacements are escaped when exporting to YAML

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 250038. njames93 added a comment. - Fix another broken dependency Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76037/new/ https://reviews.llvm.org/D76037 Files:

[PATCH] D76037: [clang] tooling replacements are escaped when exporting to YAML

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 250053. njames93 marked 6 inline comments as done. njames93 added a comment. - Extend tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76037/new/ https://reviews.llvm.org/D76037 Files:

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

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D75184#1914705 , @DmitryPolukhin wrote: > You are absolutely right about current behaviour. Thank you for catching this > odd behaviour. I'm not 100% confident what is the right behaviour but my > guess is that overriding

[PATCH] D76083: [clang-tidy] Expand the list of functions in bugprone-unused-return-value

2020-03-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Please upload diffs with full context. If you are using `git diff` to generate the patch, pass the flag `-U99` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76083/new/ https://reviews.llvm.org/D76083

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249361. njames93 marked 2 inline comments as done. njames93 added a comment. - remove unnecessary semi colons Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75714/new/ https://reviews.llvm.org/D75714 Files:

[PATCH] D75911: [clang-tidy] Added hasAnyListedName matcher

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 249356. njames93 added a comment. - Fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75911/new/ https://reviews.llvm.org/D75911 Files:

[PATCH] D75911: [clang-tidy] Added hasAnyListedName matcher

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/Matchers.cpp:17 + +Matcher hasAnyListedName(std::vector Names) { + return Matcher(new HasNameMatcher(std::move(Names))); gribozavr2 wrote: > This matcher sounds generally useful. I

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG66945b62f42f: Add Optional overload to DiagnosticBuilder operator (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75714/new/

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. If you need help for the test case. Create a file in the `clang-tools-extra/test/clang-tidy/checkers`. Name it something like `misc-unconventional-assign-operator-pre11.cpp` and paste this in // RUN: %check_clang_tidy -std=c++98,c++03 %s

[PATCH] D75569: [clang-tidy] New check for methods marked __attribute__((unavailable)) that do not override a method from a superclass.

2020-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp:34 +// Matches Objective-C methods that are not overriding a superclass method. +AST_MATCHER(ObjCMethodDecl, isNotOverriding) { return !Node.isOverriding(); } +

[PATCH] D75911: [clang-tidy] Added hasAnyListedName matcher

2020-03-10 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/utils/Matchers.cpp:17 + +Matcher hasAnyListedName(std::vector Names) { + return Matcher(new HasNameMatcher(std::move(Names))); gribozavr2 wrote:

[PATCH] D76037: [clang] tooling replacements are escaped when exporting to YAML

2020-03-11 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, gribozavr2, AlexanderLanin. Herald added a project: clang. Herald added a subscriber: cfe-commits. Escapes replacement text when exporting to yaml and unescapes when importing from yaml. Repository: rG LLVM Github

[PATCH] D77831: [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, gribozavr2. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. Herald added a project: clang. This was done with a script that looks for calls to Options.get(GlobalOrLocal) that take an integer for

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

2020-04-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D75184#1984565 , @DmitryPolukhin wrote: > I didn't notice the issue because of massive breakage with diff landed just > be bore mine Sorry about that :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D78139: [clang-tidy] modernize-use-using: Fix broken fixit with 'template' keyword

2020-04-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/AST/NestedNameSpecifier.cpp:314 +InnerPolicy); +} else if (const auto *SpecType = + dyn_cast(T)) { Can this be renamed as it shadows the `SpecType`

[PATCH] D77534: [clangd] DefineOutline: removes static token from static CXXMethodDecl

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG353a9883680e: [clangd] DefineOutline: removes static token from static CXXMethodDecl (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfcf7cc268fe4: [clang-tidy] Added support for validating configuration options (authored by njames93). Changed prior to commit: https://reviews.llvm.org/D77085?vs=255023=255764#toc Repository: rG

[PATCH] D76606: [clang-tidy] Change checks that take enum configurations to use a new access method.

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9dff9ecdd113: [clang-tidy] Change checks that take enum configurations to use a new access… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D77085#1967807 , @nemanjai wrote: > A recent commit has taken down a whole bunch of bots. The build error > messages all seem to point to code in this patch. If this is indeed the > cause, please revert. I was aware and

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

2020-04-07 Thread Nathan James via Phabricator via cfe-commits
njames93 added a subscriber: jdoerfert. njames93 added a comment. In D77085#1967883 , @nemanjai wrote: > Awesome, thanks. Certainly fixes the compile time failures in my local build. > There is still a link-time failure (undefined reference) with my

[PATCH] D77499: [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:16-17 +if (Item.equals_lower(Search)) { + if (Item.equals(Search)) +return Item.str(); + MaxEditDistance = 1; aaron.ballman wrote: > Is this case

[PATCH] D77499: [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 255387. njames93 marked 4 inline comments as done. njames93 added a comment. - Added test cases and addressed nits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77499/new/ https://reviews.llvm.org/D77499

[PATCH] D77499: [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG427c1dc4f424: [ASTMatchers] Matchers that take enumerations args provide hints with invalid… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77503: [ASTMatchers] Fixed CastKind being parsed incorrectly for dynamic matchers

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 255440. njames93 added a comment. - Added support for giving suggestions on invalid inputs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77503/new/ https://reviews.llvm.org/D77503 Files:

[PATCH] D77534: [clangd] DefineOutline: removes static token from static CXXMethodDecl

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 marked 9 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:294 + if (isa(FD) && cast(FD)->isStatic()) +DeleteKeyword(tok::kw_static, {FD->getBeginLoc(), FD->getLocation()});

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

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. As no FixIts are made the `IncludeInserter` can be removed. I'm struggling to see the value of this check though. If it was reworked to check for loop in the middle of a function it would have a lot more value. bool all_of = true; for (auto X : V) { if (!X) {

[PATCH] D77534: [clangd] DefineOutline: removes static token from static CXXMethodDecl

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 255472. njames93 marked an inline comment as done. njames93 added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77534/new/ https://reviews.llvm.org/D77534 Files:

[PATCH] D77534: [clangd] DefineOutline: removes static token from static CXXMethodDecl

2020-04-06 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 255473. njames93 added a comment. - Fix issue with last revision Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77534/new/ https://reviews.llvm.org/D77534 Files:

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

2020-04-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D77572#1967956 , @mgehre wrote: > In D77572#1965143 , @njames93 wrote: > > > I'm struggling to see the value of this check though. If it was reworked to > > check for loop in the

[PATCH] D77680: [clang-tidy] misc-unused-parameters: Don't remove parameter from lambda

2020-04-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Outright disabling for lambdas probably isn't the ideal solution, however it's at least a good stepping stone til a better solution can be found. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77680/new/

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

2020-04-13 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 257151. njames93 added a comment. Clean up code and remove unrelated changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73052/new/ https://reviews.llvm.org/D73052 Files:

[PATCH] D77831: [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

2020-04-12 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 256842. njames93 added a comment. Added a few more cases by hand Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77831/new/ https://reviews.llvm.org/D77831 Files:

[PATCH] D76496: [clang-tidy] StringFindStartswith should ignore 3-arg find()

2020-04-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D76496#1949851 , @niko wrote: > Correct me if I'm wrong, but that seems to be in violation of IWYU? Maybe I'm > misreading this, or is the idea that higher-lever tooling (e.g. IWYU fixer) > is supposed to address that? It

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

2020-04-10 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Right now it would be a great candidate for this - http://lists.llvm.org/pipermail/cfe-dev/2020-March/064980.html, however in its current state I wouldn't say its ready to get the green light right now. No point worrying about the fix-its yet, but when it is time,

[PATCH] D77791: [ASTMatchers] Add support for dynamic matching of ofKind narrowing matcher

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:102-106 + static constexpr llvm::StringRef Allowed[] = { + "UETT_SizeOf", "UETT_AlignOf", + "UETT_VecStep", "UETT_OpenMPRequiredSimdAlign", +

[PATCH] D77791: [ASTMatchers] Add support for dynamic matching of ofKind narrowing matcher

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 256262. njames93 marked 4 inline comments as done. njames93 added a comment. Added test cases and address nits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77791/new/ https://reviews.llvm.org/D77791 Files:

[PATCH] D77503: [ASTMatchers] Fixed CastKind being parsed incorrectly for dynamic matchers

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 256237. njames93 added a comment. Fixed up documentation for CastKind matcher. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77503/new/ https://reviews.llvm.org/D77503 Files:

[PATCH] D77791: [ASTMatchers] Add support for dynamic matching of ofKind narrowing matcher

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: klimek, aaron.ballman. Herald added a reviewer: jdoerfert. Herald added a project: clang. Herald added a subscriber: cfe-commits. Adds support for using the ofKind in clang-query and other dynamic matcher use cases Repository: rG LLVM

[PATCH] D77831: [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

2020-04-12 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG672207c319a0: [clang-tidy] Convert config options that are bools to use the bool overload of… (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77791: [ASTMatchers] Add support for dynamic matching of ofKind narrowing matcher

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:102-106 + static constexpr llvm::StringRef Allowed[] = { + "UETT_SizeOf", "UETT_AlignOf", + "UETT_VecStep",

[PATCH] D77503: [ASTMatchers] Fixed CastKind being parsed incorrectly for dynamic matchers

2020-04-09 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 256275. njames93 added a comment. Added unit test for matcher Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77503/new/ https://reviews.llvm.org/D77503 Files: clang/docs/LibASTMatchersReference.html

<    1   2   3   4   5   6   7   8   9   >