AaronBallman wrote: > It's not just Abseil, other code uses it too. Some code uses a > `CAPTURED_BY(foo)` macro like Abseil, some just uses > `[[clang::lifetime_capture_by(this)]]` directly. > > All of that code will become messier by splitting the macro into separate > `CAPTURED_BY_THIS` etc. and gating on `__has_cpp_attribute(x)`. > > I haven't followed this PR from the start, but are we really sure we want to > split this attribute into multiple ones? That seems much harder to deal with > for portable code. Is it worth all the churn it will cause?
Yes; the existing attributes didn't get a lot of broad community review and have some design issues that are being addressed post-release. The churn is unfortunate and could have been avoided with more eyeballs on the design, but it's also necessary IMO otherwise we'll ossify these design mistakes. FWIW, I continue to disagree that these should ever be allowed in a type position given that they only have declaration semantics. That's another significant design change I'd like to see made eventually for the same reasons as making these changes, but it will also be disruptive. > If you really do want to split it, how about this as a rollout plan: > > 1. Land the new attribute now > > 2. Deprecate the old spelling _after_ the Clang 24 version bump (should > be pretty soon), and with a separate -Wdeprecated-captured-by-this flag I'm not certain that we should add a separate deprecation flag; the reasoning for the group applies to every thing we deprecate. The point of deprecation warnings isn't to make it trivial for the user to disable them, it's to annoy the user into updating their code so that the deprecated thing can eventually be removed. https://github.com/llvm/llvm-project/pull/196635 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
