mizvekov added a comment.

In D133468#3801838 <https://reviews.llvm.org/D133468#3801838>, @davrec wrote:

> If I understand this correctly: whenever isDivergent() is true, `getDecl()` 
> or `getFoundDecl()` is an arbitrary choice between multiple candidate 
> (re)declarations of a typedef or using decl.  In particular it will be the 
> first redeclaration, //even though each different redeclaration can contain 
> different sugar information//.

Well that is only true for this particular way, as used in the tests for this 
patch, of creating a divergent TypedefType.

The general idea is just that the TypedefType will have an underlying type 
sugar which is decoupled from the one in the declaration.

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.

> 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.

> The UnderlyingType however will not be as arbitrary, as it will use 
> getCommonType to skip over everything the two don't have in common.

Yes, except that neither is just arbitrary.

> If I have this right, my question is: since the Decl/FoundDecl is an 
> arbitrary choice, should we solve this by either
>
> 1. constructing these with a null Decl/FoundDecl for such types, or if that 
> is problematic,
> 2. renaming `isDivergent` to `declIsArbitrary()` or something like that, to 
> suggest that not only do the underlying type and decl diverge, it is the 
> underlying type which is more meaningful, not the decl.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133468

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to