bernhardmgruber added a comment. @aaron.ballman and @JonasToth: Thank you for the patience and all the feedback! It means a great deal to me to have a patch accepted here!
================ Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:95-98 + if (!S->getQualifierLoc() && Name.isIdentifier() && + VisitUnqualName(Name.getAsIdentifierInfo()->getName())) + return false; + return true; ---------------- aaron.ballman wrote: > This can also be simplified into a single return statement rather than an > `if`, but it's less clear to me whether that's an improvement. WDYT? Let's simplify it. ================ Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:203 + if (ContainsQualifiers + ContainsSpecifiers + ContainsSomethingElse > 1) + return {}; + ---------------- aaron.ballman wrote: > This should return `llvm::None` I always wondered what the point of `llvm::None`, `std::nullopt` or `boost::none` is. When I write `return {};` it looks like i return an empty shell, exactly how I picture an empty optional in my head. That is why I prefer it this way. I will change it of course for this patch, but would you mind giving me a short reason, why `llvm::None` is preferable here? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56160/new/ https://reviews.llvm.org/D56160 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits