[AST][NFC] Various cleanups to GenericSelectionExpr

[riccibruno]

Summary

Various cleanups to GenericSelectionExpr, factored out of D57104. In particular:

1. Move the friend declaration to the top.
2. Introduce a constant ResultDependentIndex instead of the magic "-1".
3. clang-format
4. Group the member function together so that they can be removed as one block by D57106.

NFC.

[aaron.ballman]

LGTM!

include/clang/AST/Expr.h

5095

If you wanted to fix up i to be Idx or something more closely matching the naming conventions, that would be a nice change. Same elsewhere in the patch.

[riccibruno]

include/clang/AST/Expr.h

5095

It is just going to be removed by D57106 so I didn't bother.

[aaron.ballman]

include/clang/AST/Expr.h

5095

Ah, sounds great, thanks!

include/clang/AST/Expr.h

 /// As an extension, generic selections are allowed in C++, where the following
 /// additional semantics apply:
 ///
 /// Any generic selection whose controlling expression is type-dependent or
 /// which names a dependent type in its association list is result-dependent,
 /// which means that the choice of result expression is dependent.
 /// Result-dependent generic associations are both type- and value-dependent.
 class GenericSelectionExpr : public Expr {
-  enum { CONTROLLING, END_EXPR };
-  TypeSourceInfo **AssocTypes;
-  Stmt **SubExprs;
+  friend class ASTStmtReader;
+  friend class ASTStmtWriter;
+
+  /// The number of association expressions and the index of the result
+  /// expression in the case where the generic selection expression is not
+  /// result-dependent. The result index is equal to ResultDependentIndex
+  /// if and only if the generic selection expression is result-dependent.
   unsigned NumAssocs, ResultIndex;
+  enum : unsigned {
+    ResultDependentIndex = std::numeric_limits<unsigned>::max(),
+    ControllingIndex = 0,
+    AssocExprStartIndex = 1
+  };
+
+  /// The location of the "_Generic", "default" and of the right parenthesis.
   SourceLocation GenericLoc, DefaultLoc, RParenLoc;
 
+  TypeSourceInfo **AssocTypes;
+  Stmt **SubExprs;
+
 public:
-  GenericSelectionExpr(const ASTContext &Context,
-                       SourceLocation GenericLoc, Expr *ControllingExpr,
-                       ArrayRef<TypeSourceInfo*> AssocTypes,
-                       ArrayRef<Expr*> AssocExprs,
-                       SourceLocation DefaultLoc, SourceLocation RParenLoc,
+  GenericSelectionExpr(const ASTContext &Context, SourceLocation GenericLoc,
+                       Expr *ControllingExpr,
+                       ArrayRef<TypeSourceInfo *> AssocTypes,
+                       ArrayRef<Expr *> AssocExprs, SourceLocation DefaultLoc,
+                       SourceLocation RParenLoc,
                        bool ContainsUnexpandedParameterPack,
                        unsigned ResultIndex);
 
   /// This constructor is used in the result-dependent case.
-  GenericSelectionExpr(const ASTContext &Context,
-                       SourceLocation GenericLoc, Expr *ControllingExpr,
-                       ArrayRef<TypeSourceInfo*> AssocTypes,
-                       ArrayRef<Expr*> AssocExprs,
-                       SourceLocation DefaultLoc, SourceLocation RParenLoc,
+  GenericSelectionExpr(const ASTContext &Context, SourceLocation GenericLoc,
+                       Expr *ControllingExpr,
+                       ArrayRef<TypeSourceInfo *> AssocTypes,
+                       ArrayRef<Expr *> AssocExprs, SourceLocation DefaultLoc,
+                       SourceLocation RParenLoc,
                        bool ContainsUnexpandedParameterPack);
 
   explicit GenericSelectionExpr(EmptyShell Empty)
-    : Expr(GenericSelectionExprClass, Empty) { }
+      : Expr(GenericSelectionExprClass, Empty) {}
 
+  /// The number of association expressions.
   unsigned getNumAssocs() const { return NumAssocs; }
 
-  SourceLocation getGenericLoc() const { return GenericLoc; }
-  SourceLocation getDefaultLoc() const { return DefaultLoc; }
-  SourceLocation getRParenLoc() const { return RParenLoc; }
+  /// The zero-based index of the result expression's generic association in
+  /// the generic selection's association list.  Defined only if the
+  /// generic selection is not result-dependent.
+  unsigned getResultIndex() const {
+    assert(!isResultDependent() &&
+           "Generic selection is result-dependent but getResultIndex called!");
+    return ResultIndex;
+  }
 
-  const Expr *getAssocExpr(unsigned i) const {
-    return cast<Expr>(SubExprs[END_EXPR+i]);
+  /// Whether this generic selection is result-dependent.
+  bool isResultDependent() const { return ResultIndex == ResultDependentIndex; }
+
+  /// Return the controlling expression of this generic selection expression.
+  Expr *getControllingExpr() { return cast<Expr>(SubExprs[ControllingIndex]); }
+  const Expr *getControllingExpr() const {
+    return cast<Expr>(SubExprs[ControllingIndex]);
   }
-  Expr *getAssocExpr(unsigned i) { return cast<Expr>(SubExprs[END_EXPR+i]); }
-  ArrayRef<Expr *> getAssocExprs() const {
-    return NumAssocs
-               ? llvm::makeArrayRef(
-                     &reinterpret_cast<Expr **>(SubExprs)[END_EXPR], NumAssocs)
-               : None;
+
+  /// Return the result expression of this controlling expression. Defined if
+  /// and only if the generic selection expression is not result-dependent.
+  Expr *getResultExpr() {
+    return cast<Expr>(SubExprs[AssocExprStartIndex + getResultIndex()]);
   }
-  const TypeSourceInfo *getAssocTypeSourceInfo(unsigned i) const {
-    return AssocTypes[i];
+  const Expr *getResultExpr() const {
+    return cast<Expr>(SubExprs[AssocExprStartIndex + getResultIndex()]);
+  }
+
+  ArrayRef<Expr *> getAssocExprs() const {
+    return {reinterpret_cast<Expr *const *>(SubExprs + AssocExprStartIndex),
+            NumAssocs};
   }
-  TypeSourceInfo *getAssocTypeSourceInfo(unsigned i) { return AssocTypes[i]; }
   ArrayRef<TypeSourceInfo *> getAssocTypeSourceInfos() const {
-    return NumAssocs ? llvm::makeArrayRef(&AssocTypes[0], NumAssocs) : None;
+    return {AssocTypes, NumAssocs};
   }
 
-  QualType getAssocType(unsigned i) const {
-    if (const TypeSourceInfo *TS = getAssocTypeSourceInfo(i))
-      return TS->getType();
-    else
-      return QualType();
+  Expr *getAssocExpr(unsigned i) {
+    return cast<Expr>(SubExprs[AssocExprStartIndex + i]);
   }
-
-  const Expr *getControllingExpr() const {
-    return cast<Expr>(SubExprs[CONTROLLING]);
+  const Expr *getAssocExpr(unsigned i) const {
+    return cast<Expr>(SubExprs[AssocExprStartIndex + i]);
   }
-  Expr *getControllingExpr() { return cast<Expr>(SubExprs[CONTROLLING]); }
-
-  /// Whether this generic selection is result-dependent.
-  bool isResultDependent() const { return ResultIndex == -1U; }
 
-  /// The zero-based index of the result expression's generic association in
-  /// the generic selection's association list.  Defined only if the
-  /// generic selection is not result-dependent.
-  unsigned getResultIndex() const {
-    assert(!isResultDependent() && "Generic selection is result-dependent");
-    return ResultIndex;
+  TypeSourceInfo *getAssocTypeSourceInfo(unsigned i) { return AssocTypes[i]; }
+  const TypeSourceInfo *getAssocTypeSourceInfo(unsigned i) const {
+    return AssocTypes[i];
   }
 
-  /// The generic selection's result expression.  Defined only if the
-  /// generic selection is not result-dependent.
-  const Expr *getResultExpr() const { return getAssocExpr(getResultIndex()); }
-  Expr *getResultExpr() { return getAssocExpr(getResultIndex()); }
+  QualType getAssocType(unsigned i) const {
+    const TypeSourceInfo *TSI = getAssocTypeSourceInfo(i);
+    return TSI ? TSI->getType() : QualType();
+  }
 
-  SourceLocation getBeginLoc() const LLVM_READONLY { return GenericLoc; }
-  SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }
+  SourceLocation getGenericLoc() const { return GenericLoc; }
+  SourceLocation getDefaultLoc() const { return DefaultLoc; }
+  SourceLocation getRParenLoc() const { return RParenLoc; }
+  SourceLocation getBeginLoc() const { return getGenericLoc(); }
+  SourceLocation getEndLoc() const { return getRParenLoc(); }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == GenericSelectionExprClass;
   }
 
   child_range children() {
-    return child_range(SubExprs, SubExprs+END_EXPR+NumAssocs);
+    return child_range(SubExprs, SubExprs + AssocExprStartIndex + NumAssocs);
   }
   const_child_range children() const {
-    return const_child_range(SubExprs, SubExprs + END_EXPR + NumAssocs);
+    return const_child_range(SubExprs,
+                             SubExprs + AssocExprStartIndex + NumAssocs);
   }
-  friend class ASTStmtReader;
 };
 
 //===----------------------------------------------------------------------===//
 // Clang Extensions
 //===----------------------------------------------------------------------===//
 
 /// ExtVectorElementExpr - This represents access to specific elements of a
 /// vector, and may occur on the left hand side or right hand side.  For example

lib/AST/Expr.cpp

 void ShuffleVectorExpr::setExprs(const ASTContext &C, ArrayRef<Expr *> Exprs) {
   if (SubExprs) C.Deallocate(SubExprs);
 
   this->NumExprs = Exprs.size();
   SubExprs = new (C) Stmt*[NumExprs];
   memcpy(SubExprs, Exprs.data(), sizeof(Expr *) * Exprs.size());
 }
 
-GenericSelectionExpr::GenericSelectionExpr(const ASTContext &Context,
-                               SourceLocation GenericLoc, Expr *ControllingExpr,
-                               ArrayRef<TypeSourceInfo*> AssocTypes,
-                               ArrayRef<Expr*> AssocExprs,
-                               SourceLocation DefaultLoc,
-                               SourceLocation RParenLoc,
-                               bool ContainsUnexpandedParameterPack,
-                               unsigned ResultIndex)
-  : Expr(GenericSelectionExprClass,
-         AssocExprs[ResultIndex]->getType(),
+GenericSelectionExpr::GenericSelectionExpr(
+    const ASTContext &Context, SourceLocation GenericLoc, Expr *ControllingExpr,
+    ArrayRef<TypeSourceInfo *> AssocTypes, ArrayRef<Expr *> AssocExprs,
+    SourceLocation DefaultLoc, SourceLocation RParenLoc,
+    bool ContainsUnexpandedParameterPack, unsigned ResultIndex)
+    : Expr(GenericSelectionExprClass, AssocExprs[ResultIndex]->getType(),
            AssocExprs[ResultIndex]->getValueKind(),
            AssocExprs[ResultIndex]->getObjectKind(),
            AssocExprs[ResultIndex]->isTypeDependent(),
            AssocExprs[ResultIndex]->isValueDependent(),
            AssocExprs[ResultIndex]->isInstantiationDependent(),
            ContainsUnexpandedParameterPack),
-    AssocTypes(new (Context) TypeSourceInfo*[AssocTypes.size()]),
-    SubExprs(new (Context) Stmt*[END_EXPR+AssocExprs.size()]),
       NumAssocs(AssocExprs.size()), ResultIndex(ResultIndex),
+      AssocTypes(new (Context) TypeSourceInfo *[AssocTypes.size()]),
+      SubExprs(new (Context) Stmt *[AssocExprStartIndex + AssocExprs.size()]),
       GenericLoc(GenericLoc), DefaultLoc(DefaultLoc), RParenLoc(RParenLoc) {
-  SubExprs[CONTROLLING] = ControllingExpr;
-  assert(AssocTypes.size() == AssocExprs.size());
+  assert(AssocTypes.size() == AssocExprs.size() &&
+         "Must have the same number of association expressions"
+         " and TypeSourceInfo!");
+  assert(ResultIndex < NumAssocs && "ResultIndex is out-of-bounds!");
+
+  SubExprs[ControllingIndex] = ControllingExpr;
   std::copy(AssocTypes.begin(), AssocTypes.end(), this->AssocTypes);
-  std::copy(AssocExprs.begin(), AssocExprs.end(), SubExprs+END_EXPR);
+  std::copy(AssocExprs.begin(), AssocExprs.end(),
+            SubExprs + AssocExprStartIndex);
 }
 
-GenericSelectionExpr::GenericSelectionExpr(const ASTContext &Context,
-                               SourceLocation GenericLoc, Expr *ControllingExpr,
-                               ArrayRef<TypeSourceInfo*> AssocTypes,
-                               ArrayRef<Expr*> AssocExprs,
-                               SourceLocation DefaultLoc,
-                               SourceLocation RParenLoc,
-                               bool ContainsUnexpandedParameterPack)
-  : Expr(GenericSelectionExprClass,
-         Context.DependentTy,
-         VK_RValue,
+GenericSelectionExpr::GenericSelectionExpr(
+    const ASTContext &Context, SourceLocation GenericLoc, Expr *ControllingExpr,
+    ArrayRef<TypeSourceInfo *> AssocTypes, ArrayRef<Expr *> AssocExprs,
+    SourceLocation DefaultLoc, SourceLocation RParenLoc,
+    : Expr(GenericSelectionExprClass, Context.DependentTy, VK_RValue,
            OK_Ordinary,
            /*isTypeDependent=*/true,
            /*isValueDependent=*/true,
-         /*isInstantiationDependent=*/true,
-         ContainsUnexpandedParameterPack),
-    AssocTypes(new (Context) TypeSourceInfo*[AssocTypes.size()]),
-    SubExprs(new (Context) Stmt*[END_EXPR+AssocExprs.size()]),
-    NumAssocs(AssocExprs.size()), ResultIndex(-1U), GenericLoc(GenericLoc),
-    DefaultLoc(DefaultLoc), RParenLoc(RParenLoc) {
-  SubExprs[CONTROLLING] = ControllingExpr;
-  assert(AssocTypes.size() == AssocExprs.size());
+           /*isInstantiationDependent=*/true, ContainsUnexpandedParameterPack),
+      NumAssocs(AssocExprs.size()), ResultIndex(ResultDependentIndex),
+      AssocTypes(new (Context) TypeSourceInfo *[AssocTypes.size()]),
+      SubExprs(new (Context) Stmt *[AssocExprStartIndex + AssocExprs.size()]),
+      GenericLoc(GenericLoc), DefaultLoc(DefaultLoc), RParenLoc(RParenLoc) {
+  assert(AssocTypes.size() == AssocExprs.size() &&
+         "Must have the same number of association expressions"
+         " and TypeSourceInfo!");
+
+  SubExprs[ControllingIndex] = ControllingExpr;
   std::copy(AssocTypes.begin(), AssocTypes.end(), this->AssocTypes);
-  std::copy(AssocExprs.begin(), AssocExprs.end(), SubExprs+END_EXPR);
+  std::copy(AssocExprs.begin(), AssocExprs.end(),
+            SubExprs + AssocExprStartIndex);
 }
 
 //===----------------------------------------------------------------------===//
 //  DesignatedInitExpr
 //===----------------------------------------------------------------------===//
 
 IdentifierInfo *DesignatedInitExpr::Designator::getFieldName() const {
   assert(Kind == FieldDesignator && "Only valid on a field designator");

lib/Serialization/ASTReaderStmt.cpp

 void ASTStmtReader::VisitBlockExpr(BlockExpr *E) {
   VisitExpr(E);
   E->setBlockDecl(ReadDeclAs<BlockDecl>());
 }
 
 void ASTStmtReader::VisitGenericSelectionExpr(GenericSelectionExpr *E) {
   VisitExpr(E);
   E->NumAssocs = Record.readInt();
-  E->AssocTypes = new (Record.getContext()) TypeSourceInfo*[E->NumAssocs];
-  E->SubExprs =
-   new(Record.getContext()) Stmt*[GenericSelectionExpr::END_EXPR+E->NumAssocs];
+  E->ResultIndex = Record.readInt();
+
+  E->AssocTypes = new (Record.getContext()) TypeSourceInfo *[E->NumAssocs];
+  E->SubExprs = new (Record.getContext())
+      Stmt *[GenericSelectionExpr::AssocExprStartIndex + E->NumAssocs];
 
-  E->SubExprs[GenericSelectionExpr::CONTROLLING] = Record.readSubExpr();
+  E->SubExprs[GenericSelectionExpr::ControllingIndex] = Record.readSubExpr();
   for (unsigned I = 0, N = E->getNumAssocs(); I != N; ++I) {
     E->AssocTypes[I] = GetTypeSourceInfo();
-    E->SubExprs[GenericSelectionExpr::END_EXPR+I] = Record.readSubExpr();
+    E->SubExprs[GenericSelectionExpr::AssocExprStartIndex + I] =
+        Record.readSubExpr();
   }
-  E->ResultIndex = Record.readInt();
 
   E->GenericLoc = ReadSourceLocation();
   E->DefaultLoc = ReadSourceLocation();
   E->RParenLoc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitPseudoObjectExpr(PseudoObjectExpr *E) {
   VisitExpr(E);

lib/Serialization/ASTWriterStmt.cpp

void ASTStmtWriter::VisitBlockExpr(BlockExpr *E) {
   VisitExpr(E);
   Record.AddDeclRef(E->getBlockDecl());
   Code = serialization::EXPR_BLOCK;
 }
 
 void ASTStmtWriter::VisitGenericSelectionExpr(GenericSelectionExpr *E) {
   VisitExpr(E);
   Record.push_back(E->getNumAssocs());
+  Record.push_back(E->ResultIndex);
 
   Record.AddStmt(E->getControllingExpr());
   for (unsigned I = 0, N = E->getNumAssocs(); I != N; ++I) {
     Record.AddTypeSourceInfo(E->getAssocTypeSourceInfo(I));
     Record.AddStmt(E->getAssocExpr(I));
   }
-  Record.push_back(E->isResultDependent() ? -1U : E->getResultIndex());
 
   Record.AddSourceLocation(E->getGenericLoc());
   Record.AddSourceLocation(E->getDefaultLoc());
   Record.AddSourceLocation(E->getRParenLoc());
   Code = serialization::EXPR_GENERIC_SELECTION;
 }
 
 void ASTStmtWriter::VisitPseudoObjectExpr(PseudoObjectExpr *E) {