User Details
- User Since
- Dec 30 2018, 9:21 AM (213 w, 4 d)
Oct 13 2022
Hi! I am the original author of this check. I very much welcome your contribution! Thank you for the effort!
Apr 26 2022
Yes, sure, just please specify what user.name and user.email would you like to be associated with the commit's author?
Can someone merge this please for me? Thank you!
Apr 8 2022
Added missing '.' in helptext.
Rebased on main and applied review comments.
Apr 4 2022
I did not base this revision on my latest main (422d05e792dbd6a97f5afd4cdd5e8aa677e97444) since I could not run the python scripts with the changes introduced in 9e1f4f13984186984ba37513372c1b7e0c4ba4fd.
Oct 3 2020
Oct 2 2020
Thank you for the quick review! Can you please commit it for me as well? Thank you!
Oct 1 2020
Fixed whitespace in test code.
Aug 14 2020
Thank you for the time to review this!
Aug 10 2020
Btw: what is the general rule for Phabricator reviews here when to tick the Done checkbox of a comment? What is the semantic of this checkbox? Is the ticked state the same for everyone?
Aug 6 2020
Ping.
Jul 21 2020
Removed probably stale FIXME.
Great, it seems I forgot to submit the inline comments. Sorry about that!
Jul 19 2020
- Added two more tests with a macro supplied on the command line
- Rebased onto master
Jul 8 2020
Is there anything I am not seeing here that you still would like me to do? I feel like you are waiting for something obvious from my side :S.
Jun 25 2020
Ping.
Jun 13 2020
Ping.
Jun 3 2020
That's exactly the issue; the tests run fine on my local machine. I run check_clang_tidy.py manually, since I lack some Unix tools for the full test suite. And the CI build that failed seems to have used the first verison of my diff, which I updated in the meantime. From my point of few the diff is ready. I am just waiting for feedback, a LGTM and a nice person who can commit it for me :)
Reuploaded diff in an attempt to trigger a CI build.
Furthermore: how can I schedule another CI build with the newest version of the diff? When I go to the failed build from Harbormaster and I click restart, it rebuilds an older version of the diff. Thanks for the info!
May 25 2020
moved C++20 tests to a new file
- split code tables in documentation
- removed unnecessary return statement
Dec 4 2019
@Eugene.Zelenko I tried to find what you refer to by PR44206, but I could not find anything :/ Can you please provide me with a link? Thank you!
May 9 2019
- fixed formatting
- fixed function names in tests
- added -fexceptions to test arguments
- fixed typo in release notes
@aaron.ballman I do not have commit privileges and I would be very thankful, if you could commit this patch for me! Thank you!
May 8 2019
@aaron.ballman and @JonasToth: Thank you for the patience and all the feedback! It means a great deal to me to have a patch accepted here!
May 7 2019
Fixed small nits suggested by @aaron.ballman. Thanks!
May 4 2019
It took a long while to figure out how to handle certain macro cases. Here is what I came up with:
Mar 18 2019
Thank you for the rich feedback @aaron.ballman. I found a solution which seems to work for many of my test cases.
- extracting specifiers from the return type if it consists of a multitoken built-in type, and preprending it to 'auto'.
- extended matcher to include friend declarations of functions
- added many tests for return types which contain specifiers
Mar 9 2019
- added support for __restrict
- added two dots at end of comments
Mar 4 2019
Fixed some little nits, thanks @JonasToth!
Feb 27 2019
- renamed the check to modernize-use-trailing-return-type
- fixed the out-of-bounds access to check whether there is a space after the return type
Hi guys!
- rebased onto current master
- implemented a basic check for name collisions of unqualified names in the return type with function arugment names using RecursiveASTVisitor
- moved retrieval of FunctionTypeLoc out of findTrailingReturnTypeSourceLocation()
- replaced F.getReturnType().hasLocalQualifiers() by custom function hasAnyNestedLocalQualifiers(), as the former does not work if the qualifiers are not on the topmost type. E.g.: const int*. This is a PointerType without qualifiers, the const qualifier is part of the nested pointee type.
- inhibiting the rewrite, if the topmost return type is a decltype expression. the source range in this case does not include the expression in parenthesis after the decltype
- inserting an additional space after auto in case there was no space between the return type and the function name. E.g.: int*f();
- extended documentation with known limitations
- added more tests
Jan 26 2019
Thank you for the feedback!
Jan 22 2019
Thank you again @JonasToth for all your valueable input! I could almost successfully run my check on the llvm/lib subfolder. I created a compilation database from within Visual Studio using an extension called SourceTrail. One of the issues was the following:
Jan 17 2019
Thank you @JonasToth for providing more feedback! I will add a few more tests with templates. Maybe I should even try to run the check on Boost and see what happens.
Addressed most of the new review comments (mainly uppercasing start of comments).
Jan 11 2019
Skipping the check for functions which do not have a valid location. This occurred when I run the check on the LLVM code base. It looked like the matcher matched something like a built in operator.
Jan 10 2019
Fixed a warning when building with gcc. Added -fdeclspec when running tests to let them compile on Linux as well.
rebased on current master (there was a conflict in the release notes)
Jan 9 2019
I am satisfied with the proposed feature set for now. I will try to run the check on LLVM itself in the next days as a final test. Are there anymore feature requests or changes from your sides?
- Removed detailed diagnostic messages why FixIts could not be generated
- Excluded functions returning member pointers for now
- All tests run now
I spent some time now to get member pointers as return values working and I am afraid but it seems there is a problem with the clang AST. Given the following code in my check (where F is a FunctionDecl):
const TypeSourceInfo *TSI = F.getTypeSourceInfo(); const FunctionTypeLoc FTL = TSI->getTypeLoc().IgnoreParens().getAs<FunctionTypeLoc>(); auto rl = FTL.getReturnLoc(); rl.getSourceRange().dump(SM); rl.getLocalSourceRange().dump(SM); rl.castAs<MemberPointerTypeLoc>().getSourceRange().dump(SM); rl.castAs<MemberPointerTypeLoc>().getLocalSourceRange().dump(SM); rl.castAs<MemberPointerTypeLoc>().getBeginLoc().dump(SM); rl.castAs<MemberPointerTypeLoc>().getStarLoc().dump(SM);
with the following input:
namespace std { template <typename T> class vector;
Jan 8 2019
Jan 7 2019
@JonasToth Do you really think I should drop the extra information on why I could not provide a FixIt? If truly yes, than I would like to keep them as comments.
Fixed most of the issues pointed out by @JonasToth and added a few more tests. The tests with data member pointers currently fail and I will investiage this.
Jan 6 2019
Big thank you to @JonasToth for providing some macro test cases. I played a bit with macros and settled on the following behavior:
- if the closing parenthesis of the function is inside a macro, no FixIt will be created (I cannot relyably lex for subsequent CV and ref qualifiers and maybe we do not want to make changes in macros)
- if the return type is not CV qualified, i allow macros to be part of it because no lexing is required
- if the return type is CV qualified and contains macros, I provide no FixIt
Jan 2 2019
rebased from release_70 onto master
added more test cases, allowing check to run on out-of-line definitions and updated docs.
I fixed most of the stylistic issues. Regarding the missing test cases, I will add those and do the necessary code changes. Thank you very much for pointing them out to me!
Dec 30 2018
updated diff to one with full context
This is my first contribution to LLVM and I may not yet know the conventions here.