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

This revision was automatically updated to reflect the committed changes. Closed by commit rG782392db8122: [clang-tidy] modernize-use-using work with multi-argument templates (authored by mitchell-stellar). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[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

[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

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

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

aaron.ballman added a comment. In D67460#1739062 , @poelmanc wrote: > Thanks @aaron.ballman, I don't have commit access so will someone else commit > this? I can commit it for you when I get back into the office mid-next week, unless someone else

[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"

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. 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 incremental improvement. LGTM with a minor

[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

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

[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

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

jonathanmeier added a comment. Nice! I don't have commit access, so we'll need someone else have another look. @alexfh, since you previously worked on this, would you mind taking a look at this patch? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

[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

[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

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

jonathanmeier added a comment. Thanks, this looks much better now. I think there's no need to track nesting of parenthesis, brackets and braces separately, so we can collapse `ParenLevel`, `BraceLevel` and `SquareBracketLevel` into a single `NestingLevel`, which simplifies all the conditions

[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

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

jonathanmeier added a comment. Also note that your enhanced version of my first example actually works with your initial patch, since the two comparison operators cancel each other out in the counting logic. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

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

jonathanmeier added a comment. Unfortunately, only considering parenthesis (`l_paren`, `r_paren`) is not sufficient. We need to consider all the nesting tokens, including brackets (`l_square`, `r_square`) and braces (`l_brace`, `r_brace`), since the Standard says (C++ 17 Draft N4659, Section

[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 ==

[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

jonathanmeier added a comment. With non-type template parameters we can have expressions inside template arguments, including comparison operators like `<`, `<=`, `>` and `>=`, which lets us write typedefs like this: template struct S {}; typedef S<(0 < 0)> S_t, *S_p; Unfortunately,

[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