[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc created this revision. poelmanc added reviewers: alexfh, alexfh_. poelmanc added a project: clang-tools-extra. Herald added a project: clang. Herald added a subscriber: cfe-commits. clang-tidy's modernize-use-using feature is great! But if it finds any commas that are not within parenthe

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc updated this revision to Diff 219834. poelmanc edited the summary of this revision. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67460/new/ https://reviews.llvm.org/D67460 Files: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc added a comment. Wow, thanks for the super-quick testing, feedback and a new test case! I added a slightly enhanced version of your test case to the modernize-use-using.cpp test file and got it passing by treating tok::less and tok::right as AngleBrackets //only when ParenLevel == 0//.

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc updated this revision to Diff 219957. poelmanc added a comment. Thanks for the stressing test cases. I reverted to your original test case with a single >, added a separate one with a single <, and expanded the final case to have a non-balanced number of > and <. I added all your new ca

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc updated this revision to Diff 220002. poelmanc added a comment. Nice catch, that simplifies the code quite a bit! I added two more nested, complex multiple-template-argument tests and am happy to add more. (We could do a case fallthrough after tok::l_brace, though some linters warn abo

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc updated this revision to Diff 220007. poelmanc added a comment. Sorry one more test at the end to make sure a multi-typedef with all that Variadic stuff still doesn't get changed to using. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67460

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc added a comment. Herald added a subscriber: mgehre. Hi @alexfh, @jonathanmeier has reviewed my pull request but lacks commit access. It changes ~30 lines of code isolated to modernize-use-using.cpp and adds ~60 lines of tests. If you have time I'd greatly appreciate it if you could pro

[PATCH] D68682: Clang-tidy fixes should avoid creating new blank lines

poelmanc created this revision. poelmanc added reviewers: jonathanmeier, alexfh. poelmanc added a project: clang-tools-extra. Herald added subscribers: cfe-commits, mgehre, ormris. Herald added a project: clang. Clang-tidy can ignore most formatting concerns as formatting is clang-format's job. H

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc updated this revision to Diff 224089. poelmanc retitled this revision from "Clang-tidy fixes should avoid creating new blank lines" to "Clang-tidy fix removals removing all non-blank text from a line should remove the line". poelmanc edited the summary of this revision. poelmanc added a

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc added a comment. In D68682#1700908 , @Eugene.Zelenko wrote: > You may be interested to also look on PR43583 related to > readability-redundant-member-init. Thanks Eugene, I'm having trouble finding that. https://reviews.llvm.org/D43583 seems

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc marked an inline comment as done. poelmanc added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:27 #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/CharInfo.h" // for isWhiteSpace(char), isVerticalWhitespace(char) #include

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc added a comment. In D68682#1705865 , @Eugene.Zelenko wrote: > In D68682#1705505 , @jonathanmeier > wrote: > > > In D68682#1702025 , @poelmanc > > wrote: > > > > >

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc added a comment. In D68682#1705866 , @gribozavr wrote: > > I guess here's the high-level question: should all removals that remove all > > non-blank text from a line also delete the line? > > I see your point, but as https://llvm.org/PR43583 show

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc updated this revision to Diff 225523. poelmanc edited the summary of this revision. poelmanc added a comment. In D68682#1707764 , @alexfh wrote: > cleanupAroundReplacements is not used by clang-format itself. I believe, it's > a part of clang-for

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

poelmanc created this revision. poelmanc added reviewers: malcolm.parsons, alexfh, hokein, aaron.ballman. poelmanc added a project: clang-tools-extra. Herald added subscribers: cfe-commits, mgehre. Herald added a project: clang. readability-redundant-member-init removes redundant / unnecessary mem

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

poelmanc added a comment. In D69145#1715611 , @mgehre wrote: > Exactly due to the issue you are fixing here, we ended up disabling the > complete check because we didn't want to live with the warnings it produced > on -Wextra. > Therefore, I'm actually

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

poelmanc updated this revision to Diff 225764. poelmanc added a comment. Addressed @mgehre's feedback: - Ran clang-format - Included gcc, -Wextra, -Werror=extra, and the full text of the warning/error message to aid in searchability, so people can find this option when they encounter the proble

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

poelmanc added a comment. In D69145#1715614 , @lebedev.ri wrote: > So https://godbolt.org/z/qzjU-C > This feels like gcc being overly zealous, i'm not sure what it says with > that warning. I'd agree, while copy constructors //usually// need to copy ba

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc created this revision. poelmanc added reviewers: gribozavr, etienneb, alexfh. Herald added subscribers: cfe-commits, mgehre, dylanmckay. Herald added a project: clang. `readability-redundant-string-init` was one of several clang-tidy checks documented as failing for C++17 by @gribozavr's

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc updated this revision to Diff 225894. poelmanc added a comment. Removed the two uses of auto where the type was not an iterator or clear from the right-hand-side. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69238/new/ https://reviews.llv

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc marked 2 inline comments as done. poelmanc added a comment. Thanks for the quick feedback, fixed. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69238/new/ https://reviews.llvm.org/D69238 ___ cf

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc updated this revision to Diff 225896. poelmanc added a comment. Rebase to latest master (tests moved into new "checkers" subdirectory.) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67460/new/ https://reviews.llvm.org/D67460 Files: clang

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc marked an inline comment as done. poelmanc added a comment. Checked "Done". (I addressed @jonathanmeier's comment feedback with a previous update but forgot to check the box!) I welcome any more feedback. Thanks. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https:

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

poelmanc marked 2 inline comments as done. poelmanc added a comment. Addressed these the other day but failed to check the "Done" boxes. Done! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69145/new/ https://reviews.llvm.org/D69145 __

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

poelmanc updated this revision to Diff 225897. poelmanc added a comment. Rebase to latest master (tests moved into new "checkers" directory.) Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69145/new/ https://reviews.llvm.org/D69145 Files: clang-to

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

poelmanc added a comment. What do @malcolm.parsons, @alexfh, @hokein, @aaron.ballman, @lebedev.ri think of @mgehre's suggestion to enable `IgnoreBaseInCopyConstructors` as the default setting, so gcc users won't experience build errors and think "clang-tidy broke my code!" I could go either wa

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc updated this revision to Diff 226695. poelmanc added a comment. Remove extra `const`, braces for one-line `if` statements, and `clang::`. Added a comment explaining the need for a regex on a warning test line. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://re

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc marked 5 inline comments as done. poelmanc added a comment. Thanks for the feedback, the new patch removes the extra `const`, `clang::`, and braces on one-line `if` statements, and now explains the regex in readability-redundant-string-init.cpp. Comment at: clang-to

[PATCH] D69548: Give clang-tidy readability-redundant-string-init a customizable list of string types to fix

poelmanc created this revision. poelmanc added a reviewer: MyDeveloperDay. poelmanc added a project: clang-tools-extra. Herald added subscribers: cfe-commits, mgehre. Herald added a project: clang. This patch adds a feature requested by @MyDeveloperDay at https://reviews.llvm.org/D69238, @MyDevel

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc marked 3 inline comments as done. poelmanc added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:50 varDecl(hasType(hasUnqualifiedDesugaredType(recordType( hasDeclaration(cxxRecordDecl

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

poelmanc updated this revision to Diff 226847. poelmanc added a comment. Changed default to 1, updated help accordingly, and removed an extraneous set of braces around a one-line statement. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69145/new/ h

[PATCH] D97625: fix check-clang-tools tests that fail due to Windows CRLF line endings

poelmanc added a comment. Sorry I somehow missed the acceptance back on 10 Jan. I lack commit access, it would be great if you could push for me, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97625/new/ https://reviews.llvm.org/D97625 ___

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

poelmanc created this revision. poelmanc added reviewers: aaron.ballman, angelgarcia, hokein. poelmanc added a project: clang-tools-extra. Herald added a subscriber: yaxunl. poelmanc requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. `clang-ti

[PATCH] D95725: clang-extra: fix incorrect use of std::lock_guard by adding variable name (identified by MSVC [[nodiscard]] error)

poelmanc created this revision. poelmanc added reviewers: sammccall, ilya-biryukov. poelmanc added a project: clang-tools-extra. Herald added subscribers: usaxena95, kadircet, arphaman. poelmanc requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits

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

poelmanc created this revision. poelmanc added reviewers: aaron.ballman, angelgarcia, hokein. poelmanc added a project: clang-tools-extra. Herald added a subscriber: yaxunl. poelmanc requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As in htt

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

poelmanc updated this revision to Diff 320284. poelmanc added a comment. Fix test failure in `modernize-use-nullptr-cxx20.cpp` by replacing `#include ` with some minimal equivalent `std` code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95726/new/ https://reviews.llvm.org/D95726 Fil

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

poelmanc added a comment. Fixed test failure in `modernize-use-nullptr-cxx20.cpp` by replacing `#include ` with some minimal equivalent `std` code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95726/new/ https://reviews.llvm.org/D95726 ___

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

poelmanc added a comment. Fix test failure in `modernize-use-nullptr-cxx20.cpp` by replacing `#include ` with some minimal equivalent `std` code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95726/new/ https://reviews.llvm.org/D95726 ___ cf

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

poelmanc updated this revision to Diff 320286. poelmanc added a comment. Fix test failure in `modernize-use-nullptr-cxx20.cpp` by replacing `#include ` with some minimal equivalent `std` code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95714/new/ https://reviews.llvm.org/D95714 Fil

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

poelmanc added a comment. Thanks for the quick feedback everyone. I agree too, withdrawing this in favor of https://reviews.llvm.org/D95714. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95726/new/ https://reviews.llvm.org/D95726

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

poelmanc updated this revision to Diff 320330. poelmanc added a comment. Thanks to the great feedback, changed `unless(hasAncestor(cxxRewrittenBinaryOperator()))` to `unless(hasParent(expr(hasParent(cxxRewrittenBinaryOperator()` and added a test to verify the improvement (and removed an ext

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

poelmanc added a comment. @njames93 Thank you so much for the quick feedback, I made your suggested changes and added a test that it properly converts `result = (a1 > (ptr == 0 ? a1 : a2));` to `result = (a1 > (ptr == nullptr ? a1 : a2));` now. In these examples so far, checking the grandparent

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

poelmanc added a comment. In D95714#2532565 , @njames93 wrote: > This does highlight an issue where the mimicked std library stubs used for > tests don't correspond exactly to what the stdlib actually looks like and can > result in subtly different ASTs

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

poelmanc updated this revision to Diff 320393. poelmanc added a comment. Thanks @steveire, that suggestion worked perfectly! I added the additional test case and shortened the mimicked `strong_ordering` code to a version from `clang/unittests.ASTMatchers/ASTMatchersTraversalTest.cpp`, but also m

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

poelmanc created this revision. poelmanc added reviewers: sammccall, hokein. poelmanc added a project: clang-tools-extra. Herald added a subscriber: xazax.hun. poelmanc requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. `modernize-loop-convert

[PATCH] D95725: clang-extra: fix incorrect use of std::lock_guard by adding variable name (identified by MSVC [[nodiscard]] error)

poelmanc updated this revision to Diff 320594. poelmanc added a comment. Change Guard to Lock. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95725/new/ https://reviews.llvm.org/D95725 Files: clang-tools-extra/clangd/support/Function.h Index: clang-tools-extra/clangd/support/Functio

[PATCH] D95725: clang-extra: fix incorrect use of std::lock_guard by adding variable name (identified by MSVC [[nodiscard]] error)

poelmanc marked an inline comment as done. poelmanc added a comment. s/Guard/Lock/! I don't have commit access so appreciate a push. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95725/new/ https://reviews.llvm.org/D95725 ___ cfe-commits mail

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

poelmanc updated this revision to Diff 320637. poelmanc added a comment. Add period to end of comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95714/new/ https://reviews.llvm.org/D95714 Files: clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp clang-tools-extra/test/c

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 320678. poelmanc changed the repository for this revision from rCTE Clang Tools Extra to rG LLVM Github Monorepo. poelmanc added a comment. Herald added subscribers: dexonsmith, mgorny. Glad to be back after a year away from clang-tidy, and sorry to have let

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc marked 14 inline comments as done. poelmanc added inline comments. Comment at: clang/lib/Format/Format.cpp:2381 +// if so adds a Replacement to NewReplacements that removes the entire line. +llvm::Error MaybeRemoveBlankLine(tooling::Replacements &NewReplaces, +

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

poelmanc updated this revision to Diff 320704. poelmanc marked an inline comment as done. poelmanc added a comment. Fix formatting, add suggested test case (which works.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95771/new/ https://reviews.llvm.org/D95771 Files: clang-tools-extra

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

poelmanc added a comment. @njames93 Thanks for the review and for accepting this revision. I lack llvm-project commit access so if it's good to go I would greatly appreciate it if you or someone could push this whenever you have have a chance. Thanks! CHANGES SINCE LAST ACTION https://review

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

poelmanc added a comment. Thanks for all the great feedback I received here. To give credit where credit's due, this updated revision to UseNullptrCheck.cpp is now actually 100% @steveire's //suggested// code. Even one of the tests cases was his. Whenever it's ready to land I'd appreciate it if

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

poelmanc added a comment. In D95714#2540685 , @njames93 wrote: > Can I ask if you could tidy the description of this, basically remove all the > stuff about hasGrandparent etc, probably best just remove everything after > `result = (a1 nullptr a2);` in t

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 321451. poelmanc added a comment. Herald added a subscriber: nullptr.cpp. Change loop end condition in `findLineEnd` and add several `assert` statements. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68682/new/ https://reviews.llvm.org/D68682 File

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

poelmanc added a comment. In D95771#2547102 , @njames93 wrote: > Should this be committed using `poelmanc `? That or `Conrad Poelman ` would be great, thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95771/new/ https://reviews.llvm.org/D95

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc added a comment. On 2 Feb Harbormaster found a bug from my switching some char* code to use StringRef. I uploaded a new patch on 4 Feb, but Harbormaster does not appear to have rerun. What triggers Harbormaster - do I need to take some action? I haven'

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

poelmanc added a comment. In D95771#2547366 , @njames93 wrote: > In D95771#2547160 , @poelmanc wrote: > >> In D95771#2547102 , @njames93 wrote: >> >>> Should this be committe

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 322033. poelmanc added a comment. Update one one comment, hopefully trigger HarborMaster to rerun. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68682/new/ https://reviews.llvm.org/D68682 Files: clang-tools-extra/clang-tidy/readability/Redundant

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 322108. poelmanc added a comment. Comment change, "beginning" to "start" for consistency, being sure to set Repository on "diff" page (not just on edit page) to see if https://github.com/google/llvm-premerge-checks/issues/263 was the problem. Repository:

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

poelmanc added a comment. In D95714#2548735 , @njames93 wrote: > LGTM, Same as last time for the commit? That would be great, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95714/new/ https://reviews.llvm.org/D95714

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

poelmanc updated this revision to Diff 322253. poelmanc added a comment. Thanks for your patience, this one should work, as I used my normal `git show HEAD -U99` workflow. (The requested change was just to add a period to a comment, so as a short-cut I tried "Raw Diff" -> copy -> "Update Di

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 322265. poelmanc added a comment. Call `llvm::Annoation` constructor rather than `operator=` to fix linux build issue, fix some issues identified by clang-format and clang-tidy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc added a comment. In D68682#2547906 , @kadircet wrote: > It looks like premerge tests skipped your last diff with id 321451 > (https://reviews.llvm.org/harbormaster/?after=87950). You can re-trigger by > uploading a new diff, in the meantime i wo

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

poelmanc updated this revision to Diff 322302. poelmanc added a comment. Capitalize `IsOrHasDescendant`, add `} namespace std` per HarborMaster feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95714/new/ https://reviews.llvm.org/D95714 File

[PATCH] D96744: clang-format IncludeBlocks: Regroup determination of "main" for framework-style includes fix

poelmanc created this revision. poelmanc added reviewers: MyDeveloperDay, hokein, sammccall. poelmanc added projects: clang-tools-extra, clang-format. poelmanc requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Most modern libraries and applic

[PATCH] D96744: clang-format IncludeBlocks: Regroup determination of "main" for framework-style includes fix

poelmanc added a comment. Thanks for the speedy reply and the great tool. With appropriate Regex and Priority settings, `IncludeCategories` worked flawlessly for me aside from not identifying the main header. Treating `#include "foo/bar/baz.h"` as the main header in file `bar/random/baz.h` seem

[PATCH] D96744: clang-format IncludeBlocks: Regroup determination of "main" for framework-style includes fix

poelmanc added a comment. In D96744#2564828 , @MyDeveloperDay wrote: > Does this need to be an option? It's easy to add an option, but there are already two //main include//-related options, so before adding a third I wanted to give this some thought. A

[PATCH] D97620: Fix DecisionForestBenchmark.cpp compile errors

poelmanc created this revision. poelmanc added a reviewer: usaxena95. poelmanc added a project: clang-tools-extra. Herald added subscribers: kadircet, arphaman. poelmanc requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. clang-to

[PATCH] D97620: Fix DecisionForestBenchmark.cpp compile errors

poelmanc updated this revision to Diff 326937. poelmanc added a comment. Shorten comment and add period. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97620/new/ https://reviews.llvm.org/D97620 Files: clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp

[PATCH] D97625: fix check-clang-tools tests that fail due to Windows CRLF line endings

poelmanc created this revision. poelmanc added a reviewer: clang-tools-extra. poelmanc added a project: clang-tools-extra. Herald added a subscriber: jdoerfert. poelmanc requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Running `check-clang-t

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc updated this revision to Diff 326957. poelmanc added a comment. Removed a function in modernize/LoopConvertCheck.cpp that's no longer needed due to this patch. Fixed clang-format and clang-tidy issues identified by HarborMaster in prior submission. Hopefully it's ready to go, but as al

[PATCH] D97620: Fix DecisionForestBenchmark.cpp compile errors

poelmanc updated this revision to Diff 327174. poelmanc added a comment. Fix comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97620/new/ https://reviews.llvm.org/D97620 Files: clang-tools-extra/clangd/benchmarks/CompletionModel/DecisionForestBenchmark.cpp Index: clang-tools

[PATCH] D97620: Fix DecisionForestBenchmark.cpp compile errors

poelmanc marked an inline comment as done. poelmanc added a comment. Fixed comment as requested (keys()->size().) I don't have commit access so if you can push it that would be great, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97620/new/ https://reviews.llvm.org/D97620

[PATCH] D96744: clang-format IncludeBlocks: Regroup determination of "main" for framework-style includes fix

poelmanc abandoned this revision. poelmanc added a comment. Herald added a project: All. We eventually updated our coding standards and our fairly massive code base to change only the very top "main" include from `#include ` to `#include "Foo.h"`. For all include statements after that, we still

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc marked 2 inline comments as done. poelmanc added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:67-70 +// With -std c++14 or earlier (!LangOps.CPlusPlus17), it was sufficient to +// return CtorExpr->getSourceRange(). H

[PATCH] D69548: Give clang-tidy readability-redundant-string-init a customizable list of string types to fix

poelmanc updated this revision to Diff 226987. poelmanc added a comment. Added release notes, fixed backticks in documentation, removed blank line, removed new hasListedName matcher and used existing hasAnyName matcher. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://r

[PATCH] D69548: Give clang-tidy readability-redundant-string-init a customizable list of string types to fix

poelmanc marked 3 inline comments as done. poelmanc added a comment. @aaron.ballman Thanks for the `hasAnyName` feedback! From the name `internal::VariadicFunction` I assumed arguments were needed at compile-time, but thanks to your suggestion I found the overload taking `ArrayRef`. Repository

[PATCH] D69548: Give clang-tidy readability-redundant-string-init a customizable list of string types to fix

poelmanc marked an inline comment as done. poelmanc added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:21 +const char DefaultStringNames[] = "basic_string"; + aaron.ballman wrote: > I think the default shou

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc added a comment. Thanks @aaron.ballman, I don't have commit access so will someone else commit this? To address the minor nit, should I upload a new patch with post-increment/post-decrement changed to pre-increment/pre-decrement? (Does uploading changes undo the "Ready to Land" status

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc added a comment. Just documenting here that I sent the following email to cfe-...@lists.llvm.org: > When parsing a named declaration with an equals sign with clang -std > c++11/14, clang builds an initializer expression whose SourceRange covers > from the variable name through the end

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc added a comment. In D69238#1736365 , @NoQ wrote: > I suspect that it's not just the source range, but the whole AST for the > initializer is different, due to C++17 mandatory copy elision in the > equals-initializer syntax. Like, before C++17 it

[PATCH] D69548: Give clang-tidy readability-redundant-string-init a customizable list of string types to fix

poelmanc updated this revision to Diff 228541. poelmanc edited the summary of this revision. poelmanc added a comment. Now allows namespaces on types and defaults to `::std::basic_string` as requested. The code uses namespaced string type names to check types, and uses non-namespaced string type

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc added a comment. In D69238#1739429 , @gribozavr2 wrote: > If it is indeed the extra AST node for the elidable constructor, see if you > can use the `ignoringElidableConstructorCall` AST matcher to ignore it, > therefore smoothing over AST differ

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

poelmanc added a comment. In D69145#1725026 , @malcolm.parsons wrote: > I like that this check warns about copy constructors that don't copy. > The warning can be suppressed with a NOLINT comment if not copying is > intentional. Thanks for the comment

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc updated this revision to Diff 228556. poelmanc edited the summary of this revision. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69238/new/ https://reviews.llvm.org/D69238 Files: clang-tools-extra/clang-tidy/readability/RedundantStringIni

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc added a comment. In D69238#1739627 , @NoQ wrote: > Clang's `-ast-dump` > . Wow. That makes this so much easier... Thank you so much! Looking at the AST showed that the `CXXConstructEx

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc updated this revision to Diff 228558. poelmanc added a comment. I just rebased this patch on the latest master. I believe I've addressed all the comments raised so far. Should I add mention of this change to the release notes? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST A

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc updated this revision to Diff 228767. poelmanc added a comment. Changed post-increment/decrement to pre-increment/decrement. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67460/new/ https://reviews.llvm.org/D67460 Files: clang-tools-extr

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

poelmanc marked 2 inline comments as done. poelmanc added a comment. Done, thanks. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67460/new/ https://reviews.llvm.org/D67460 ___ cfe-commits mailing list c

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

poelmanc added a comment. In D69238#1740914 , @gribozavr2 wrote: > Thanks! Do you have commit access or do you need me to commit for you? I don't have commit access, if you could commit it for me that would be great. Thanks! Repository: rCTE Clang T

[PATCH] D69548: Give clang-tidy readability-redundant-string-init a customizable list of string types to fix

poelmanc updated this revision to Diff 228769. poelmanc added a comment. Make requested fixes to documentation. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69548/new/ https://reviews.llvm.org/D69548 Files: clang-tools-extra/clang-tidy/readabili

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc updated this revision to Diff 228795. poelmanc edited the summary of this revision. poelmanc added a comment. Move isWhitespace and skipNewlines to clang/Basic/CharInfo.h (renaming to isWhitespaceStringRef and skipNewlinesChars to resolve name clashes) and add double-quotes around "/n"

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

poelmanc marked 4 inline comments as done. poelmanc added inline comments. Comment at: clang/lib/AST/CommentLexer.cpp:20 + +// Consider moving this useful function to a more general utility location. +const char *skipNewline(const char *BufferPtr, const char *BufferEnd) { ---

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

poelmanc created this revision. poelmanc added reviewers: malcolm.parsons, angelgarcia, aaron.ballman. Herald added subscribers: cfe-commits, mgehre. Herald added a project: clang. `modernize-use-equals-default` replaces default constructors/destructors with `= default;`. When the optional semico

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

poelmanc created this revision. poelmanc added reviewers: alexfh, djasper. Herald added subscribers: cfe-commits, mgehre. Herald added a project: clang. In addition to adding `override` wherever possible, clang-tidy's `modernize-use-override` nicely removes `virtual` when `override` or `final` is

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

poelmanc marked an inline comment as done. poelmanc added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp:306 + if (ApplyFix) { +// Peek ahead to see if there's a semicolon after Body->getSourceRange() +Optional Token; --

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

poelmanc updated this revision to Diff 229161. poelmanc edited the summary of this revision. poelmanc added a comment. Update to add and use new `findNextTokenSkippingComments` and `findNextTokenSkippingKind` utility functions. Since the former calls the latter with just one token type, this req

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

poelmanc updated this revision to Diff 229202. poelmanc added a comment. Change to add just one helper function `findNextTokenSkippingComments` to `LexerUtils.h`, requiring no change to `Token::isOneOf`, and properly call it from `UseEqualsDefaultCheck.cpp`. In D70144#1744737

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

poelmanc added a comment. In D70165#1744007 , @JonasToth wrote: > LGTM! > Did you check on a real code-base that suffers from the issue, if that works > as expected? Thanks! I have now run it on our real code base and it worked as expected. I lack com

  1   2   >