[ASTContext] Use AllowCXX in all merge*Type methods, strip references

[jdoerfert]

Summary

Add and pass AllowCXX not only in mergeFunctionTypes but all merge*Type
methods. Use it to determine if reference types should be stripped or
are invalid in mergeTypes. The reason for this is that
mergeFunctionTypes allows CXX functions with exception specifiers but
crashes on reference types (in the return or argument position). This is
going to be used by D88384.

[aaron.ballman]

Do you have test cases for this change? I didn't see any relevant ones in D88384.

clang/lib/AST/ASTContext.cpp

9174

It seems possible to call typesAreBlockPointerCompatible() in C++ mode (ASTContext::areCommonBaseCompatible() calls sameObjCTypeArgs() which calls canAssignObjCObjectTypes() which calls ASTContext::typesAreBlockPointerCompatible(). I'm not certain if the assertion will actually trigger though, so tests would be appreciated.

Are there other cases where mergeType() can be transitively called in C++ mode without having already stripped references?

9430–9433

This will try to merge a T& and T&& based on T alone, is that expected or should this be caring about lvalue vs rvalue reference types?

[jdoerfert]

In D88491#2306099, @aaron.ballman wrote:

Do you have test cases for this change? I didn't see any relevant ones in D88384.

In the revert (rG4fc69ab002382675d84f611f22599cb3cb4a0787) I noted a test broke. I went back and it is clang/test/Headers/nvptx_device_math_complex.cpp which is already in tree.
I'll try to gracefully accept but reject reference types next. That might work.

clang/lib/AST/ASTContext.cpp

9174

AllowCXX is only used from OpenMP code and probably named badly. The idea is that we want to allow roughly matching declarations, so far that meant we don't want to be too strict on exception qualifiers.

9430–9433

This is debatable. The standard is all but clear about this and it is not 100% clear to me what is reasonable. As noted in the general comment, we might be able to get all the functionality we need by gracefully handling this case and saying it is not mergable. I'll give that a try.

[jdoerfert]

Changing the assume to a proper exit did the trick for the tests. I recommited D88384 with that minor modification. Apologies for the noise.

clang/include/clang/AST/ASTContext.h

bool canAssignObjCInterfacesInBlockPointer(
                                           const ObjCObjectPointerType *RHSOPT,
                                           bool BlockReturnType);
   bool areComparableObjCPointerTypes(QualType LHS, QualType RHS);
   QualType areCommonBaseCompatible(const ObjCObjectPointerType *LHSOPT,
                                    const ObjCObjectPointerType *RHSOPT);
   bool canBindObjCObjectType(QualType To, QualType From);
 
   // Functions for calculating composite types
-  QualType mergeTypes(QualType, QualType, bool OfBlockPointer=false,
-                      bool Unqualified = false, bool BlockReturnType = false);
-  QualType mergeFunctionTypes(QualType, QualType, bool OfBlockPointer=false,
+  QualType mergeTypes(QualType, QualType, bool OfBlockPointer = false,
+                      bool Unqualified = false, bool BlockReturnType = false,
+                      bool AllowCXX = false);
+  QualType mergeFunctionTypes(QualType, QualType, bool OfBlockPointer = false,
                               bool Unqualified = false, bool AllowCXX = false);
   QualType mergeFunctionParameterTypes(QualType, QualType,
                                        bool OfBlockPointer = false,
-                                       bool Unqualified = false);
+                                       bool Unqualified = false,
+                                       bool AllowCXX = false);
   QualType mergeTransparentUnionType(QualType, QualType,
-                                     bool OfBlockPointer=false,
-                                     bool Unqualified = false);
+                                     bool OfBlockPointer = false,
+                                     bool Unqualified = false,
+                                     bool AllowCXX = false);
 
   QualType mergeObjCGCQualifiers(QualType, QualType);
 
   /// This function merges the ExtParameterInfo lists of two functions. It
   /// returns true if the lists are compatible. The merged list is returned in
   /// NewParamInfos.
   ///
   /// \param FirstFnType The type of the first function.

clang/lib/AST/ASTContext.cpp

