Pitch: Static Custom Attributes (Round #2)

Yes, please.

Off the top of my head, suppose you wanted to describe @IBAction. This attribute has a pretty complex set of rules about what it supports; you might express them with something like this:

@staticAttribute struct IBAction {
  // This is the initializer used for the attribute. It needs to be marked
  // to distinguish it from the other members.
  @staticAttribute init() {}

  // The remaining members are all "shapes" of declarations that you can
  // validly apply this attribute to. If the declaration "looks" like one
  // of these, ignoring the names but looking at things like the declaration
  // kind, parameter counts and types, result type, throwing-ness, and
  // attributes, then you can apply the attribute to that declaration.
  
#if os(macOS)
  // macOS allows only one signature, with a sender parameter and Void result.
  @objc func action1(_ sender: AnyObject?) {}
#else
  // iOS supports zero, one, or two parameters.
  @objc func action0() {}
  @objc func action1(_ sender: AnyObject?) {}
  @objc func action2(_ sender: AnyObject?, _ event: AnyObject?) {}

  // To support WatchKit, it also supports a bunch of variants which take
  // value types. I don't remember exactly which ones WatchKit uses, but
  // that's fine, this is just an example.
  @objc func actionInt(_ value: Int) {}
  @objc func actionDouble(_ value: Double) {}
  @objc func actionBool(_ value: Bool) {}
#endif
}

This idea is at best half-baked, so it probably makes sense to defer it, but hopefully you get the idea.

Yes, this sounds right.

Doug

Yeah, I like this idea. @compileTimeAttribute is more descriptive of what these do.

Doug

7 Likes

The compiler knows which kind of accessor each entity is, so although we don't have distinct subclasses of the compiler's AccessorDecl class for get/set/etc., it certainly does distinguish them as different entities (with a separate accessor kind). I'm fine with separating out the different kinds of accessors.

Doug

The direction here looks good.

Nit: there is no @deprecated -- I've typed it too :)

Might be worth spelling out that trailing-closure syntax will not work for custom attribute initializers (or that you plan to make it do so.)

The pitch states a goal of assisting SourceKit-based tooling, so I think it's worth addressing at least roughly how custom attributes could be exposed through the SourceKit interface.

Currently, this Swift:

@available(*, deprecated)
func f() {}

...generates this doc-structure:

{
    key.name: "f()"
    ...
    key.attributes: [
      {
          key.offset: 0,
          key.length: 25,
          key.attribute: source.decl.attribute.available
      }
    ]
}

So attributes have a hard-coded kind and there is no decode of their arguments. [This means there is a compatibility issue to work through around lifting current built-ins to stdlib.]

For these custom attributes, the most straight-forward & minimally-sufficient thing feels to be to treat them as a more first-class piece of AST and additionally include them in the regular doc structure, decoding the (potentially highly nested) function-call-ish expression in the same way as regular call expressions are done today. For example:

@MyCustom(some: .value)
func f() {}

...to:

{
  key.name: "f()"
  ...
  key.substructure: [
    {
      key.kind: source.lang.swift.expr.attribute,    # discuss...
      key.name: "MyCustom",
      key.offset: 0,
      key.length: 23,
      key.nameoffset: 1,
      key.namelength: 8,
      key.bodyoffset: 10,
      key.bodylength: 12,
      key.substructure: [
        {
          key.kind: source.lang.swift.expr.argument,
          key.name: "some",
          key.offset: 10,
          key.length: 12,
          key.nameoffset: 10,
          key.namelength: 4,
          key.bodyoffset: 16,
          key.bodylength: 6
        }
      ]
    }
  ]
}

Also note that neither import declarations nor property accessors (along with any attrs set on them) are currently exposed through SourceKit -- affects testability and utility of some bits of the proposal.

Happy to help on the implementation side in this kind of area if you need it.

I spotted another gap in the declarations list. There is currently no way to distinguish let from var from computed var from mutable computed var. I can imagine attributes that would prefer to make this distinction. One of the selling points of this proposal is the compiler validating correct usage. We shouldn't skimp on this kind of granularity.

If you model declarations as an OptionSet instead of an enum you would still be able to support something like variable that covers all of them when the granularity isn't required.

4 Likes

It's really nice to see this discussion going forward. I was feeling bad for not being able to comment earlier but I can see @hartbit and @Douglas_Gregor are already doing a great job without me.

My problem with this general idea is that I believe many attributes might need some sort of extra validation that we'll never be able to cover with our design. In particular, the compiler's internal attributes will always need bespoke code (at least in order to manifest the attribute's effects in the language) and I don't think that can really be avoided. Because of that, trying to add validation mechanisms here might be too much complication for too little gain.

We thought of adding parameter attributes precisely as a way (sort of a workaround?) to allow declaring custom attributes similar to @escaping. I say similar because @escaping actually ends up in the function's type, but since this is compile-time only I think many of the same use-cases can be covered this way.

It's worth noting that the list of declarations used here in no way limits the declarations that might be used for a potential @runtimeAttribute proposal down the line.

Sounds good!

I personally think this MyWrapperStruct "workaround" wouldn't be a problem, but I'm not opposed to allowing enums either. I hadn't thought of enums with associated values actually, they seem like an important use case. If it doesn't add too much complexity to the implementation, I'm not opposed to it.

If this means that there's no way to expose the values of the parameters of an attribute through SourceKit, that could be a serious impediment to our current design. Would anyone who understands this better care to weigh in?

The definition of a "declaration" in this thread seems kind of murky, and maybe that's related to where each person draws the line on granularity. For instance, we thought were already covering all of the declarations (from a list of compiler declaration classes), but in fact you've given a few good examples of how they can be subdivided further.

I think it'd be interesting to subdivide them in some cases (especially where it would be hard to determine by hand what subdivision is being used), but I do worry that too much granularity might make attributes harder to declare. That said...

...this is a very interesting idea.

Yes, it means that today, in master, parameters of an attribute are not broken down. The post goes on to suggest how that could be addressed as part of this proposal.

I still think the positioning for this case is not right. Swift went away from having keywords for parameters/attributes. Everything goes onto the parameter type. I don't think we should re-introduce this old behavior with this proposal and require consistency.

Think the most important thing to keep in mind is that even if we don’t support full granularity in the initial proposal (to be clear: I still hope we do) we want a design capable of being refined and enhanced with additional granularity down the road. I suspect people will find good use cases for any level of granularity we can imagine and would appreciate the compiler validating valid usage of their attributes.

With the above in mind, I think we should seriously consider the OptionSet direction. The cool thing about that design in this context is that it really only requires source compatibility. I think we would even be able to change the value-level representation of the declarations in the future if necessary. In theory people could use this type in their code as a runtime value buy I can’t imagine why anyone would do that and it would be a pretty clear abuse of the feature. We could even document that the value-level representation is not guaranteed to be stable across Swift versions.

The attributes you refer to were moved because they really apply to the type of the parameter, not the parameter itself. It is certainly possible somebody has a use case for applying a custom attribute directly to a parameter. I don’t see why it should be banned just because none of the built-in attributes make sense in this position.

Oh ok, I’ll admit I’m not familiar with SourceKit so I didn’t understand some of your proposal. But it seems to me that the .value argument isn’t represented in your suggested output. Do you think it’s unnecessary, or that it could be obtained in some other way?

I think the reason that happened was because these specific attributes made more sense in the type rather than in the parameter, not that it was supposed to be a more general philosophy. I could be wrong though.

Agreed!

Well then the parameter type attribute is missing in the proposal, or did I missed it?

Yeah, it’s not there. I think we didn’t include it (partly) because it would involve adding attribute support to the type system, which might complicate the proposal significantly. Maybe it could be added later if it really proves to be an important missing feature. @hartbit and @Douglas_Gregor should feel free to correct me though.

Not trying to be persistent, but can you tell me what I may miss? You have new parameter attributes, you have type attributes. What constrains you not to have parameter type attributes? That I don‘t understand.

Right, you have to pull it out of the source using the bodyoffset and bodylength fields of the expr.argument thing. This is how call expressions are represented today - you can try it out using sourcekitd-test -req=structure (or grab SourceKitten and do sourcekitten structure).

Probably good to have @akyrtzi's input on this pitch at some point.

Unlike sourcekitd, SwiftSyntax has no limitations for providing the expression structure with full fidelity. But this only addresses a small part of the question on how to make use of these new attributes.

I'd like to offer some feedback to improve the proposal.

The proposal states this as the primary motivation for introducing the new attribute mechanism, and contrasts it with Sourcery templates, but it doesn't actually explain how they are going to be used by such tools and why they are a much better mechanism. Let's walk through some examples from a tool author's point of view that wants to take advantage of this mechanism:

Very simple case, we can just get the name here and infer any property we want to associate with it.

We are now getting into interpreting expressions. This is still simple enough, we have the argument label name and it is a single dot-member expression that we can infer properties from its name.

Ok, not we are getting into needing to interpret array expressions. Let's explore the space a bit more, how about this:

At this point I think I'm going to hit the limit of what I can interpret. So let's say that there will be a tacit understanding between the users and tools to keep things simple in code. The above seems unreasonably complicated, but are there things that seem reasonable for users to want to write but are hitting similar limitations ? Say a user has this:

Ok, as a tool author this is simple enough, I can see the argument label name and the integer value it accepts. But say the user wants to avoid hard-coding the values in each place:

Oops, my tool is unable to interpret this now.

These examples are intended to illustrate an issue that is not acknowledged in the proposal, which is that the currently hardcoded compiler attributes have constrained syntax grammar, not full expression generality, and their constrained syntax ensures they can be interpreted with a very simple syntactic processing.

Now, to clarify, I'm actually a fan of introducing custom attributes but I'd recommend that more thought should be given on how the feature, as proposed, affects and benefits tooling, particularly since tooling is stated as the primary motivation for the proposal.
Here's some food for thought, how is the @compileTimeAttribute feature relating to the compile-time interpreter ? What if we restrict the expressions to being evaluable at compile-time, what advantages and disadvantages would we get ?

5 Likes

Thanks for the insightful comments @akyrtzi. This is very much appreciated indeed.

This is where I admit I'm slightly out of my league as I have little intuition for what is easily implementable. I would expect that compile-time attributes only support compile-time expressions, as the compiler needs to execute those expressions during compilation. But I don't know the extend of compile-time expressions in the compiler today: I have the impression something exists, as its being used for improving os_log, but I would really appreciate if someone could explain that to me.

Thanks to @johnfairh's questions, I've tried to give more thought to this. It seems clear to me that, to be useful, compile-time attributes, with their data, need to be available both in SwiftSyntax and SourceKit. But I don't know to do this, from an implementation point of view. I'm naive, so I would suggest adding a requirement that compile-time attribute types must conform to Encodable, but I have no idea if the compiler can synthesize Encodable methods for the attribute types and use them to store the generated data in the AST for SwiftSyntax and SourceKit to access.

I've given more thought to the compileTimeAttribute initializer thanks the great feedback received and I'd like to comment on a few things.

I think this is an awesome idea! I'll work on it with the other co-authors.

I agree with @Vinicius_Vendramini that we'll never be able to allow the level of granularity that everybody wishes and that by trying to, we're making the feature worse for the majority. But I'll go even further, I think we may have already gone too far with the scopes configuration: I'm not sure what it enables is immensely useful and not all scopes have sense for all declarations.

How about dropping the scopes configuration for now? We can always add it back afterwards.

This makes me think that we should perhaps remove certain declarations like those until we are sure we can make them accessible through the tooling.

I’ve been wondering about how tooling will be able to interpret attribute arguments. This idea of encoding the attribute value is a pretty interesting direction.

As a Sourcery user let me imagine how this might work. Sourcery uses SourceKit (via SourceKitten) in its implementation so it would need to get attribute data via that framework. I believe the source data is routed through JSON somewhere in this pipeline. Sourcery currently has an annotations: [String: NSObject] dictionary that exposes its custom comment annotations. Similarly, attribute data would make sense via an attributes dictionary with String keys named after the attribute type. Maybe the Value could be Data that was encoded in a known way by the compiler.

Custom annotations in Sourcery are defined by each template. Templates taking advantage of user-defined attributes would need to either ship a framework or just be dropped into user projects as source the way some Sourcery protocols are today. Because the template is responsible for defining the attributes it will be able to understand the data that is serialized by the compiler. I happen to use Swift templates which would make decoding easy - all we need to do is make the same attribute declaration available in the template code and decode the Data using the correct decoder.

One important thing to point out: there is some sloppiness with regards to the attribute declaration here. Many Sourcery users will not want to set up a framework containing the attributes and link that into the main project as well as the Sourcery template. So a declaration of the serialized attribute would be in one module and the attribute that is deserialized would be syntactically identical but declared in a different module.