bernhardmgruber marked 2 inline comments as done.
bernhardmgruber added a comment.
@aaron.ballman I do not have commit privileges and I would be very thankful, if
you could commit this patch for me! Thank you!
================
Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:203
+ if (ContainsQualifiers + ContainsSpecifiers + ContainsSomethingElse > 1)
+ return {};
+
----------------
aaron.ballman wrote:
> bernhardmgruber wrote:
> > 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?
> AFAIK, there's no functional difference between the two and it's more a
> matter of consistency. Multiple different types can have the notion of a null
> state, and this allows you to consistently specify that null state across
> types in an explicit way.
>
> I suppose there might also be a very slight argument for clarity in that a
> user reading the code with `{}` could be confused into thinking that is
> default constructing the contained type rather than creating an empty
> optional object.
Thank you for the explanation! I see the point of being more explicit.
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
