aaron.ballman added a comment. In D82825#2123125 <https://reviews.llvm.org/D82825#2123125>, @njames93 wrote:
> In D82825#2122789 <https://reviews.llvm.org/D82825#2122789>, @aaron.ballman > wrote: > > > Other than the documentation and settling on the option name in the parent > > revision, I think this LGTM (the changes will be mechanical and can be done > > without further review once the parent commit is in, unless you want more > > eyes on it). > > > Are you happy with the reduced warnings in the this alias, when running > locally over llvm I noticed a lot of warnings about else after return where > condition variables are used. usually of the format > > if (llvm::Expected<T> X = ... ) { > return *X; > } > else { > handle(X.takeError()); > } I can see arguments either way on this. Personally, I would not want the check to diagnose this code because that would encourage people to widen the scope and lifetime of `X` or require them to introduce a new compound scope to get the same behavior, and I think that's more problematic than the `else` following a `return`. I am not certain if others feel the same way though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82825/new/ https://reviews.llvm.org/D82825 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
