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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Hi Bernhard,

thanks for you patch!
You mentioned that this is your first contribution, if you didn't find these docs already they might help you with the LLVM source-code a bit:

- https://llvm.org/docs/ProgrammersManual.html
- https://llvm.org/docs/DeveloperPolicy.html
- https://llvm.org/docs/CodingStandards.html



================
Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:2
+// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14
+
+namespace std {
----------------
bernhardmgruber wrote:
> aaron.ballman wrote:
> > lebedev.ri wrote:
> > > Missing test coverage:
> > > * macros
> > > * is there tests for implicit functions?
> > > * Out-of-line function with body.
> > Also:
> >   * functions with attributes in the type position `int f() [[]];`
> >   * functions without attributes in the type position `[[]] int f();` and `int f [[]] ();`
> >   * lambdas?
> >   * Special functions without return types, like constructors and destructors
> >   * Conversion operators. `struct S {  operator int() const; };`
> @lebedev.ri: I do not understand what kind of tests I should write for macros. Do you mean to rewrite functions inside macro definitions as well?
> 
> like rewriting
> ```
> #define DEFINE_F int f();
> ```
> into
> 
> ```
> #define DEFINE_F auto f() -> int;
> ```
> 
> Apart from that, I believe a lot of difficult trickery can be done with macros and I am fine without supporting those (i.e. omitting a possible rewrite, erroneus replacements should never happen). Can you come up with a particular example you would like to be rewritten?
usually we leave code inside macros alone as you said because of all the potential issues macros create.
it is best, to cover some expected macro-cases and find a reasonable (sometimes configurable) policy for them.

Some things that come to my mind (macro is all upper case):

- `RETURN_TYPE foo();`
- `CONST return_type foo();`
- `ALWAYS_INLINE DLL_EXPORT int foo();` ( probably the same as above)
- `int foo() ANOTHER_ATTRIBUTE;`
- `int FUNCTION_NAME_MACRO(foo, bar)();`
- `FULL_DEFINITION_IN_MACRO(foo, bar);`
- GTest-like function/method creation in bigger macros. Sometimes full classes with members are created in macros, in principle the same as the prior case

The safest and easiest way is to bail out anytime you find a macro-id in the range you want to transform. Some things, like case 1-5 could in theory be transformable, but should we? If you decide to transform them it is necessary to not use the expanded macro-tokens but the original source text, which should be tested too.


================
Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:173
+auto l1 = [](int arg) {};
+auto l2 = [](int arg) -> double {};
----------------
you could figure out the return type of the lambda if it contains a return, otherwise it should be `void`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56160/new/

https://reviews.llvm.org/D56160