On 08/22/2017 09:51 PM, Jason Merrill wrote:
The C and C++ front ends already have a diagnose_mismatched_attributes function, which seems like the natural place to add more conflict checking.
There are a few major problems with handling attribute conflicts in diagnose_mismatched_attributes. The function only diagnoses conflicts, it doesn't resolve them (choose one attribute or the other). Currently, when they are resolved, it's done in each attribute handler. But this is done inconsistently because of the limitations of the infrastructure: no access to the last pushed declaration. Often, it's not done at all. Making the declaration available to every handler is one of the design improvements of the patch. Attributes are defined in the attribute_spec tables in a number of different places: all the back ends, in addition to the front ends, but processed by the general infrastructure in decl_attributes in attribs.c. The infrastructure already has all the smarts to either accept or reject a conflicting attribute. None of this is available in diagnose_mismatched_attributes. The current implementation of the function hardcodes knowledge about a small number of attribute relationships in the code. That's a poor approach given the table-driven attribute_spec design. For one, it makes it difficult for back-end maintainers to add a new attribute that's mutually exclusive with another (the back-end maintainer would need to change the front end). It might explain why no back-end attribute conflicts are diagnosed there. Some, maybe even most of these problems could be overcome by moving the conflict resolution from decl_attributes to diagnose_mismatched_attributes. But it would mean duplicating a fair amount of the logic between the two functions. I think that would result in an inferior solution. From both a design and implementation point of view, I feel the logic fits best in the attribs.c. Martin
