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

2019-11-14 Thread Conrad Poelman via Phabricator via cfe-commits
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

2019-11-14 Thread Conrad Poelman via Phabricator via cfe-commits
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] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-11-14 Thread Conrad Poelman via Phabricator via cfe-commits
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] D67460: clang-tidy: modernize-use-using work with multi-argument templates

2019-11-14 Thread Conrad Poelman via Phabricator via cfe-commits
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] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-14 Thread Conrad Poelman via Phabricator via cfe-commits
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] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-11-16 Thread Conrad Poelman via Phabricator via cfe-commits
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] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-16 Thread Conrad Poelman via Phabricator via cfe-commits
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

2019-11-16 Thread Conrad Poelman via Phabricator via cfe-commits
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] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-11-16 Thread Conrad Poelman via Phabricator via cfe-commits
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] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-18 Thread Conrad Poelman via Phabricator via cfe-commits
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] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

2019-11-18 Thread Conrad Poelman via Phabricator via cfe-commits
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] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

2019-11-19 Thread Conrad Poelman via Phabricator via cfe-commits
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] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

2019-11-20 Thread Conrad Poelman via Phabricator via cfe-commits
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] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

2019-11-20 Thread Conrad Poelman via Phabricator via cfe-commits
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

2019-11-20 Thread Conrad Poelman via Phabricator via cfe-commits
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] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

2019-12-20 Thread Conrad Poelman via Phabricator via cfe-commits
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

2019-12-20 Thread Conrad Poelman via Phabricator via cfe-commits
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

2019-12-21 Thread Conrad Poelman via Phabricator via cfe-commits
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

2019-12-22 Thread Conrad Poelman via Phabricator via cfe-commits
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] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

2019-12-22 Thread Conrad Poelman via Phabricator via cfe-commits
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

2019-12-22 Thread Conrad Poelman via Phabricator via cfe-commits
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

2019-12-22 Thread Conrad Poelman via Phabricator via cfe-commits
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

<    1   2