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

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov marked 3 inline comments as done.
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.
----------------
mizvekov wrote:
> 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 :)
I added the handling for those being different here, but I don't think it matters for any existing type attribute that I could find.


================
Comment at: clang/lib/AST/ASTContext.cpp:12698-12699
+    return Ctx.getAutoType(Underlying, AX->getKeyword(),
+                           AX->isInstantiationDependentType(),
+                           AX->containsUnexpandedParameterPack(), CD, As);
+  }
----------------
rsmith wrote:
> 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`.
I think in this case, a dependent Auto will never be sugar, and a non-dependent auto can't have a pack either, so we can just pass in false for both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130308