[PATCH] D149904: Generic selection expressions that accept a type operand

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/include/clang/AST/Expr.h:5686
   /// if and only if the generic selection expression is result-dependent.
-  unsigned NumAssocs, ResultIndex;
+  unsigned NumAssocs, ResultIndex, IsExprPredicate;
   enum : unsigned {
----------------
I realize this is a rarely used type, but should these be bitfielded somehow?  Particularly now that you're adding a 'bool' field?


================
Comment at: clang/include/clang/AST/Expr.h:5691
 
+  unsigned getIndexOfControllingExpression() const {
+    // If controlled by an expression, the first offset into the Stmt *
----------------
What is the value of these functions, if they only return 0 or 1?


================
Comment at: clang/include/clang/AST/Expr.h:5712
+    // predicate expression.
+    return (int)isExprPredicate();
+  }
----------------
Should these have asserts also?  Wouldn't saying this is index '0' be incorrect here if t his is the typePredicate?


================
Comment at: clang/include/clang/Parse/Parser.h:2551
+  bool isTypeIdForGenericSelection() {
+    bool isAmbiguous;
+    if (getLangOpts().CPlusPlus)
----------------
slight preference to put this declaration in the 'if' condition below, the split in location confused me for a bit trying to figure out where it was being used.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:19851
+    bool IsExpr = GSE->isExprPredicate();
+    if (IsExpr)
+      ExOrTy = GSE->getControllingExpr();
----------------
I find myself wondering if a `getControllingOperand` here is valuable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149904