aaron.ballman closed this revision. aaron.ballman added a comment. I've commit on your behalf in 345053390ac17dd4a2e759de9e0e24c2605035db <https://reviews.llvm.org/rG345053390ac17dd4a2e759de9e0e24c2605035db>, thank you for the patch!
================ Comment at: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:430 + AT->getKeyword() == AutoTypeKeyword::Auto && + !hasAnyNestedLocalQualifiers(F->getDeclaredReturnType())) + return; ---------------- bernhardmgruber wrote: > aaron.ballman wrote: > > bernhardmgruber wrote: > > > aaron.ballman wrote: > > > > aaron.ballman wrote: > > > > > bernhardmgruber wrote: > > > > > > aaron.ballman wrote: > > > > > > > Why do we need to check that there aren't any nested local > > > > > > > qualifiers? > > > > > > Because I would like the check to rewrite e.g. `const auto f();` > > > > > > into `auto f() -> const auto;`. If I omit the check for nested > > > > > > local qualifiers, then those kind of declarations would be skipped. > > > > > I'm still wondering about this. > > > > > Because I would like the check to rewrite e.g. const auto f(); into > > > > > auto f() -> const auto;. If I omit the check for nested local > > > > > qualifiers, then those kind of declarations would be skipped. > > > > > > > > I don't think I understand why that's desirable though? What is it > > > > about the qualifier that makes it worthwhile to repeat the type like > > > > that? > > > Thank you for insisting on that peculiarity! The choice is stylistically > > > motivated to align function names: > > > > > > ``` > > > auto f() -> int; > > > auto g() -> std::vector<float>; > > > auto& h(); > > > const auto k(); > > > decltype(auto) l(); > > > ``` > > > vs. > > > ``` > > > auto f() -> int; > > > auto g() -> std::vector<float>; > > > auto h() -> auto&; > > > auto k() -> const auto; > > > auto l() -> decltype(auto); > > > ``` > > > > > > But judging from your response, this might be a surprise to users. Would > > > you prefer having an option to enable/disable this behavior? > > > But judging from your response, this might be a surprise to users. Would > > > you prefer having an option to enable/disable this behavior? > > > > Maybe it will be, maybe it won't. :-D The reason I was surprised was > > because it feels like a formatting related choice rather than a > > modernization related choice. However, I've always struggled to understand > > the utility of this check (it's one I disable in every .clang-tidy > > configuration file I can), so my reasoning may be flawed. I feel like the > > goal of this check isn't to format code nicely, it's to modernize to using > > a trailing return type where that adds clarity. But I don't think `auto& > > func()` changing into `auto func() -> auto&` adds clarity (I think it > > removes clarity because the second signature is strictly more complex than > > the first), and similar for qualifiers. However, I think the exact same > > thing about `int func()` changing into `auto func() -> int`. > > > > Given that we document this function to rewrite all functions to a trailing > > return type signature, I guess the behavior you've proposed here is > > consistent with that goal and so I'm fine with it. > > However, I've always struggled to understand the utility of this check > > (it's one I disable in every .clang-tidy configuration file I can) > > I am sorry to hear that, but I have heard this from many other people as > well. I am sometimes questioning myself whether it was a mistake to put this > check into clang-tidy and annoy a lot of people. It might have been better as > a standalone tool. > > > I feel like the goal of this check isn't to format code nicely, it's to > > modernize to using a trailing return type where that adds clarity. > > I like that thinking! I started with trailing return types as a stylistic > choice, but I soon realized that it indeed can add clarity by shifting away > complicated return types to the end of a line where they bother me less. > > > But I don't think `auto& func()` changing into `auto func() -> auto&` adds > > clarity (I think it removes clarity because the second signature is > > strictly more complex than the first). > > With regard to clarity, you are right. And I noted down now that I shall add > an option to prevent some of these rewrites. Thank you for the feedback! > > I am sorry to hear that, but I have heard this from many other people as > well. I am sometimes questioning myself whether it was a mistake to put this > check into clang-tidy and annoy a lot of people. It might have been better as > a standalone tool. FWIW, I don't question whether it was a mistake to add it. I just figure it wasn't targeted at me, and that's fine too. I can see the utility for people who want that kind of declaration style. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80514/new/ https://reviews.llvm.org/D80514 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
