[PATCH] D125259: [C11] Diagnose unreachable generic selection associations

[Erich Keane via Phabricator via cfe-commits]

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