gribozavr added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:306 + if (CEArg->isElidable()) { + if (const auto *TempExp = CEArg->getArg(0)) { + if (const auto *UnwrappedCE = ---------------- Eugene.Zelenko wrote: > gribozavr wrote: > > Eugene.Zelenko wrote: > > > gribozavr wrote: > > > > Eugene.Zelenko wrote: > > > > > gribozavr wrote: > > > > > > Eugene.Zelenko wrote: > > > > > > > Return type is not mentioned explicitly, so auto should not be > > > > > > > used. > > > > > > An explicit type is not needed for readability here. The rule is > > > > > > to use auto when it improves readability, not when the type is not > > > > > > spelled in immediate vicinity. > > > > > I think it's reasonable to follow modernize-use-auto. > > > > modernize-use-auto is only a heuristic. > > > But set of processed situations are very reasonable. > > In abstract it might sound reasonable. In practice it is still a heuristic > > and not a law. > I think it's reasonable to keep in memory that not everybody keeps > functions/methods' return types in memory. 'auto' and the rules about 'auto' serve readability purposes. They are not for ensuring that types are always visible in the source code. Knowing the type is not the goal in itself. The types in the source code need to serve a purpose, like everything else we write. If you think that 'auto' is not reasonable here, I would ask you to explain why the code becomes less readable for an average developer familiar with Clang -- who knows what constructors are, and what constructor arguments are. As an illustration, consider this code: ``` f(g()); ``` We write code like this all the time and we are OK with not seeing the return type of `g` in the source code. If we extract a variable to give it a descriptive name, we should be OK with 'auto' in: ``` auto descriptiveName = g(); f(x); ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62736/new/ https://reviews.llvm.org/D62736 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits