[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:5562
+  if (IsDependent)
+    goto Return;
+
----------------
erichkeane wrote:
> Oh, please don't do this.
perhaps another way to do this if you want to avoid repeating the return expression would be a small lambda that contains the return expression and uses a simple name - so the returns can just be `return ret();` ? (or I guess nest the switch in an `if`, or in another function that uses ref parameters to populate the result)

Though in this case it's only twice, so I'd probably repeat the expression?


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:5591
+  default:
+    llvm_unreachable("unhandled type trait (usualy deliberate)");
+  }
----------------
erichkeane wrote:
> What do you mean by `usually deliberate` here? This is a message users will see, so telling them an assertion is deliberate seems incorrect? 
I second the question - though I'd push back on the "This is a message users will see" - unreachables won't be seen by users, they get lowered to UB in release builds - the text isn't in the program anymore. The text is only for Clang developers (and/or clang-as-library users).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151952/new/

https://reviews.llvm.org/D151952