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

Reply via email to