[PATCH] D56959: [AST] NFC: Introduce new class GenericSelectionExpr::Association

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/AST/Expr.h:5068
 
+  Association getAssociation(unsigned I) const {
+    return Association(cast<Expr>(SubExprs[END_EXPR + I]), AssocTypes[I],
----------------
Rather than gin these objects up on demand every time they're needed, I'd prefer to see the class store `Association` objects directly. I don't think it will be easy to do that and still support `getAssocExprs()` and `getAssocTypeSourceInfos()`, so I think those APIs should be removed in favor of this one. There's currently not many uses of `getAssocExprs()` or `getAssocTypeSourceInfos()` (I spot one each in Clang) so migration to the new API should not be onerous.

This should also have a range-based accessor version so that users aren't required to use iterative loops to access the information (a lot of the places you're already touching could use that range-based interface).


================
Comment at: lib/AST/ASTDumper.cpp:1467
     dumpChild([=] {
-      if (const TypeSourceInfo *TSI = E->getAssocTypeSourceInfo(I)) {
+      const auto Assoc = E->getAssociation(I);
+      const TypeSourceInfo *TSI = Assoc.getTypeSourceInfo();
----------------
Don't use `auto` here.


================
Comment at: lib/AST/StmtPrinter.cpp:1266
     OS << ", ";
-    QualType T = Node->getAssocType(i);
+    auto Assoc = Node->getAssociation(i);
+    QualType T = Assoc.getType();
----------------
Don't use `auto`.


================
Comment at: lib/AST/StmtProfile.cpp:1263
   for (unsigned i = 0; i != S->getNumAssocs(); ++i) {
-    QualType T = S->getAssocType(i);
+    auto Assoc = S->getAssociation(i);
+    QualType T = Assoc.getType();
----------------
Or here (or anywhere similar in this patch; I'll stop commenting on them).


Repository:
  rC Clang

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

https://reviews.llvm.org/D56959