This is in preparation of the visibility related changes discussed in https://llvm.discourse.group/t/external-function-declaration-has-changed-help-needed/2357. This change disallows attributes that sym_visibility/sym_name/type etc which are parsed/inferred from other places to be present in the attribute dictionary. This way, for the follow on changes, the function parser has just one parsed visibility value (parsed as keyword private/public/nested) to consider instead of trying to also look into the parsed attribute dictionary.
For this situation, having FunctionImplementation emit the error seems better given that "attribute is not allowed in attribute dictionary" does not provide much context on what the error is, or how it should be fixed. Can we provide a better error message instead?
Can you add a bit more context here?
Though maybe this is just me getting confused thinking about it. When I think of the attribute dictionary, I think of the complete dictionary on the operation and not just this specific one. We could provide specialized error messages for these attributes:
sym_visibility attribute should not be specified in the attribute dictionary, and instead be specified before the name of the function or something.
|263 ↗||(On Diff #315011)|
Yes, when I say "attribute dictionary" I mean the parsed one, and not the complete one with parsed + inferred attributes. To be clear, we would need to specialize the error message per attribute. Let me try that.