Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SIP-63 - Scala 3 Macro Annotations #80

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

nicolasstucki
Copy link
Contributor

No description provided.


/** Annotation classes that extend this trait will be able to transform the annotated definition. */
trait MacroAnnotation extends StaticAnnotation:
/** Transforms a tree `definition` and optionally adds new definitions.
Copy link
Contributor

@lihaoyi lihaoyi Feb 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cask and MainArgs are perhaps one of the more common libraries using "fake" macro annotations today, and I suspect they wouldn't be implementable using this current proposal as is

That because they require that multiple @annotated methods be considered to generate a single combined routing object. In MainArgs you can have multiple @main methods, and in Cask you can have multiple @cask.get/@cask.post/etc. routes.

In both cases, we currently have a expression-macro that looks at the class body, looks for all the @annotated defs within it, and generates a single expression that references all of them.

These are different usage patterns than the use cases included in this proposal, but I don't think they are unreasonable. It's fundamentally the Registry Pattern, which e.g. is a very common use case for Python's @decorators which Scala macro annotations are very similar to. Consider this example from the popular Flask project:

from flask import Flask

app = Flask(__name__)

@app.route('/') # Registers `def index` in `app`
def index():
    return 'Index Page'

@app.route('/hello') # Registers `def hello` in `app`
def hello():
    return 'Hello, World'

Or Python's Click library:

import click

@click.group() # registers `def cli` with `click`
def cli():
    pass

@cli.command() # registers `def initdb` with `cli`
def initdb():
    click.echo('Initialized the database')

@cli.command()# registers `def dropdb` with `cli`
def dropdb():
    click.echo('Dropped the database')

Is there some way we can support that sort of use case with Macro Annotations?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe an aggregate version that only returns new trees to go at the end of the surrounding scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could have a macro annotation @flask that looks for normal annotations @app.route(...) in its scope

@flask def app: Flask = Flask()

@app.route("/")
def index() = "Index Page"

@app.route("/hello")
def hello() = "Hello, World"

it could then generate a main as follows

class app {
  <static> def main(args: Array[String]): Unit = {
    app
      .registerRoute("/", index)
      .registerRoute("/hello", hello)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might also have it as

@flask val app: Flask = Flask()

and modify it to

val app: Flask = 
  Flask()
    .registerRoute("/", index)
    .registerRoute("/hello", hello)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the limitation we have is that we need to instantiate the annotation at compile time. If we have @app.route('/') where app is a reference to a local value that will only exist later (at runtime) we cannot instantiate it as a macro annotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another approach that would follow closer the module initialization semantics of the Python version might be

object app extends Flask {

  @route("/")
  def index() = "Index Page"

  @route("/hello")
  def hello() = "Hello, World"
}

where the @root macro annotation assumes it is inside a Flask instance and generates something like

object app extends Flask {

  val _ = (this: Flask).registerRoot("/" , index)
  def index() = "Index Page"

  val _ = (this: Flask).registerRoot("/hello", hello)
  def hello() = "Hello, World"
}

Copy link
Contributor

@lihaoyi lihaoyi Mar 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolasstucki perhaps using the raw Python examples was a bit overly specific, so let's not over-fit on that exact syntax. Let's instead consider the Cask/MainArgs equivalents, which are a bit more idiomatic Scala:

// Cask
object MinimalApplication extends cask.MainRoutes{
  @cask.get("/")
  def hello() = {
    "Hello World!"
  }

  @cask.post("/do-thing")
  def doThing(request: cask.Request) = {
    request.text().reverse
  }

  initialize()
}
// MainArgs
object Main{
  @main
  def foo(@arg(short = 'f', doc = "String to print repeatedly")
          foo: String,
          @arg(doc = "How many times to print string")
          myNum: Int = 2,
          @arg(doc = "Example flag")
          bool: Flag) = {
    println(foo * myNum + " " + bool.value)
  }
  @main
  def bar(i: Int,
          @arg(doc = "Pass in a custom `s` to override it")
          s: String  = "lols") = {
    println(s * i)
  }
  def main(args: Array[String]): Unit = ParserForMethods(this).runOrExit(args)
}

In both Cask and MainArgs, we do a "batch" registration:

  • Cask's initialize() function takes an implicit macro that resolves the macro-annotations, generates the trees, then assigns it to a (private) mutable variable for use later

  • MainArgs' ParserForMethods macro is an explicit macro call that resolves the macro annotations, generates the trees, and we ourselves use it as part of def main

(Mill actually has a similar design and implementation for it's T.command definitions, that's very similar to how MainArgs works. It does a similar explicit macro call, albeit the macro call is performed in generated code so the user doesn't see it)

In fact, the @flask val app: Flask = Flask() example you gave is a bit unnecessary: we can do the same thing without a macro annotation, via val app: Flask = Flask(), by making Flask.apply a macro (what MainArgs does) or making it take an implicit macro (what Cask does)

This is in contrast to the Python examples, which use mutable registries that each decorator appends to, similar to your (this: Flask).registerRoot("/" , index) example. While that is a valid design, I think the use of mutable state is a bit un-idiomatic Scala, especially given the "immutable" approach of registering all the generated trees at once.

I think my ideal scenario would be

object Foo {
  @bar
  def qux() = "Index Page"

  @baz
  def cow() = "Hello, World"
}

Expanding to some kind of generalized (in pseudocode)

object Foo {
  @bar
  def qux() = "Index Page"

  @baz
  def cow() = "Hello, World"

  ${generateDefinition(expandMacro(@bar def qux), expandMacro(@baz def cow))}
}

That gives us flexibility: generateDefinition can generate a:

  • object Main{ def main(args: Array[String]): Unit = ??? }, for Scala 3 @main methods
  • def main(args: Array[String]): Unit = ??? for MainArgs' `@main methods
  • val routes = Seq(???) for Cask's routes

Perhaps the next questions would be, assuming bar and qux contain their own logic for macroExpand, then:

  1. Who defines the generateDefinition logic? Is it on the enclosing template? Or some property of bar or qux?
  2. If we need to generate multiple definitions, e.g. we have an object foo that contains both @cask.gets and MainArgs @main methods, how do we ensure the @cask.gets all get passed to Cask's generateDefinition logic, while all the MainArgs @mains get passed to MainArgs' generateDefinition logic?

Going with the mutable-registration-pattern approach sidesteps these questions - we can generate a separate statement before each annotated def that performs the registration when executed - it's just a bit awkward and unidiomatic. But if we cannot find a better solution then it's certainly an option.

This is all a bit convoluted, but I think it's doable to find an elegant approach here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that macro annotations might not be the best tool to get an elegant solution to this particular problem. Based on these examples, inline traits (report, prototype) are a much better fit.

Cask with inline traits
object MinimalApplication extends MainAutoRoot {
  @get("/")
  def hello() = "Hello World!"

  @post("/do-thing")
  def doThing(request: Request) = request.text().reverse

  // generated code
  // def routes: Route = Seq(
  //   Route("hello", hello),
  //   Route("doThing", doThing, ...),
  // )
}
inline trait MainAutoRoot {
   def routes: Route = generateRoutes // macro `generateRoutes` finds the routes defined in the class that extends `Routes`
}

trait MainRoot {
  def roots: Roots
  def main(...) = // do something with routes
}
  1. Who defines the generateDefinition logic? Is it on the enclosing template? Or some property of bar or qux?

In an inline trait, it would be the trait itself.

  1. If we need to generate multiple definitions, e.g. we have an object foo that contains both @cask.gets and MainArgs @main methods, how do we ensure the @cask.gets all get passed to Cask's generateDefinition logic, while all the MainArgs @mains get passed to MainArgs' generateDefinition logic?

With inline traits it becomes simpler because there is a single point of expansion for the definition. The definition expansion can collect all (non-macro) annotated definitions in scope.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolasstucki I think using an inline trait sounds reasonable. But if that's our stance, then that basically cuts out Example 3: Main annotation from the use cases of macro annotations: it doesn't make sense to say macro annotations provide a way to provide a worse version of MainArgs. We'll need to find other use cases to replace it, to make sure that macro annotations have enough motivating use cases to be worth it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it does not work for the particular API design choices of MainArgs, it allows for many other kinds of main annotation use cases. For example, it can be used to implement the generalized main annotation in scala/scala3#19937.

Comment on lines +287 to +288
#### Separate/incremental compilation
As we transform the definitions after pickling, separate/incremental compilation is not affected by the macro annotations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if a macro annotated member of a trait generates a new private val/var/lazy val in the trait? That would still affect classes extending that trait, in a way that incremental compilation should be aware of.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how we would support these. They might need to be disallowed.

There might be reasons to evaluate the inner macro annotation first, then the outer one.
In that case, we have two options: we reverse the order of evaluation, or we add the ability to specify the order of evaluation of each macro annotation.

### Allow new definitions to be macro annotated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this means.

Copy link
Contributor Author

@nicolasstucki nicolasstucki Mar 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refers to new definitions created by the macro expansion. I will reword it.

@anatoliykmetyuk anatoliykmetyuk changed the title SIP-NN - Scala 3 Macro Annotations SIP-63 - Scala 3 Macro Annotations Mar 8, 2024
@soronpo
Copy link
Contributor

soronpo commented Mar 8, 2024

Please open a thread on contributors.scala-lang.org to gain community feedback, and link it to this proposal.

@nicolasstucki
Copy link
Contributor Author

Please open a thread on contributors.scala-lang.org to gain community feedback, and link it to this proposal.

Here is the contributors thread: https://contributors.scala-lang.org/t/scala-3-macro-annotations-sip-63-discussions/6593

nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Mar 13, 2024
…nnotation`)

`MainAnnotation` and its implementation `newMain` predate `MacroAnnotation`.
The `MacroAnnotation` is subsumed  feature and allows much more flexibility.
`MainAnnotation` and `newMain` could be reimplemented as a macro annotation
in an external library.

See SIP-63: scala/improvement-proposals#80
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Mar 13, 2024
…nnotation`)

`MainAnnotation` and its implementation `newMain` predate `MacroAnnotation`.
The `MacroAnnotation` is subsumed  feature and allows much more flexibility.
`MainAnnotation` and `newMain` could be reimplemented as a macro annotation
in an external library.

See SIP-63: scala/improvement-proposals#80
@lihaoyi
Copy link
Contributor

lihaoyi commented Mar 21, 2024

One question that came up in the last SIP meeting is whether or not it's possible to implement #78 using macro annotations, and what it would look like if we did. @nicolasstucki do you have any insights there? We already have full implementations and test cases in the https://github.com/com-lihaoyi/unroll/ repo, so it should be possible to try it out using macro annotations and see if it can be made to work

@nicolasstucki
Copy link
Contributor Author

One question that came up in the last SIP meeting is whether or not it's possible to implement #78 using macro annotations, and what it would look like if we did. @nicolasstucki do you have any insights there?

I wrote and example implementation of a strip down version of #78 for unrolling the last parameters of a def. This implementation shows that we have access to all the we need to construct the definition new definition.

However, we still have some limitations in the current implementation, such as missing the parameter macro annotations (we are woking on a prototype and should have something soonish). We might also me missing some helper functions in the reflection API to help us deal with this transformation more elegantly (for example a simple way to get the default arguments).

I have not tried with constructors yet. We will have a similar situation for class constructors. But, case class constructors parameter unrolling might not fit the current specification of macro annotations. As the annotation is on the parameter, the transformed tree will be the constructor of the case class, this only allows us to add methods in the case class itself but not in the compassion object.

nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Mar 27, 2024
…nnotation`)

`MainAnnotation` and its implementation `newMain` predate `MacroAnnotation`.
The `MacroAnnotation` is subsumed  feature and allows much more flexibility.
`MainAnnotation` and `newMain` could be reimplemented as a macro annotation
in an external library.

See SIP-63: scala/improvement-proposals#80
nicolasstucki added a commit to scala/scala3 that referenced this pull request Apr 2, 2024
…nnotation`) (#19937)

`MainAnnotation` and its implementation `newMain` predate
`MacroAnnotation`. The `MacroAnnotation` is subsumed feature and allows
much more flexibility. `MainAnnotation` and `newMain` could be
reimplemented as a macro annotation in an external library.

See SIP-63: scala/improvement-proposals#80

### When should this be removed?
As an experimental feature, we can remove it in any patch release. We
have 2 options:

1. Conservative: we wait until the next minor release to minimize the
impact on anyone that was experimenting with this feature. (decided to
take this approach in the Scala core meeting on the 13.03.2023)
2. ~Complete: We remove it as soon as we can, next patch release of 3.4
or 3.5. We also remove it from the next 3.3 LTS patch release.~
@nicolasstucki
Copy link
Contributor Author

@jchyb and @hamzaremmal will take over the development of this SIP.

@smarter
Copy link
Member

smarter commented May 15, 2024

This proposal was discussed at the last SIP meeting, but there was concern that the current proposal is currently too limited and did not have enough usecases as-is:

  • The vote for accepting it lead to a 4/4 split.
  • The committee unanimously voted against rejecting the proposal.

In particular, the fact that existing libraries like cask and mainargs cannot be ported to work as-is without requiring extra annotations seems problematic. If we look at the cask example from @lihaoyi again:

// Cask
object MinimalApplication extends cask.MainRoutes{
  @cask.get("/")
  def hello() = {
    "Hello World!"
  }

  @cask.post("/do-thing")
  def doThing(request: cask.Request) = {
    request.text().reverse
  }

  initialize()
}

The current proposal would require an extra annotation on the object definition to gather all the method-level annotations.

To avoid this extra level of boilerplate, I'd like to suggest we introduce "inheritable macro annotation", concretely if we define MainRoutes in cask as:

class caskGen extends InheritableMacroAnnotation { ... }
@caskGen trait MainRoutes { ... }

Then we should desugar:

object MinimalApplication extends cask.MainRoutes

into:

@caskGen object MinimalApplication extends cask.MainRoutes

I believe inheritable macro-annotations could be useful in many situations. For example, if I want to memoize instances of an open class hierarchy, I'd like to be able to define:

class memo extends InheritableMacroAnnotation { ... }
@memo trait Base

so that anyone inheriting from Base automatically gets a memoized class without having to remember to put the annotation on the class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants