[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[MyDeveloperDay via Phabricator via cfe-commits]

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