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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:12654
+      return QualType();
+    // FIXME: It's inneficient to have to unify the original types.
+    return Ctx.getAdjustedType(Ctx.getCommonSugaredType(OX, OY), Underlying);
----------------
Typo "inefficient" (multiple instances)


================
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.
----------------
Should we bail out if this happens, as we do in the other cases above?


================
Comment at: clang/lib/AST/ASTContext.cpp:12695-12696
+    if (CD)
+      As = getCommonTemplateArguments(Ctx, AX->getTypeConstraintArguments(),
+                                      AY->getTypeConstraintArguments());
+    return Ctx.getAutoType(Underlying, AX->getKeyword(),
----------------
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;
}
```


================
Comment at: clang/lib/AST/ASTContext.cpp:12698-12699
+    return Ctx.getAutoType(Underlying, AX->getKeyword(),
+                           AX->isInstantiationDependentType(),
+                           AX->containsUnexpandedParameterPack(), CD, As);
+  }
----------------
Instantiation-dependence and "contains unexpanded parameter pack" can depend on which sugar we choose to preserve. I think you strictly-speaking would need to recompute these based on the sugar we end up with rather than inheriting them from `AX`.


================
Comment at: clang/lib/AST/ASTContext.cpp:12767-12769
+    if (!declaresSameEntity(DX, DY))
+      return QualType();
+    return Ctx.getUsingType(::getCommonDecl(DX, DY), Underlying);
----------------
Can avoid a repeated check here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130308