mizvekov added a comment. In D133468#3799475 <https://reviews.llvm.org/D133468#3799475>, @erichkeane wrote:
> I'm still having trouble from the description and test figuring out what this > means. Can you better clarify the intent/effect of this? The effect is that you can effectively have a TypedefType/UsingType that has an underlying type that is not identical to the one which is held in the TypedefDecl/UsingDecl. Another way we could achieve this, though I am saying it just for exposition as it wouldn't be good, would be by emitting a redeclaration with the type we want. The reason that is not good is because redeclarations are expensive to create and they are not uniqued, and they would waste a lot of memory. Want we really want this for is resugaring: A TypedefType can have a dependent type, and then when an instantiation of that is created, it will have a Type substitution node that only references the canonical type of the template argument used to instantiate it. With resugaring, we can replace that underlying type with one referencing the original argument, and would thus necessitate to "change" the underlying type. We don't want to emit redeclarations during resugaring as that is too expensive, as I explained. Since resugaring is not implemented before this patch, and in order to test this functionality, we use a nifty trick that became possible after we merged D130308 <https://reviews.llvm.org/D130308>: We can unify two different TypedefTypes/UsingTypes that refer to different TypedefDecl/UsingDecl redeclarations (as written in source), each redeclaration having a non-identical type, which after unification becomes yet a different type, which we will use as the underlying type of our unified TypedefType/UsingType. > Is the point that the type-aliases could also store a non-canonicalized type? > if so, perhaps the patch should better reflect that (AND, perhaps do some > assertions to make sure we don't unintentionally store types that don't > canonicalize identically). They can store a separate QualType which must be the SameType as their canonical type, yes. The assertion is already in place for both TypedefType and UsingType constructors. ================ Comment at: clang/include/clang/AST/Type.h:1802 + /// True if the underlying diverges from the declared one. + unsigned isDivergent : 1; + }; ---------------- erichkeane wrote: > It isn't clear to me what you mean by 'diverges' and 'divergent here. I ate the `type`, will fix that, but what I mean here is that a TypedefType/UsingType, without this patch, can only have the same type as is declared in the declaration it references. With this patch, if the UnderlyingType passed in the constructor is different from the one that is declared, then the node will be able to hold this different type, and we will say that the node is 'divergent', which is the term I came up with, synonymous with different. ================ Comment at: clang/lib/AST/Type.cpp:3443 + if (isDivergent()) + *reinterpret_cast<QualType *>(this + 1) = Underlying; } ---------------- erichkeane wrote: > this bit doesn't seem right to me. Why isn't this using the normal setter > here? Or even the trailing objects setter? Well there isn't supposed to be a normal setter, but right, I forgot to update this to use the trailing objects setter. 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
