erichkeane added inline comments.
================ Comment at: clang/lib/Sema/SemaExpr.cpp:1702 + + if (Reason) + Diag(Types[i]->getTypeLoc().getBeginLoc(), ---------------- aaron.ballman wrote: > erichkeane wrote: > > Splitting this up into the separate line for only 2 items seems like a > > waste, would it look better to just do the diag in each 'if'? > I tried it both ways and I think using only one call to `Diag` is better (it > makes it easier if we want to update the diagnostic text in the future, say > to add more information to it like a source range, etc). However, I don't > feel super strongly -- if you'd like to see it changed still, I can do so. I don't feel super strongly either. It just seemed to be 4 lines of rigamarole to 'save' 1 line of copy/paste :D So the judgement comes down to 'how much do we expect the message format/reasons to emit it' will change vs the extra mental 'overhead'. Since you already committed it, I'd say it is not worth a cleanup patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125259/new/ https://reviews.llvm.org/D125259 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
