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

[David Rector via Phabricator via cfe-commits]

davrec added a subscriber: sammccall.
davrec added a comment.

> In general, forgetting about just the above particular way of creating them, there might still exist only one TypedefDecl in the whole program, which every TypedefType is referencing, and they can still be divergent, ie have different sugared underlying type.

Can you give a code example demonstrating such a case, and any other distinctive cases where isDivergent() will be true?

>> But these types are a different matter:
>>
>>   auto t29 = 0 ? (RPX1){} : (RPY1){};
>>   auto t32 = 0 ? (UPX1){} : (UPY1){};
>>
>> In the process of unifying the sugar to get the deduced type of t29/t32, we will compare their sugar, which has the same basic structure for each, but different decls/found decls; so unifying these types will simply involve unifying their decls; and getCommonDecl(X,Y) has been defined to will simply get the older declaration between them.  So, the TypedefType/UsingType sugar in t29/t32 will reference the X1 decls and friends, *not* the Y1s, as its Decls/FoundDecls.  (Right?)
>
> But again, we are discussing one particular mechanism of creating these divergent types.
> How that mechanism works was defined by a previous patch.
>
> But I don't think the choice between declarations is arbitrary. That type unification mechanism has one basic principle, when two properties of a type are different, we pick the one that is closest to the canonical property, (except in some very special circumstances where the standard requires something different).
> So the choice for the older decl is principled, the canonical decl will be the oldest one. If that wasn't the case, we would need some other choice.



> I don't think the declaration picked is arbitrary, and again they are the "Same" declaration, so picking null would be throwing out more information than is necessary for no principled reason.

Hmm.  Redeclarations never change semantic information, and in general don't change sugar information, so the older one is indeed the common one from any principled standpoint - but as your test cases demonstrate, in the case of typedef and using-decl redeclarations, the sugar information can be changed.  Does this result in some arbitrariness in choosing the older decl as the common decl?  My first instinct is that it does.  But as I think about it further...I think I agree with you, it is still principled to choose the earlier using-decl/typedef decl (though an argument can be made for choosing the very first decl, not just the earlier one, for typedefs and using-decls).

But, if we agree the decl choice is principled, the next question: what is the different principle behind choosing a different underlying type?   Why can't the underlying type for `t29`/`t32` be sugar for X1 and friends too?

To state the issue more practically: when users of the AST see isDivergent()==true, what do they do?  Which path should they take to fetch information from this node?

If we absolutely have to allow these types to diverge, we need very good documentation that helps them answer this.

Wrangling in @sammccall here since he developed `UsingType` and so might be affected/need clarity on how to handle `isDivergent()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133468