bool ASTContext::typesAreCompatible(QualType LHS, QualType RHS,
   return !mergeTypes(LHS, RHS, false, CompareUnqualified).isNull();
 }
 
 bool ASTContext::propertyTypesAreCompatible(QualType LHS, QualType RHS) {
   return typesAreCompatible(LHS, RHS);
 }
 
 bool ASTContext::typesAreBlockPointerCompatible(QualType LHS, QualType RHS) {
   return !mergeTypes(LHS, RHS, true).isNull();
 }
 
 /// mergeTransparentUnionType - if T is a transparent union type and a member
 /// of T is compatible with SubType, return the merged type, else return
 /// QualType()
 QualType ASTContext::mergeTransparentUnionType(QualType T, QualType SubType,
                                                bool OfBlockPointer,
-                                               bool Unqualified) {
+                                               bool Unqualified,
+                                               bool AllowCXX) {
   if (const RecordType *UT = T->getAsUnionType()) {
     RecordDecl *UD = UT->getDecl();
     if (UD->hasAttr<TransparentUnionAttr>()) {
       for (const auto *I : UD->fields()) {
         QualType ET = I->getType().getUnqualifiedType();
-        QualType MT = mergeTypes(ET, SubType, OfBlockPointer, Unqualified);
+        QualType MT = mergeTypes(ET, SubType, OfBlockPointer, Unqualified,
+                                 /* BlockReturnType */ false, AllowCXX);
         if (!MT.isNull())
           return MT;
       }
     }
   }
 
   return {};
 }
 
 /// mergeFunctionParameterTypes - merge two types which appear as function
 /// parameter types
 QualType ASTContext::mergeFunctionParameterTypes(QualType lhs, QualType rhs,
                                                  bool OfBlockPointer,
-                                                 bool Unqualified) {
+                                                 bool Unqualified,
+                                                 bool AllowCXX) {
   // GNU extension: two types are compatible if they appear as a function
   // argument, one of the types is a transparent union type and the other
   // type is compatible with a union member
   QualType lmerge = mergeTransparentUnionType(lhs, rhs, OfBlockPointer,
-                                              Unqualified);
+                                              Unqualified, AllowCXX);
   if (!lmerge.isNull())
     return lmerge;
 
   QualType rmerge = mergeTransparentUnionType(rhs, lhs, OfBlockPointer,
-                                              Unqualified);
+                                              Unqualified, AllowCXX);
   if (!rmerge.isNull())
     return rmerge;
 
-  return mergeTypes(lhs, rhs, OfBlockPointer, Unqualified);
+  return mergeTypes(lhs, rhs, OfBlockPointer, Unqualified,
+                    /* BlockReturnType */ false, AllowCXX);
 }
 
 QualType ASTContext::mergeFunctionTypes(QualType lhs, QualType rhs,
                                         bool OfBlockPointer, bool Unqualified,
                                         bool AllowCXX) {
   const auto *lbase = lhs->castAs<FunctionType>();
   const auto *rbase = rhs->castAs<FunctionType>();
   const auto *lproto = dyn_cast<FunctionProtoType>(lbase);
   const auto *rproto = dyn_cast<FunctionProtoType>(rbase);
   bool allLTypes = true;
   bool allRTypes = true;
 
   // Check return type
   QualType retType;
   if (OfBlockPointer) {
     QualType RHS = rbase->getReturnType();
     QualType LHS = lbase->getReturnType();
     bool UnqualifiedResult = Unqualified;
     if (!UnqualifiedResult)
       UnqualifiedResult = (!RHS.hasQualifiers() && LHS.hasQualifiers());
-    retType = mergeTypes(LHS, RHS, true, UnqualifiedResult, true);
-  }
-  else
+    retType = mergeTypes(LHS, RHS, true, UnqualifiedResult,
+                         /* BlockReturnType */ true, AllowCXX);
+  } else
     retType = mergeTypes(lbase->getReturnType(), rbase->getReturnType(), false,
-                         Unqualified);
+                         Unqualified, /* BlockReturnType */ false, AllowCXX);
   if (retType.isNull())
     return {};
 
   if (Unqualified)
     retType = retType.getUnqualifiedType();
 
   CanQualType LRetType = getCanonicalType(lbase->getReturnType());
   CanQualType RRetType = getCanonicalType(rbase->getReturnType());
