SE-0439 clarification: trailing commas in @attached/@freestanding?

Evolution Discussion
1

SE-0439 allows trailing commas in symmetrically delimited comma-separated lists, but carves out built-in attributes with custom parsing such as @inline(never,). From the acceptance thread:

Should macro role attributes be excluded too?

Today this is rejected:

@attached(
    member, 
    names: 
        named(foo1),
        ...,
        named(fooN),  // <- Unexpected ',' separator
)
macro Foo()

I noticed the original implementation PR briefly discussed and removed support for this. As of today, swift-format is generating invalid code due to this.

These are built-in attributes, but their names: / conformances: lists can grow arbitrarily. Not needing to special-case the final element is SE-0439's core motivating case.

Could this be considered a bug fix or is this something that has to go through evolution as an amendment to SE-0439?

2 Likes
2

There's some additional discussion on a companion issue I filed that explains some of the reasoning further: `parseMacroRoleArguments` should allow trailing commas · Issue #3306 · swiftlang/swift-syntax · GitHub

My own personal opinion is that the rules should be relaxed. It's extremely subtle for users to have to remember "is this an attribute that takes a comma-delimited list that is semantically a list of parameters or is this an attribute that has some custom interpretation of the parameters even though it looks indistinguishable from a comma-delimited list?" It also makes it harder to write tooling around those rules, because there's nothing at all in the syntax tree that distinguishes @attached(member, names: foo, bar) from @MyCustomAttribute(x, label: y, z). Structurally, they're identical.

I can see the argument for disallowing trailing commas for built-in attributes that take arguments that are completely unique syntax like @abi (which takes a decl header), but I think if the attribute arguments parse as a single LabeledExprListSyntax, we should just allow the trailing comma.

6 Likes
3

Yes, unfortunately, I think the generalities we laid out left some things underspecified for attributes, and what's implemented is a conservative interpretation of the guidance.

The example we gave—@inline(never,)—illustrates the kind of trailing comma we clearly don't intend to support, and the case of comma-delimited syntax which is also semantically a list and is also parsed by the compiler as a list is clearly what we do intend to support.

Between those two poles, I'd invoke another point we made in that acceptance: users shouldn't have to "think like a parser." So if the compiler happens to have a custom code path but what's being parsed is semantically a list, then my understanding of our discussion is that it ought to be allowed to leave a trailing comma.

This is not to say that there won't be harder cases that need formal clarification, but I would think names: named(...), named(...), ... isn't a controversial one in this respect.

1 Like
4

So this unfortunately still causes problems for tooling, because @inline(never) is parsed as an AttributeSyntax(..., LabelListExprSyntax(...)), making it indistinguishable from @ArbitraryAttribute(ArbitraryName). The latter is allowed today to have a trailing comma by the SE-0439 rules, despite being structurally identical to the former.

The easy but "cop-out" solution here is to have swift-format and other tooling simply not insert trailing commas into any attributes, even those that could theoretically support them. But that's somewhat unsatisfying in the sense that it carves out a special rule that's overly conservative for one particular syntactic construct.

There are other options, like hardcoding a list of built-in attributes, or using the first letter's case to guess what kind of attribute it is. Those are also various degrees of distasteful.

5

From the implementation standpoint, I agree.

But on the flip-side, @inline(never) is clearly not a list for those who don't "think like a parser." Additionally, supposing @inline(never,) were permitted, it would be arbitrary if nonisolated(nonsending,) or private(set,) were not permitted and distasteful if they were. And so if we have to hardcode a list of built-in attributes that take semantic lists, that would be the less distasteful option.

2 Likes
6

SwiftFormat handles this by assuming that any attribute that starts with a lowercase letter or underscore is a built-in attribute that doesn't support trailing commas, which is not ideal either but has worked well in practice for us and is better than excluding all attributes.

I tend to think all attributes that are at all list-like should support trailing commas (@available, @attached, @freestanding). @xwu's argument about @inline(never,) vs private(set,) is definitely compelling though.

7

I can also see an argument that allowing these cases is the more consistent option. Maintaining a manual allow list of built in attributes seems like its own kind of distasteful special casing.

We already allow foo(bar,) in ordinary function calls, despite that being something nobody would write deliberately. So perhaps the same broad rule should apply here too, rather than carving out attribute specific exceptions.

2 Likes