[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-11 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 169277. ymandel added a comment. Adusted code in respone to comments. Major differences are: - more warning in situations where a FixIt can't be suggested. - more tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files:

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-11 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 169282. ymandel added a comment. Spacing fix. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ConstValueReturnCheck.cpp

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-11 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Thank you for the detailed comments! They were quite helpful, especially as this is my first llvm/clang review. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:25 + +// Finds the location of the relevant "const" token in `Def`.

[PATCH] D52971: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py to support multiple prefixes

2018-10-11 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Hi, It looks like this change has disabled FileCheck for all clang-tidy lit tests that don't use check-prefixes. So, they all trivially pass even if their CHECK... lines are wrong. An easy repro is just to randomly modify any CHECK line in a lit file (e.g.

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-11 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 169280. ymandel added a comment. comment tweak Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ConstValueReturnCheck.cpp

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-11 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 169290. ymandel marked 11 inline comments as done. ymandel added a comment. Comment fix. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ConstValueReturnCheck.cpp

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 5 inline comments as done. ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:60 +diag(Def->getInnerLocStart(), + "return type is 'const'-qualifed (possibly behind a type alias), " + "which hinders

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:25 + +// Finds the location of the relevant "const" token in `Def`. +llvm::Optional findConstToRemove( JonasToth wrote: > ymandel wrote: > > JonasToth wrote: > > >

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 169461. ymandel marked 3 inline comments as done. ymandel added a comment. Adjusted reporting of warnings. Also, addressed other comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:64 + + // Fix the definition. + llvm::Optional Loc = findConstToRemove(Def, Result); JonasToth wrote: > ymandel wrote: > > JonasToth wrote: > > > I feel that this

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-16 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 169941. ymandel added a comment. Changed check result into struct (was pair). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ConstValueReturnCheck.cpp

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-16 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:93 +Diagnostics << FixItHint::CreateRemoval( +CharSourceRange::getTokenRange(*PrevLoc, *PrevLoc)); + } else { JonasToth wrote: > ymandel wrote: >

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-16 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 169938. ymandel marked 5 inline comments as done. ymandel added a comment. Refactors generation of clang-tidy fixits and notes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Correction: the code *catches* the trailing return cases, I just couldn't find a way to *fix* them; specifically, I was not able to get the necessary source ranges to re-lex the trailing return type. Repository: rCTE Clang Tools Extra

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked an inline comment as done. ymandel added inline comments. Comment at: test/clang-tidy/readability-const-value-return.cpp:174 +int **const * n_multiple_ptr(); +int *const & n_pointer_ref(); aaron.ballman wrote: > I'd like to see some more complex

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 170418. ymandel added a comment. Expanded test cases. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ConstValueReturnCheck.cpp

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 170423. ymandel marked an inline comment as done. ymandel added a comment. Fix some message comments in tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:64 + + // Fix the definition. + llvm::Optional Loc = findConstToRemove(Def, Result); JonasToth wrote: > ymandel wrote: > > JonasToth wrote: > > > ymandel wrote: > > > >

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Thanks! What's the next step in the process? Does someone need to approve this change? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-31 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171900. ymandel added a comment. Rename readability-const-value-return to readability-const-return-type Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-31 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171897. ymandel marked 2 inline comments as done. ymandel added a comment. Small tweaks in response to comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-31 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. I've renamed the check. I noticed that the script did not rename the header guard macro, though. Please let me know if/where to file a bug. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 ___

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-31 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In https://reviews.llvm.org/D53025#1281097, @aaron.ballman wrote: > I think this is getting really close! One question: would it make more sense > to name this `readability-const-type-return` or > `readability-const-return-type` instead? It's not that the functions

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-31 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 4 inline comments as done. ymandel added a comment. Aaron, Jonas, Thank you both so much for being fantastic reviewers! This was my first experience contributing to clang (upstream) and it has been extremely positive. In https://reviews.llvm.org/D53025#1282194, @aaron.ballman

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-31 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171985. ymandel marked an inline comment as done. ymandel added a comment. Rebased patch onto trunk. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<<

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171293. ymandel marked 4 inline comments as done. ymandel added a comment. Reword comment on the check, for clarity. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 3 inline comments as done. ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171700. ymandel added a comment. Reworded diagnostic message and corresponding documentation. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<<

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171765. ymandel marked 2 inline comments as done. ymandel added a comment. Add comment to field. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-30 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171764. ymandel marked an inline comment as done. ymandel added a comment. Modify dialog printing to highlight const token. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In https://reviews.llvm.org/D53025#1271367, @JonasToth wrote: > In https://reviews.llvm.org/D53025#1270784, @ymandel wrote: > > > Correction: the code *catches* the trailing return cases, I just couldn't > > find a way to *fix* them; specifically, I was not able to get

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171103. ymandel added a comment. Fix bug in const-token finding and add tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ConstValueReturnCheck.cpp

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 171105. ymandel added a comment. Formatting & comment tweaks. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ConstValueReturnCheck.cpp

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added reviewers: aaron.ballman, JonasToth, ioeric. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Adds a new check readability-const-value-return, which checks for functions with a ``const``-qualified return type and recommends removal of the

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 168856. ymandel added a comment. Minor changes addressing comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ConstValueReturnCheck.cpp

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 168857. ymandel marked 2 inline comments as done. ymandel added a comment. Dropped unneeded clang qualifier. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 3 inline comments as done. ymandel added a comment. Thanks for the review. I've addressed the comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53025: [clang-tidy] implement new check for const return types.

2018-10-09 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 168858. ymandel added a comment. Add missing const. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ConstValueReturnCheck.cpp

[PATCH] D56786: Changes to `CXXMemberExpr` matchers.

2019-01-16 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. Herald added a subscriber: cfe-commits. Assorted changes to matchers for `CXXMemberExpr`: - fixes the comments on `hasObjectExpression`, - clarifies comments on `thisPointerType` and `on`, - adds comments to `onImplicitObjectArgument` - adds new matcher

[PATCH] D56786: Changes to `CXXMemberExpr` matchers.

2019-01-16 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D56786#1359879 , @steveire wrote: > Can you break this up into multiple commits? Sure, but any suggestions on granularity? E.g. i can split into two: the fixes/clarifications in one and the new matcher in another; or i

[PATCH] D56786: [ASTMatchers] Changes to `CXXMemberExpr` matchers.

2019-01-16 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added a comment. In D56786#1359920 , @lebedev.ri wrote: > In D56786#1359903 , @ymandel wrote: > > > In D56786#1359879 ,

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

2019-01-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 182295. ymandel added a comment. Extended test for matcher `on`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56850/new/ https://reviews.llvm.org/D56850 Files: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp Index:

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

2019-01-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 182299. ymandel added a comment. Respond to comments; fix mistake (revealed by test). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56849/new/ https://reviews.llvm.org/D56849 Files: clang/docs/LibASTMatchersReference.html

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

2019-01-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 182301. ymandel marked 2 inline comments as done. ymandel added a comment. Update HTML docs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56849/new/ https://reviews.llvm.org/D56849 Files: clang/docs/LibASTMatchersReference.html

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

2019-01-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 4 inline comments as done. ymandel added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2905 +/// cxxMemberCallExpr(on(callExpr())) +/// matches `(g()).m()` /// aaron.ballman wrote: > Missing a full stop. also fixed

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

2019-01-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:475 + auto MatchesType = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X"); + EXPECT_TRUE(matches( + R"cc(

[PATCH] D56933: [Tooling][RFC] Introduce Stencil library to simplify source code generation in refactorings.

2019-01-18 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. Herald added subscribers: cfe-commits, jfb. REQUEST FOR COMMENT: this is not intended (yet) as a proper revision. It complements the design document for Transformer: https://docs.google.com/document/d/1ppw0RhjwsrbBcHYhI85pe6ISDbA6r5d00ot3N8cQWeQ/edit?usp=sharing

[PATCH] D56786: [ASTMatchers] Changes to `CXXMemberExpr` matchers.

2019-01-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D56786#1360706 , @steveire wrote: > It seems that you update docs for existing matchers without changing those > matchers. You could put all of that in one patch. > > Then, you seem to add some tests for existing matches. You

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

2019-01-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D56850#1361436 , @lebedev.ri wrote: > Please always subscribe lists. > The reviews that happen without lists are null and void. Thanks for updating my diffs with the relevant list. Noted for next time. > Is this testing

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 178507. ymandel added a comment. Update test that was calling builAST with an actual Twine. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55765/new/ https://reviews.llvm.org/D55765 Files: include/clang/Tooling/Tooling.h

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-18 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 178653. ymandel added a comment. Adjust comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55765/new/ https://reviews.llvm.org/D55765 Files: include/clang/Tooling/Tooling.h lib/Tooling/Tooling.cpp

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-18 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 178651. ymandel added a comment. Convert other Twine-typed args to StringRef. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55765/new/ https://reviews.llvm.org/D55765 Files: include/clang/Tooling/Tooling.h

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-18 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Alex, would you mind submitting this since I don't yet have submit privileges? I'll request them shortly, but I'd rather this go one in the meantime. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55765/new/

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 178468. ymandel added a comment. Change buildAST functions to take a StringRef instead of a Twine. In practice, code is almost never constructed on the fly using a Twine, so StringRef is simpler and avoids needing a copy when constructing the MemBuffer.

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 178470. ymandel added a comment. Change header correspondingly. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55765/new/ https://reviews.llvm.org/D55765 Files: include/clang/Tooling/Tooling.h lib/Tooling/Tooling.cpp

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added a reviewer: alexfh. Herald added a subscriber: cfe-commits. `buildASTFromCodeWithArgs()` was creating a memory buffer referencing a stack-allocated string. This diff changes the implementation to copy the code string into the memory buffer so that

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

2019-01-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 182919. ymandel marked an inline comment as done. ymandel added a comment. Remove unnecessary expectation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56850/new/ https://reviews.llvm.org/D56850 Files:

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

2019-01-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:558 + int m; + int f(X x) { return m; } +}; steveire wrote: > Are we missing a matcher that would reach the type of X in this case? >

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

2019-01-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 182918. ymandel marked an inline comment as done. ymandel added a comment. Use backticks instead of quotes for quoted code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56849/new/ https://reviews.llvm.org/D56849 Files:

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

2019-01-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 182933. ymandel marked 3 inline comments as done. ymandel added a comment. Extended test of `hasObjectExpression`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56850/new/ https://reviews.llvm.org/D56850 Files:

[PATCH] D56851: [ASTMatchers] Adds `CXXMemberCallExpr` matcher `invokedAtType`.

2019-01-28 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3300 +/// matches `x.m()` and `p->m()`. +AST_MATCHER_P_OVERLOAD(clang::CXXMemberCallExpr, invokedAtType, +

[PATCH] D56851: [ASTMatchers] Adds `CXXMemberCallExpr` matcher `invokedAtType`.

2019-01-28 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 2 inline comments as done. ymandel added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3300 +/// matches `x.m()` and `p->m()`. +AST_MATCHER_P_OVERLOAD(clang::CXXMemberCallExpr, invokedAtType, +

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193306. ymandel added a comment. Remove dependency on NodeIds. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59376/new/ https://reviews.llvm.org/D59376 Files:

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. I've removed the dependency on NodeIds given our (off thread) discussions regarding their overall value and appropriateness in clang::tooling vs clang::ast_matchers. PTAL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193260. ymandel marked 3 inline comments as done. ymandel added a comment. Small tweaks in response to comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59376/new/ https://reviews.llvm.org/D59376 Files:

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Thank you for the extremely helpful and detailed review!! Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:110 + // constructed with the builder class. + static constexpr char RootId[] = "___root___"; +};

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-03-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 192267. ymandel marked 5 inline comments as done. ymandel added a comment. - Remove lvalue-ref overloads in builder - add StringRef overloads for TextGenerator-taking methods - constrain Name targeting for ctor initializers to explicitly written

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-03-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked an inline comment as done. ymandel added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:89 + /// relevant name, not including qualifiers. + Name, +}; ilya-biryukov wrote: > ymandel wrote: > > ilya-biryukov

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-03-22 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked an inline comment as done. ymandel added a comment. Working on the new version now. Will note with "PTAL" once that's ready. Sorry that wasn't clear in earlier responses. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59376/new/

[PATCH] D60213: Add clangTooling dependency to fix the build.

2019-04-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. Fixes https://reviews.llvm.org/D59371, which broke the build for shared libraries. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D60213 Files:

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193584. ymandel added a subscriber: ABataev. ymandel added a comment. Sever dependency of clangToolingRefactor on clangTooling via FixIt. Transformer used FixIt, which causes a problematic dependency. This diff copies the (minimal) code from FixIt to

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Ilya, this diff fixes the breakage. PTAL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59376/new/ https://reviews.llvm.org/D59376 ___ cfe-commits mailing list

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel reopened this revision. ymandel added a comment. This revision is now accepted and ready to land. Reopening to fix the breakage caused by previous attempt at commit. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59376/new/ https://reviews.llvm.org/D59376

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

2019-04-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193496. ymandel added a comment. Sever dependency on NodeId and some general cleanup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files:

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

2019-04-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193498. ymandel added a comment. Remove noisy default-defined constructors/operators Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files:

[PATCH] D60213: Add clangTooling dependency to fix the build.

2019-04-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D60213#1453516 , @ABataev wrote: > Nope, it won't work. There is a cyclic dependency, unfortunately. > clangTooling depends on clangToolingRefactor already. You need to revert your > commit and refactor it to remove the

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D59376#1454617 , @ilya-biryukov wrote: > Keeping it **only** in the cpp file also LG if we don't plan to have other > usages for now, but please remove the version from `FixIt.h` The Stencil library will need this as well

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. In D59376#1454768 , @ilya-biryukov wrote: > Per @ioeric's suggestion: why not move the helper into > `Tooling/Refactoring/ExtendedRange.h`? > If it's in `ToolingRefactoring`, both stencil and transformer can access it. > > For

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

2019-04-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel created this revision. ymandel added reviewers: ilya-biryukov, ioeric. Herald added a project: clang. Introduces a utility library in Refactoring/ to collect routines related to source-code manipulation. In this change, we move "extended-range" functions from the FixIt library (in

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

2019-04-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193735. ymandel added a comment. Add tests for SourceCode and remove corresponding tests from FixIt tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60269/new/ https://reviews.llvm.org/D60269 Files:

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

2019-04-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193741. ymandel added a comment. Various tweaks to get tests compiling/passing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60269/new/ https://reviews.llvm.org/D60269 Files:

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

2019-04-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193736. ymandel added a comment. Herald added a subscriber: mgorny. Update CMakeLists.txt and rename (misnamed) test file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60269/new/

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-01 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193219. ymandel marked 28 inline comments as done. ymandel added a comment. Assorted changes in response to comments. Most notably, dropped constructor from RewriteRule, in favor of putting meaningful initialization in the builder. Repository: rG LLVM

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-01 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments. Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:70 +// +// * Explanation: explanation of the rewrite. +// ilya-biryukov wrote: > ymandel wrote: > > ilya-biryukov wrote: > > > NIT: maybe mention what it should be

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-03 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC357576: [LibTooling] Add Transformer, a library for source-to-source transformations. (authored by ymandel, committed by ). Changed prior to commit: https://reviews.llvm.org/D59376?vs=193306=193480#toc

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

2019-04-05 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193861. ymandel added a comment. Delete unused function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60269/new/ https://reviews.llvm.org/D60269 Files: clang/include/clang/Tooling/FixIt.h

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

2019-04-05 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193865. ymandel added a comment. Fix trailing whitespace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60269/new/ https://reviews.llvm.org/D60269 Files: clang/include/clang/Tooling/FixIt.h

[PATCH] D60269: [LibTooling] Add "SourceCode" library for functions relating to source-code manipulation.

2019-04-05 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL357764: [LibTooling] Add SourceCode library for functions relating to source-codeā€¦ (authored by ymandel, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-05 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193869. ymandel added a comment. Rebase onto master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59376/new/ https://reviews.llvm.org/D59376 Files: clang/include/clang/Tooling/Refactoring/Transformer.h

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-05 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL357768: [LibTooling] Add Transformer, a library for source-to-source transformations. (authored by ymandel, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

2019-04-05 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193906. ymandel added a comment. Adjust to use SourceCode instead of FixIt. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59371/new/ https://reviews.llvm.org/D59371 Files:

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193750. ymandel added a comment. Rebasing to parent D60269 so that diffs show correctly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59376/new/

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-04-04 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 193748. ymandel added a comment. Switch from using FixIt to SourceCode, as solution to circular dependency problem. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59376/new/ https://reviews.llvm.org/D59376

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-03-29 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 192889. ymandel marked an inline comment as done. ymandel added a comment. - Assorted changes in response to comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59376/new/ https://reviews.llvm.org/D59376

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-03-29 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 19 inline comments as done. ymandel added a comment. Thanks for (more) helpful comments. I think the code is a lot better now than it started out. :) Also, converted `RewriteRule` to a simple struct as per our discussion. Comment at:

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

2019-03-28 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment. Thanks for the detailed review and really helpful comments! Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:38 +/// @{ +using ast_matchers::CXXCtorInitializerMatcher; +using ast_matchers::DeclarationMatcher;

  1   2   3   >