[PATCH] D114235: [clang] Extend ParsedAttr to allow custom handling for type attributes

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 11 07:41:25 PST 2022


aaron.ballman added a comment.

First off, thank you so much for your patience while I took the time to think about this. I know it can be frustrating to not hear review feedback in a timely manner, so I'm sorry for that.

I've been sitting on this for a while because type attributes are a complex beast. There's the attribute parsing component to it, which is generally pretty straightforward and is largely what's handled here in your patch. But the bigger issue is the effects on the type system, which is one of the only ways to make a type attribute particularly useful. Unfortunately, type system effects are scattered all over the code base and can have drastic impact on compiler performance (template instantiation depth limits, memory overhead pressures, etc), and we tend to have quite a few assertions in the compiler to help users catch the places they need to update.

Given that utility from this would require significant changes in Clang itself, I'm not certain what a plugin buys us in terms of functionality compared to the risk of type attribute plugins making the compiler somewhat unstable in terms of performance and potentially even assertions, etc. tl;dr: type attributes aren't easily pluggable yet so by the time you make them useful, you basically have a clang fork instead of a plugin augmenting Clang. Based on that, I'm inclined to not ask you to do further work on it (that work would be akin to me asking you "please rework large parts of the type system" which would be a high risk activity and not a reasonable request as part of this patch). However, I also wanted to hear what your ideas are on how you were expecting to use the functionality here from the patch, as maybe there's utility that I'm not thinking of.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114235



More information about the cfe-commits mailing list