Should we allow duplicate attributes?

[DanilaFe]

Motivated by Jade's code in the below screenshot.

My intention with the @chplcheck.ignore attribute was for it to target a single rule. Thus, to silence multiple rules, you could just list the attribute twice. However, this is currently disallowed by the compiler (and explicitly listed as a limitation in the tech note for attributes).

I understand that not all attributes should support being used multiple times (e..g, maybe not deprecated or unstable). But I see no problem with allowing some (like chplcheck.ignore) to be repeated.

[mppf]

Right, we could decide to enable @chplcheck.ignore("UnusedLoopIndex", "SimpleDomainAsRange") as an alternative. As to the question of why an attribute can't be used multiple times, I do not remember the reason for that limitation. @arezaii — do you?

[arezaii]

I think we didn't have a use case for multiple and it led to questions about redefinition vs. multiple definition during assignment, and when retrieving attributes we do so by name so having multiple with the same name added complexity when there was no use case.

[bradcray]

Stylistically, I like Michael's varargs proposal over multiple attributes of the same kind as the approach here. But I don't know whether that requires more from attributes than they support today (and other attributes may not be as amenable to a varargs approach as this one looks like it may be?)

Or wait, is varargs already enabled and this issue is just asking about permitting both forms? (I'm just seeing now that Jade's screenshot starts with the varargs form).

[jabraham17]

Currently, the chplcheck.ignore attribute is of the form chplcheck.ignore(rule, comment), where comment is optional. So a fix specific to chplcheck.ignore would be to change it to chplcheck.ignore((rule, comment), ...) where it can take a varargs of strings, or 2-tuples of strings to support the optional comment.

This is similar to the approach I took with llvm.metadata, where the attribute accepts an arbitrary number of arguments to work around the issue of duplicate attributes.

In my opinion, duplicated attributes is cleaner and I would rather us support this, then have to do the varargs

[bradcray]

I see, thanks. And I'd agree that a 1+(1 optional)-arg attribute like this is less ideal for varargs than a simple 1-arg attribute which is what I was imagining this was (and alluding to in saying other attributes may not be as amenable). Based on that, I agree that multiple attributes seems preferable to varargs here, assuming we want to keep that optional comment field.

For my own edification, what does the comment field do (mechanically speaking) vs. using @chplcheck.ignore(rule) // comment?

[jabraham17]

For my own edification, what does the comment field do (mechanically speaking) vs. using @chplcheck.ignore(rule) // comment?

Currently, nothing. It offers no benefit over a comment. I think there are future goals for its usage in tools (for example, chplcheck could have a --report-ignored flag that uses this)

[DanilaFe]

For my own edification, what does the comment field do (mechanically speaking) vs. using @chplcheck.ignore(rule) // comment?

Tooling (like chplcheck) can make sense of why it was disabled. Not needed now, but could be useful if it were to provide a detailed report (10 warnings, 7 silenced, here's how).

Right, we could decide to enable @chplcheck.ignore("UnusedLoopIndex", "SimpleDomainAsRange") as an alternative

Because in general, attributes can contain additional metadata (like the comment field in this case), the proposed solution is insufficient. Sure, you can argue away the comment field, but there are other plausible fields that could be there ('since' or 'until' for warnings, if you want to give time to fix a certain issue, but start warning for it later, 'version' warnings for things that are the best way to do things in Chapel version N, but not the best way to do it in Chapel version N+1, etc).

In general, if any time you may repeat an attribute, you need to make it a vararg attribute, that would leave a bad taste in my mouth. I think of attributes as function calls (ish), and saying "you can only call a function once, so if you need to pass more data, use varargs" seems odd. It also goes against precedent in Rust's (?) and Python's attributes.