rapidsna wrote: > It's generally not a good idea to use sugar to represent constructs that are > semantically significant: anything that uses the canonical type will throw it > away. We try to avoid throwing away sugar in most cases, but we aren't always > successful.
> It's possible there are other considerations here (maybe it matters less if > the attribute is only relevant inside the definition?), but I'd like an > explanation of the tradeoffs. Thanks @efriedma-quic! Yes, these are really good points. The main reason that we decided it to be a sugar type was that this type of attribute doesn't change the shape of the underlying type so we wanted such as `isa<PointerType>(BoundAttributedTy)` or `isa<ArrayType>(BoundAttributedTy)` still holds true depending on the underlying canonical type (in the current implementation the attribute only applies to array type but it will be extended to support pointer type as well). And we figured that adding a sugar type might less disruptive to the rest of Clang code base compared to extending the existing Clang types. But as you pointed out we encountered cases where the sugar wasn't preserved as it needed to be so we had to specially handle such cases in our own implementation, so we're open for other suggestions. An alternative design could be to create new canonical types that inherit `IncompleteArrayType` or `PointerType` respectively so `isa` still holds, but unfortunately `IncompleteArrayType` is `final`. We could also add an additional field in the `IncompleteArrayType` class to contain the optional `count` expression, then the question is what we want to do for `ConstantArrayType` later when we like to have it count attribute as well (this could be potentially relevant for flexible array member-like array member with a constant size). Similarly, we could add more fields to `PointerType` to contain the optional count expression (or upper bound expression) and necessary data. For this, we should be careful not to grow the size of `PointerType` by default, possibly by adding additional data as `TrailingObject`. We haven't experimented these alternative designs so I'm curious about @AaronBallman's thoughts. https://github.com/llvm/llvm-project/pull/78000 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
