vsapsai added inline comments.

================
Comment at: clang/lib/Sema/SemaType.cpp:266
+        auto *NewAttrTy = cast<AttributedType>(T.getTypePtr());
+        for (TypeAttrPair &A : AttrsForTypes) {
+          if (A.first == AttrTy)
----------------
jkorous wrote:
> Do you think it would make sense to terminate the loop early if 
> `AttrsForTypesSorted == true`?
I agree with you it would be nice to reuse `AttrsForTypesSorted` property but 
in practice it doesn't matter. We sort `AttrsForTypes` only in 
`takeAttrForAttributedType` and we should do that after we've populated 
`AttrsForTypes` and after we've replaced `auto` types. It isn't strictly 
required but that's how it was working in debugger. Also I've tried to add 
`assert(! AttrsForTypesSorted);` and it wasn't triggered during `ninja 
check-clang`

Having `AttrsForTypes` sorted can be useful in the future but I don't think we 
should add code for that now.

Despite the linear iteration, we are not pessimizing the most common use case 
with no attributes for lambda parameters (expression "most common use case" is 
based on my experience and not substantiated by any data). So I think the 
performance shouldn't degrade noticeably.


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

https://reviews.llvm.org/D58659



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

Reply via email to