Skip to content

Commit

Permalink
Update macro spec to allow more introspection in Phase 2.
Browse files Browse the repository at this point in the history
  • Loading branch information
davidmorgan committed Nov 21, 2023
1 parent aab3033 commit 1b43d3c
Showing 1 changed file with 23 additions and 42 deletions.
65 changes: 23 additions & 42 deletions working/macros/feature-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,36 +268,12 @@ order of macros:
Here, the macros applied to C are run `First()`, `Second()`, then `Third()`.
* **Macros are applied to superclasses, mixins, and interfaces first, in**
**Phase 2** For example:
```dart
@Third()
class B extends A with C implements D {}
@Second()
class A implements C {}
@First()
class C {}
@first
class D {}
```
Here, the macros on `A`, `C` and `D` run before the macros on `B`, and `C`
also runs before `A`. But otherwise the ordering is not defined (it is not
observable).
This only applies to Phase 2, because it is the only phase where the order
would be observable. In particular this allows macros running on `B` to see
any members added to its super classes, mixins, or interfaces, by other
macros running in phase 2.
Aside from these rules, macro introspection is limited so that evaluation order
is not user visible. For example, if two macros are applied to two methods in
the same class, there is no way for those macros to interfere with each other
such that the application order can be detected.
Aside from these rules, macros are constrained so that the result is the same
whatever the application order. In most cases this achieved by the split into
phases: within each phase macros can run in any order because the output is not
visible to other macros until the next phase. As a special case, introspection
of types in Phase 2 waits as needed for other macro applications to complete,
failing if there are cycles.
### Augmentation library structure and ordering
Expand Down Expand Up @@ -606,17 +582,18 @@ of a function or initializer for a variable. It is encouraged to provide a body
(or initializer) if possible, but you can opt to wait until the definition phase
if needed.

When applied to a class, enum, or mixin a macro in this phase can introspect on
all of the members of that class and its superclasses, but it cannot introspect
on the members of other types. For mixins, the `on` type is considered a
superclass and is introspectable. Note that generic type arguments are not
introspectable.

When applied to an extension, a macro in this phase can introspect on all of the
members of the `on` type, as well as its generic type arguments and the bounds
of any generic type parameters for the extension.
Phase two macros can introspect on all of the members of a type. If the type
to be introspected is declared in the same library cycle and has one or more
macros applied to it then this introduces an ordering constraint between the
macro applications: the introspection call waits for complete results before
returning, meaning it waits for the macro applications on the target type to
finish.

TODO: Define the introspection rules for extension types.
If a cycle arises in macro applications waiting for other macro applications to
complete then this is an error. The introspection call throws
`MacroDependencyCycleError`. The macro implementation may catch the error

This comment has been minimized.

Copy link
@jakemac53

jakemac53 Nov 21, 2023

Contributor

I don't think we need to require them to rethrow the error? If they can handle it reasonably, it should be fine for them to do so?

This comment has been minimized.

Copy link
@davidmorgan

davidmorgan Nov 22, 2023

Author Contributor

The problem with allowing them to handle the error is that it makes the whole error handling path part of the macros feature--it has to be deterministic, from the order in which the macros receive the exception to the details of the error that gets thrown (all data attached, in case they take action based on it) to how they handle it; what if they loop calling the introspect in the hope that other macros give up causing it to eventually succeed? If any of the macros has an "await" in that recovery code then the outcome would change each time no matter what we do.

So I suggest we require that any successful build be exception free, if macros need information currently only available by try/catch it should move into an API.

What do you think? Thanks.

This comment has been minimized.

Copy link
@jakemac53

jakemac53 Nov 22, 2023

Contributor

The problem with allowing them to handle the error is that it makes the whole error handling path part of the macros feature--it has to be deterministic, from the order in which the macros receive the exception to the details of the error that gets thrown (all data attached, in case they take action based on it) to how they handle it;

Determinism is exactly why the APIs are the way they are today is to ensure deterministic results. I don't believe we can design a solution that loses that determinism, it is a critical design feature.

I think this is solvable though. We just say that these dependencies that are set up between macros do not disappear when the macro completes running. This would ensure repeated calls would keep getting the same failure, even if the other macro handled the same error and bailed out.

what if they loop calling the introspect in the hope that other macros give up causing it to eventually succeed?

Assuming my suggestion above, this is never useful and will loop forever.

If any of the macros has an "await" in that recovery code then the outcome would change each time no matter what we do.

Same, this is why it must be deterministic, so that such race conditions can't exist. If this is possible, it implies much more general race conditions are at play.

This comment has been minimized.

Copy link
@davidmorgan

davidmorgan Nov 23, 2023

Author Contributor

Thanks! Agreed that returning the same result on repeated calls is an improvement, done.

I am still nervous about asking macros to optionally resume after an exception: exceptions are usually off the "happy path" for an API so I would expect quality to drop. E.g. people might catch in the wrong try/catch block, ..., there are no checked exceptions here ;)

What do you think about a "ResultOr" pattern instead?

Updated with that proposal and sent as an actual PR :) thanks!

#3480

only to report diagnostics to the user. If it does, it must then rethrow the
error with `rethrow`.

### Phase 3: Definitions

Expand All @@ -629,8 +606,8 @@ function body.
Phase three macros can add new supporting declarations to the surrounding scope,
but these are private to the macro generated code, and never show up in
introspection APIs. These macros can fully introspect on any type reachable from
the declarations they are applied to, including introspecting on members of
classes, etc.
the declarations they are applied to without introducing application ordering
constraints as in Phase 2.

## Macro declarations

Expand Down Expand Up @@ -696,6 +673,8 @@ For example, in `ClassDeclarationsMacro`, the introspection object is a
to the immediate superclass, as well as any immediate mixins or interfaces,
but _not_ its members or entire class hierarchy.

TODO: update this example.

### Builder argument

The second argument is an instance of a [builder][] class. It exposes both
Expand All @@ -707,6 +686,8 @@ primary method is `declareInClass`, which the macro can call to add a new member
to the class. It also implements the `ClassIntrospector` interface, which allows
you to get the members of the class, as well as its entire class hierarchy.

TODO: update this example.

[builder]: https://github.com/dart-lang/sdk/blob/main/pkg/_fe_analyzer_shared/lib/src/macros/api/builders.dart

### Introspection API ordering
Expand Down

0 comments on commit 1b43d3c

Please sign in to comment.