[PATCH] D130308: WIP: [clang] extend getCommonSugaredType to merge sugar nodes

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:12670
+      return QualType();
+    // FIXME: The modified types can be different as well.
+    // FIXME: It's inneficient to have to unify the modified types.
----------------
rsmith wrote:
> Should we bail out if this happens, as we do in the other cases above?
I think this means what we are trying to merge is unrelated, so yes. I just didn't come up with a test case yet :)


================
Comment at: clang/lib/AST/ASTContext.cpp:12695-12696
+    if (CD)
+      As = getCommonTemplateArguments(Ctx, AX->getTypeConstraintArguments(),
+                                      AY->getTypeConstraintArguments());
+    return Ctx.getAutoType(Underlying, AX->getKeyword(),
----------------
rsmith wrote:
> These template arguments might in general be unrelated. Is that a problem here? Eg:
> 
> ```
> template<typename T, template<typename> typename Predicate> concept satisfies = Predicate<T>::value;
> auto f(bool c) {
>   satisfies<std::is_trivially_copyable> auto x = 0;
>   satisfies<std::is_integral> auto y = 0;
>   // Common sugar here should presumably be "unconstrained auto deduced as `int`".
>   return c ? x : y;
> }
> ```
Right, yes, we should bail out in that case as well I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130308