if (!canUseRight)
       allRTypes = false;
 
     // Check parameter type compatibility
     SmallVector<QualType, 10> types;
     for (unsigned i = 0, n = lproto->getNumParams(); i < n; i++) {
       QualType lParamType = lproto->getParamType(i).getUnqualifiedType();
       QualType rParamType = rproto->getParamType(i).getUnqualifiedType();
       QualType paramType = mergeFunctionParameterTypes(
-          lParamType, rParamType, OfBlockPointer, Unqualified);
+          lParamType, rParamType, OfBlockPointer, Unqualified, AllowCXX);
       if (paramType.isNull())
         return {};
 
       if (Unqualified)
         paramType = paramType.getUnqualifiedType();
 
       types.push_back(paramType);
       if (Unqualified) {
static QualType mergeEnumWithInteger(ASTContext &Context, const EnumType *ET,
   // integral type of the same size.
   if (isBlockReturnType && other->isIntegerType() &&
       Context.getTypeSize(underlyingType) == Context.getTypeSize(other))
     return other;
 
   return {};
 }
 
-QualType ASTContext::mergeTypes(QualType LHS, QualType RHS,
-                                bool OfBlockPointer,
-                                bool Unqualified, bool BlockReturnType) {
+QualType ASTContext::mergeTypes(QualType LHS, QualType RHS, bool OfBlockPointer,
+                                bool Unqualified, bool BlockReturnType,
+                                bool AllowCXX) {
+  // If AllowCXX is specifically set we strip reference types on demand.
+  assert((AllowCXX ||
+          (!LHS->getAs<ReferenceType>() && !RHS->getAs<ReferenceType>())) &&
+         "C++ references should have been stripped");
+  if (const ReferenceType *lRT = LHS->getAs<ReferenceType>())
+    LHS = lRT->getPointeeType();
+  if (const ReferenceType *rRT = RHS->getAs<ReferenceType>())
+    RHS = rRT->getPointeeType();
+
   // C++ [expr]: If an expression initially has the type "reference to T", the
   // type is adjusted to "T" prior to any further analysis, the expression
   // designates the object or function denoted by the reference, and the
   // expression is an lvalue unless the reference is an rvalue reference and
   // the expression is a function call (possibly inside parentheses).
   assert(!LHS->getAs<ReferenceType>() && "LHS is a reference type?");
   assert(!RHS->getAs<ReferenceType>() && "RHS is a reference type?");
 
#include "clang/AST/TypeNodes.inc"
   {
     // Merge two pointer types, while trying to preserve typedef info
     QualType LHSPointee = LHS->castAs<PointerType>()->getPointeeType();
     QualType RHSPointee = RHS->castAs<PointerType>()->getPointeeType();
     if (Unqualified) {
       LHSPointee = LHSPointee.getUnqualifiedType();
       RHSPointee = RHSPointee.getUnqualifiedType();
     }
-    QualType ResultType = mergeTypes(LHSPointee, RHSPointee, false,
-                                     Unqualified);
+    QualType ResultType = mergeTypes(LHSPointee, RHSPointee, false, Unqualified,
+                                     BlockReturnType, AllowCXX);
     if (ResultType.isNull())
       return {};
     if (getCanonicalType(LHSPointee) == getCanonicalType(ResultType))
       return LHS;
     if (getCanonicalType(RHSPointee) == getCanonicalType(ResultType))
       return RHS;
     return getPointerType(ResultType);
   }
if (getLangOpts().OpenCL) {
       LHSPteeQual.removeAddressSpace();
       RHSPteeQual.removeAddressSpace();
       LHSPointee =
           QualType(LHSPointee.getTypePtr(), LHSPteeQual.getAsOpaqueValue());
       RHSPointee =
           QualType(RHSPointee.getTypePtr(), RHSPteeQual.getAsOpaqueValue());
     }
     QualType ResultType = mergeTypes(LHSPointee, RHSPointee, OfBlockPointer,
-                                     Unqualified);
+                                     Unqualified, BlockReturnType, AllowCXX);
     if (ResultType.isNull())
       return {};
     if (getCanonicalType(LHSPointee) == getCanonicalType(ResultType))
       return LHS;
     if (getCanonicalType(RHSPointee) == getCanonicalType(ResultType))
       return RHS;
     return getBlockPointerType(ResultType);
   }
   case Type::Atomic:
   {
     // Merge two pointer types, while trying to preserve typedef info
     QualType LHSValue = LHS->castAs<AtomicType>()->getValueType();
     QualType RHSValue = RHS->castAs<AtomicType>()->getValueType();
     if (Unqualified) {
       LHSValue = LHSValue.getUnqualifiedType();
       RHSValue = RHSValue.getUnqualifiedType();
     }
-    QualType ResultType = mergeTypes(LHSValue, RHSValue, false,
-                                     Unqualified);
+    QualType ResultType = mergeTypes(LHSValue, RHSValue, false, Unqualified,
+                                     BlockReturnType, AllowCXX);
     if (ResultType.isNull())
       return {};
     if (getCanonicalType(LHSValue) == getCanonicalType(ResultType))
       return LHS;
     if (getCanonicalType(RHSValue) == getCanonicalType(ResultType))
       return RHS;
     return getAtomicType(ResultType);
   }
   case Type::ConstantArray:
   {
     const ConstantArrayType* LCAT = getAsConstantArrayType(LHS);
     const ConstantArrayType* RCAT = getAsConstantArrayType(RHS);
     if (LCAT && RCAT && RCAT->getSize() != LCAT->getSize())
       return {};
 
     QualType LHSElem = getAsArrayType(LHS)->getElementType();
     QualType RHSElem = getAsArrayType(RHS)->getElementType();
     if (Unqualified) {
       LHSElem = LHSElem.getUnqualifiedType();
       RHSElem = RHSElem.getUnqualifiedType();
     }
 
-    QualType ResultType = mergeTypes(LHSElem, RHSElem, false, Unqualified);
+    QualType ResultType = mergeTypes(LHSElem, RHSElem, false, Unqualified,
+                                     BlockReturnType, AllowCXX);
     if (ResultType.isNull())
       return {};
 
     const VariableArrayType* LVAT = getAsVariableArrayType(LHS);
     const VariableArrayType* RVAT = getAsVariableArrayType(RHS);
 
     // If either side is a variable array, and both are complete, check whether
     // the current dimension is definite.