sammccall added a comment. TL;DR: let's go with this patch as-is rather than borrowing problems from the future. This enum isn't growing very rapidly.
In D134728#3822775 <https://reviews.llvm.org/D134728#3822775>, @ckandeler wrote: > In D134728#3822653 <https://reviews.llvm.org/D134728#3822653>, @sammccall > wrote: > >> It doesn't scale very well though: we're limited to 30 modifiers in total, >> this patch brings us up to 16, if we followed this class.constructor >> precedent for function.operator, class.constructor.copy enum.scoped etc we >> could end up exhausting this. BTW, I remembered the other idea that could eat a bunch of the modifiers: rainbow highlighting a la https://github.com/clangd/clangd/issues/889. The idea is you could encode e.g. an 8 bit hash of the symbol identity as the presence/absence of 8 modifiers. But we never shipped it, and I suspect 8 would be enough. > In light of this (and assuming it really has to be this way) FWIW the reason is that modifiers are encoded as an integer, and the spec says integers are <2^31-1 for interop purposes. (I'm not sure why they didn't go for 2^53-1, everyone has either doubles or 64-bit integers right? sigh). > would it make sense to introduce a generic modifier that changes its meaning > depending on the main highlighting kind? It would have the drawback of not > being self-documenting Neat idea. That's workable at a technical level, but requires editors/themes to be pretty tightly coupled to this idiosyncracy of clangd. So I'm not sure it's a good fit for the loosely-coupled LSP ecosystem. > but with the given constraints might just be a pragmatic solution Yeah, however the constraints aren't very tight at this point, I was just thinking out loud a bit. Let's go for the simple thing for now - we may never need something cleverer, and if we do then we'll have more examples to drive the design. I think that's worth the potential breaking change. > // Class -> Constructor/Destructor We'd need Generic1 and Generic2 so you can distinguish constructor/destructor right? If the distinction isn't important, maybe we should add a single "constructor or destructor" modifier... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134728/new/ https://reviews.llvm.org/D134728 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
