MyDeveloperDay added inline comments.
================
Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:45
+ std::string(Message) +
+ " (no FixIt provided, function argument list end is inside
macro)");
+ return {};
----------------
bernhardmgruber wrote:
> JonasToth wrote:
> > I think you can ellide that extra message. Not emitting the fixit is clear
> > already.
> I actually like having a reason why my check could not provide a FixIt.
> Especially because there are multiple reasons why this might happen.
@bernhardmgruber I had the same comment given to me on a review recently with
regard to diag message, let me try and help you with what I though was the
rational... I think the premise is something like:
1) "no FixIt provided" is implied by the fact it isn't fixed
2) "function type source info is missing" doesn't tell the developer what they
have to do to have it be fixed
sure it helps you as the checker developer but probably that isn't much use to
a developer using the checker on their code and so might confuse them.
It also makes grepping for messages in a log file difficult because it means
multiple messages coming from your checker have a different pattern (although
there might be a common sub pattern)
For the most part where a fixit is not supplied where it should someone would
create a test case which you could consume in your tests
To be honest as a general observation as a newbie myself, what I've noticed is
that a lot of checker review comments are very similar,
- 80 characters in rst files
- clang-format
- alphabetic order
- Comments with proper puncuation
- code in comments in ``XXX``
- don't overly use auto
- don't use anonymous namespace functions use static functions
- run it on a big C++ project
- run it over all of LLVM
- consistency of coding style (elide unnecessary const)
- elide unnecessary braces/brackets/code/etc..
We really should try and write a "Writing a clang checker, and getting it
though review" primer, because I really feel for these "gaints" that we ask to
review all this code, they must go over the same thing and have to present the
same reasons time and time again...
which is why If you don't mind I'd like to try to help give something back by
filling in some of the reasoning gaps here to a fellow new starter
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56160/new/
https://reviews.llvm.org/D56160
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
