[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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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 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#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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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 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] 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: 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. 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] 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 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 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] 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] 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] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

poelmanc marked 4 inline comments as done. poelmanc added a comment. In D68682#1754798 , @kadircet wrote: > `ToolingTests/ApplyAtomicChangesTest.FormatsCorrectLineWhenHeaderIsRemoved` > also seems to be failing, you can run it with `ninja ToolingTests &&

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

poelmanc marked an inline comment as done. poelmanc added inline comments. Comment at: clang/include/clang/Basic/CharInfo.h:94 +LLVM_READONLY inline bool isWhitespace(llvm::StringRef S) { + for (StringRef::const_iterator I = S.begin(), E = S.end(); I != E; ++I) { +if (!isWhi

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

poelmanc marked 13 inline comments as done. poelmanc added inline comments. Comment at: clang/lib/Format/Format.cpp:2385 + const char *FileText = Code.data(); + StringRef FilePath; // Must be the same for every Replacement + for (const auto &R : Replaces) { ka

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

poelmanc updated this revision to Diff 235090. poelmanc marked 2 inline comments as done. poelmanc added a comment. Fix algorithm to fix failing `ToolingTests/ApplyAtomicChangesTest.FormatsCorrectLineWhenHeaderIsRemoved`. That test revealed a very specific but real failure case: if existing Remo

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

poelmanc updated this revision to Diff 235039. poelmanc added a subscriber: sammccall. poelmanc added a comment. Update patch to rebase on latest: Changed `SourceLocation::contains` to `SourceLocation::fullyContains`, and removed new `SourceLocation` comparison operators since coincidentally @sa

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

poelmanc updated this revision to Diff 234976. poelmanc added a comment. Address most of the feedback, I'll comment individually. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68682/new/ https://reviews.llvm.org/D68682 Files: clang-tools-extra/cl

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

poelmanc added a comment. Any further feedback or thoughts on this? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70270/new/ https://reviews.llvm.org/D70270 ___ cfe-commits mailing list cfe-commits@list

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

poelmanc marked an inline comment as done. poelmanc added a comment. In D68682#1753357 , @kadircet wrote: > also could you rename the revision so that it reflects the fact that, this is > a change to clang-format and has nothing to do with clang-tidy ?

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

poelmanc marked 5 inline comments as done. poelmanc added inline comments. Comment at: clang/lib/AST/CommentParser.cpp:19 -static inline bool isWhitespace(llvm::StringRef S) { +// Consider moving this useful function to a more general utility location. +bool isWhitespace(llvm::

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

poelmanc updated this revision to Diff 230375. poelmanc marked 3 inline comments as done. poelmanc retitled this revision from "Clang-tidy fix removals removing all non-blank text from a line should remove the line" to "format::cleanupAroundReplacements removes whole line when Removals leave pre

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

poelmanc added a subscriber: MyDeveloperDay. poelmanc added a comment. Just a quick update, I made some progress addressing the architectural limitation of `AnnotatedLines` and `evaluate()`. I changed the `AnnotatedLines` constructor to also take a `Line *PrevAnnotatedLine` argument and set `Li

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

poelmanc added a comment. Thanks @lebedev.ri for taking the time to think this through and reply. All that makes sense, so I've changed the default to `0`. In addition to adding it to the ReleaseNotes, once clang-tidy 10 is released I'll reply to a StackOverflow question about this error with a

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

poelmanc updated this revision to Diff 229934. poelmanc edited the summary of this revision. poelmanc added a comment. Switch default to `0`. Add Release Note with some detail to increase the chances of someone finding this with an Internet search on the error message. Repository: rCTE Clang

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

poelmanc added a comment. > If there's a way to match only `CXXRecordDecl`s that are immediately followed > by a `TypedefDecl`... Alternatively, within `check()` when we get the `TypedefDecl`, is there any way to navigate up the AST to find its immediately preceding sibling node in the AST and

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

poelmanc marked 6 inline comments as done. poelmanc added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:30 + // They appear in the AST just *prior* to the typedefs. + Finder->addMatcher(cxxRecordDecl().bind("struct"), this); } --

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

poelmanc added a subscriber: mgehre. 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

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

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

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

poelmanc added a comment. > ! In D70144#1745583 , @JonasToth > wrote: > >> ! In D70144#1745532 , >> @malcolm.parsons wrote: > > Should `clang::format::cleanupAroundReplacements()` handle this instead? My initial

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

poelmanc marked an inline comment as not done. poelmanc added a comment. In D67460#1737529 , @aaron.ballman wrote: > I'm a bit worried that this manual parsing technique will need fixing again > in the future, but I think this is at least a reasonable in

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

poelmanc created this revision. poelmanc added reviewers: alexfh, aaron.ballman. poelmanc added a project: clang-tools-extra. Herald added subscribers: cfe-commits, mgehre. Herald added a project: clang. In D67460#1737529 , @aaron.ballman wrote: > I'm a b

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

poelmanc added a comment. In D70165#1745530 , @alexfh wrote: > While I have no objections against this patch, I wonder whether someone had a > chance to ask GCC developers about this? Is it a conscious choice to suggest > `override` when `final` is prese

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

poelmanc added a comment. In D70144#1745583 , @JonasToth wrote: > In D70144#1745532 , @malcolm.parsons > wrote: > > > Should `clang::format::cleanupAroundReplacements()` handle this instead? > > > Of yes. Totally f

[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

[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] 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 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] 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 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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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#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] 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] 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] 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] 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 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] 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] 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] 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] 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 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] 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] 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] 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 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] 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:

  1   2   >