Support the overloadable attribute with _Generic expressions

[aaron.ballman]

Summary

Currently, using a generic selection expression whose resulting associated expression is a function with the overloadable attribute crashes Clang. This patch supports properly resolving the overloaded function, which fixes PR30201 (https://llvm.org/bugs/show_bug.cgi?id=30201).

[george.burgess.iv]

Just a drive-by nit. Thanks for the patch!

lib/Sema/SemaOverload.cpp

12996

Is there a reason this isn't a SmallVector instead?

[rsmith]

Lgtm other than George's nit.

[aaron.ballman]

Richard accepted, so claiming this as accepted so I can close it.

[aaron.ballman]

Commit in r280483.

[dblaikie]

lib/Sema/SemaOverload.cpp

12996

Another note on this - we should generally prefer copy init over direct init (less power, less responsibility/easier to read):

std::vector<Expr*> AssocExprs = GSE->getAssocExprs().vec();

(& as for George's question: since ArrayRef::vec returns std::vector, it's cheaper to store in a std::vector (by move) than to make a copy into a SmallVector)

[aaron.ballman]

lib/Sema/SemaOverload.cpp

12996

Yeah, I originally used std::vector<> because of ArrayRef's interface. I am happy to go either route, depending on preference, as I doubt this will wind up on the hot path with any regularity.

[george.burgess.iv]

lib/Sema/SemaOverload.cpp

12996

as for George's question: since ArrayRef::vec returns std::vector, it's cheaper to store in a std::vector (by move) than to make a copy into a SmallVector

I was thinking that we would end up using the SmallVector(begin(), end()) ctor instead, so the vector temp wouldn't be needed. :)

Regardless, it was just a nit, so I'm perfectly happy if it stays a vector.

I doubt this will wind up on the hot path with any regularity

Agreed.

[aaron.ballman]

lib/Sema/SemaOverload.cpp

12996

I was thinking that we would end up using the SmallVector(begin(), end()) ctor instead, so the vector temp wouldn't be needed. :)

I went that route, but didn't like requiring a local ArrayRef<Expr *> to prevent calling GSE->getAssocExprs() twice. There really is no convenient way to get a SmallVector from an ArrayRef, so perhaps another option would be to have a SmallVector constructor that accepts an ArrayRef, or have a way to get all of the elements as an llvm::iterator_range directly from an ArrayRef.

include/clang/AST/Expr.h

     return cast<Expr>(SubExprs[END_EXPR+i]);
   }
   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;
+  }
   const TypeSourceInfo *getAssocTypeSourceInfo(unsigned i) const {
     return AssocTypes[i];
   }
   TypeSourceInfo *getAssocTypeSourceInfo(unsigned i) { return AssocTypes[i]; }
+  ArrayRef<TypeSourceInfo *> getAssocTypeSourceInfos() const {
+    return NumAssocs ? llvm::makeArrayRef(&AssocTypes[0], NumAssocs) : None;
+  }
 
   QualType getAssocType(unsigned i) const {
     if (const TypeSourceInfo *TS = getAssocTypeSourceInfo(i))

lib/Sema/SemaOverload.cpp

                                     ICE->getValueKind());
   }
 
+  if (auto *GSE = dyn_cast<GenericSelectionExpr>(E)) {
+    if (!GSE->isResultDependent()) {
+      Expr *SubExpr =
+          FixOverloadedFunctionReference(GSE->getResultExpr(), Found, Fn);
+      if (SubExpr == GSE->getResultExpr())
+        return GSE;
+
+      // Replace the resulting type information before rebuilding the generic
+      // selection expression.
+      std::vector<Expr *> AssocExprs(GSE->getAssocExprs().vec());
+      unsigned ResultIdx = GSE->getResultIndex();
+      AssocExprs[ResultIdx] = SubExpr;
+
+      return new (Context) GenericSelectionExpr(
+          Context, GSE->getGenericLoc(), GSE->getControllingExpr(),
+          GSE->getAssocTypeSourceInfos(), AssocExprs, GSE->getDefaultLoc(),
+          GSE->getRParenLoc(), GSE->containsUnexpandedParameterPack(),
+          ResultIdx);
+    }
+    // Rather than fall through to the unreachable, return the original generic
+    // selection expression.
+    return GSE;
+  }
+
   if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(E)) {
     assert(UnOp->getOpcode() == UO_AddrOf &&
            "Can only take the address of an overloaded function");

test/Sema/generic-selection.c

-// RUN: %clang_cc1 -std=c1x -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify %s
 
 void g(void);
 

   // expression is not evaluated.
   (void)_Generic(*(int *)0, int: 1);
 }
+
+int __attribute__((overloadable)) test (int);
+double __attribute__((overloadable)) test (double);
+char testc(char);
+
+void PR30201(void) {
+  _Generic(4, char:testc, default:test)(4);
+}