aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/SemaType.cpp:7727 + // Move function type attribute to the declarator. + case ParsedAttr::AT_LifetimeContract: ---------------- xazax.hun wrote: > aaron.ballman wrote: > > xazax.hun wrote: > > > aaron.ballman wrote: > > > > This is not an okay thing to do for C++ attributes. They have a > > > > specific meaning as to what they appertain to and do not move around to > > > > better subjects like GNU-style attributes. > > > Yeah, I know that. But this is problematic in the standard. See the > > > contracts proposal which also kind of violates this rule by adding an > > > exception. Basically, this is the poor man's version of emulating the > > > contracts proposal. In the long term we plan to piggyback on contracts > > > rather than hacking the C++ attributes. > > The contracts proposal was not adopted and I am opposed to adding this > > change for any standard attribute. We've done a lot of work to ensure that > > those attributes attach to the correct thing based on lexical placement and > > I don't want to start to undo that effort. > I am not sure how to proceed in this case. As far as I understand this wasn't > the main reason why didn't we adopt the contracts proposal. > > While I do understand why is it good to make the lexical placement matter, > but the current placement hinders non-trivial use-cases. Currently, we either > have a type attribute or we cannot mention any of the formal parameters. This > is why the contracts cannot work without changing attributes in the standard, > and the same reason why this patch will never work without doing this. Or do > you have an alternative in mind? > > Do you think introducing a frontend flag for this attribute would help? This > would make it clear that we are currently using a non-standard feature that > needs to be enabled explicitly. Again, this is a temporary solution, that > will go away as soon as we have contracts. Since contracts need to solve the > same problem, whatever the solution will be, this patch can piggyback on that. > > What do you think? > I am not sure how to proceed in this case. As far as I understand this wasn't > the main reason why didn't we adopt the contracts proposal. There were a lot of flaws, and this was one of them. I don't think you could point to any one flaw and say "this is why we didn't get contracts" though. > Or do you have an alternative in mind? I agree that this is an existing deficiency in both the C and C++ standards with the attribute syntax for functions. There simply is no way to make a function declaration attribute that can refer to the parameters currently. The only alternative that comes to mind is to use only a GNU-style spelling rather than trying to use a [[]] spelling. > Do you think introducing a frontend flag for this attribute would help? This > would make it clear that we are currently using a non-standard feature that > needs to be enabled explicitly. Again, this is a temporary solution, that > will go away as soon as we have contracts. Since contracts need to solve the > same problem, whatever the solution will be, this patch can piggyback on that. There's no guarantee that contracts will come back with the same syntax used previously, so I wouldn't want to rely on that to solve the issue until we have a better idea of what contracts will look like coming out of the new study group. My preference is to use the GNU-style spelling and not provide a [[]] spelling. We've done that for other attributes (like `enable_if` and `diagnose_if`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72810/new/ https://reviews.llvm.org/D72810 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
