[PATCH] D133468: [clang] Implement divergence for TypedefType and UsingType

[David Rector via Phabricator via cfe-commits]

davrec added a comment.



> I will give an example using a Typedef for exposition, but the same motivation applies with UsingType.
>
> Say we have this code:
>
>   template <class T> struct A { using type1 = T; };
>   using Int = int;
>   
>   using type2 = A<Int>::type1;
>
> See this example live (with that resugaring patch) at: https://godbolt.org/z/jP64Pern8
>
> We want the underlying type of `type2` to have that `Int` sugar. It would also be good if we could keep the TypedefType sugar referencing the instantiation of `type1` from the `A<int>` template.
>
> The problem here is that the TypedefType is currently limited to having the declaration's underlying type, which is just from an instantiation of `A<int>`, and knows nothing about camel case `Int`.
> This is similar to the infamous problem with `std::string` turning into `std::basic_string` in templates.
>
> We could emit a new TypedefDecl with the underlying type that we want, but creating declarations is expensive, so we want to avoid that.
> We could create a new AST node that represented more accurately what was happening. But the question is, is this worth the cost?
> Do you see use cases for this yourself?
>
> We want resugaring to be cheap and introduce as little changes in the AST as possible, to get a better chance of affording to have it to be on all the time.
> If we introduce new nodes with more information, that might increase the cost and complexity, and it's hard to justify without an use case.

This explanation and example are very helpful.  Parts of them probably should make it into the documentation for `isDivergent()` or whatever it is called.

More importantly the documentation needs to emphasize that when `isDivergent()` is true, the underlying type really is more meaningful than the decl, and should be relied on wherever possible.  After all the reason they differ *for now* is that the underlying type may not have an associated decl, simply due to the cost concerns of introducing such a decl.  But it is possible in the future we could change course and decide to introduce the implicit TypedefDecls or UsingDecls.  Then they would no longer be divergent, and it is the underlying decl that would change, not the underlying type.

Whether `isDivergent()` is the right name, vs. `resugared()`, or `hasLessCanonicalizedType()` or `hasMoreSpecificType()` as @erichkeane suggested (all seem reasonable), is less important to me than just documenting that method well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133468