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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Do you have test cases for this change? I didn't see any relevant ones in D88384 <https://reviews.llvm.org/D88384>.



================
Comment at: clang/lib/AST/ASTContext.cpp:9174
 bool ASTContext::typesAreBlockPointerCompatible(QualType LHS, QualType RHS) {
   return !mergeTypes(LHS, RHS, true).isNull();
 }
----------------
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?


================
Comment at: clang/lib/AST/ASTContext.cpp:9430-9433
+  if (const ReferenceType *lRT = LHS->getAs<ReferenceType>())
+    LHS = lRT->getPointeeType();
+  if (const ReferenceType *rRT = RHS->getAs<ReferenceType>())
+    RHS = rRT->getPointeeType();
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88491