aaron.ballman added reviewers: rjmccall, rsmith, erichkeane. aaron.ballman added a comment.
In D120296#3343673 <https://reviews.llvm.org/D120296#3343673>, @yonghong-song wrote: > @aaron.ballman ping, did you get time to look at this patch? Sorry, I was in C meetings all last week, still digging out from under the emails that piled up. I think the approach taken here is likely incorrect. Not all `AttributedType` objects need this "extra info" as a string, and there's no reason to believe future attributes will want their extra info to be in the form of a string. However, there's also a fundamental flaw -- the `AttributedType` isn't tracking that extra information, so it cannot be profiled in all circumstances (I left a comment in the review for that). As best I can tell, you want a different attribute argument to result in a different type in the type system. That needs to happen by adding a new type to the type system instead of using the generic `AttributedType` type. This new type can track the extra information needed for uniquing the type within the type system. As an example, `VectorType` is formed via an attribute, but it needs additional information about what kind of vector it is. I don't think you can do what you're trying in this patch because the `AttributedType` tracks the attribute *kind* but not the actual semantic attribute in use. You need an `AttributedTypeLoc` to get access to that via the type system, and you don't always have access to type locations. ================ Comment at: clang/include/clang/AST/Type.h:4774 void Profile(llvm::FoldingSetNodeID &ID) { Profile(ID, getAttrKind(), ModifiedType, EquivalentType); } ---------------- This form of type profiling was not updated (and can't be, because there's nothing tracking what extra info is associated with the type), which further suggests we need a new type in the type system. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120296/new/ https://reviews.llvm.org/D120296 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
