yonghong-song added inline comments.
================
Comment at: clang/include/clang/AST/Type.h:4793
+
+ QualType ModifiedType;
+ QualType EquivalentType;
----------------
erichkeane wrote:
> I suspect both of these aren't necessary. If the intent here is to just wrap
> a single type, I think we can have only 1. Then the fields here are just the
> Tag, and the WrappedType.
I didn't find WrappedType in clang code base. Maybe I missed something?
But I understand what you mean. Here, unlike AttributedType which needs to deal
with attributes which may encode as qualifiers, BTFTagAttributedType won't have
this issue so one 'WrappedType' (representing underlying type of the btf_tag
attribute) should be enough.
================
Comment at: clang/include/clang/Serialization/ASTRecordReader.h:131
+ StringRef readStringRef() { return readString(); }
+
----------------
erichkeane wrote:
> This is a bug. readString returns a std::string temporary, which creating
> the StringRef to, it now no longer exists.
Will fix.
================
Comment at: clang/lib/Sema/SemaType.cpp:194
+ std::pair<const BTFTagAttributedType*, const Attr*>;
+ SmallVector<BTFTagTypeAttrPair, 8> AttrsForBTFTagTypes;
+ bool AttrsForBTFTagTypesSorted = true;
----------------
erichkeane wrote:
> This seems pretty absurdly expensive here... Should we instead have the
> BTFTagAttributedType store its Attr here?
Indeed this is a good idea. This can simplify the code a lot.
================
Comment at: clang/lib/Sema/TreeTransform.h:6872
+ const BTFTagAttributedType *oldType = TL.getTypePtr();
+ StringRef Tag = oldType->getTag();
+ QualType modifiedType = getDerived().TransformType(TLB, TL.getModifiedLoc());
----------------
erichkeane wrote:
> Most of this tree-transform doesn't really do much, since this is a 'C' only
> type, but otherwise we'd probably want to allow the tag itself to be
> dependent.
>
> We still need this though, since there are other non-template tree-transforms.
>
> You also might need to make this not contain a `StringRef` based on the
> serialization issues above.
I will try to do some experiment and simplify this. Indeed this is C and non
templates are involved.
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
