Chicken and egg problem when evolving `dart:_macros` library #54976

jakemac53 · 2024-02-21T19:28:42Z

See https://dart-review.googlesource.com/c/sdk/+/353561, this exercises a pattern where we both add a new API to dart:_macros and expect to be able to use it in the compilers, atomically.

In particular, I added the exception types to dart:_macros in this CL, and migrated analyzer/CFE to use package:macros which exports dart:_macros at the same time. They expect these new exception APIs to exist, but they don't.

Presumably this is related in some way to the pre-built SDK, but I don't fully understand all the context here.

I could likely avoid the issue in this case, but we will have this same pattern but not be able to avoid it in the future.

cc @johnniwinther @whesse

The text was updated successfully, but these errors were encountered:

bwilkerson · 2024-02-21T19:52:12Z

I don't see where macros is mentioned in the DEPS file, which means that it isn't in the package_config.json file against which the SDK code is analyzed / compiled.

Aside from that, you might want to consider developing package:macro in the Dart SDK. I suspect that doing so would prevent this and several other problems.

jakemac53 · 2024-02-21T20:06:45Z

I don't see where macros is mentioned in the DEPS file, which means that it isn't in the package_config.json file against which the SDK code is analyzed / compiled.

See the relation chain, it is added one CL earlier :).

All the APIs are actually coming from dart:_macros though. This is actually what causes the problem. If it was purely a package it wouldn't be an issue. But dart: libraries are to some extent pre-compiled or something. I don't fully understand how the pre-built SDK exactly interacts with them. But APIs are not immediately available to the compiler itself, is what I am observing.

jakemac53 · 2024-02-21T20:24:00Z

One idea here, but it might be hard to implement, is to somehow extract out the support for running macros such that it can compile the platform without referencing any macro related types (no imports at all to package:macros and thus no transitive ones to dart:_macros).

I don't know how feasible that would be though.

bwilkerson · 2024-02-21T20:30:01Z

See the relation chain, it is added one CL earlier :).

Ah, I missed that, sorry.

I have to wonder whether it might be cause by the chaining. That is, I wonder whether the second CL would be having a problem if the first had landed so that the SDK being used to analyze the second CL already had the package defined (in package_config.json).

dart-github-bot · 2024-02-21T20:45:14Z

Item	Details
Summary	Atomically adding an API to `dart:_macros` and expecting to use it in compilers causes an error.
Triage to	`area-library`, `area-analyzer`

(what's this?)

jakemac53 · 2024-02-21T21:13:37Z

That is, I wonder whether the second CL would be having a problem if the first had landed so that the SDK being used to analyze the second CL already had the package defined (in package_config.json).

Most of the package works fine - it is only the new APIs added to it which don't work.

mraleph · 2024-02-21T21:24:42Z

The error makes sense: currently when bootstrapping we will to run HEAD CFE on the prebuilt dart - which does not contain any dart:_macros in the core libraries. This means CFE can't really use any Dart language features or APIs which are not yet in the core-libraries of the prebuilt dart.

For Dart only code (i.e. things that don't require or depend on VM changes) we can probably make it work by having a special mode where we compile HEAD CFE using core library source rather than precompiled platform embedded into prebuilt dart - but that will still break if compiling CFE requires invoking macros and there is an API skew between prebuilt CFE and macroses at HEAD.

jakemac53 · 2024-02-21T21:27:08Z

It is, at least from my perspective, acceptable if we cannot use any macros in the CFE itself.

mraleph · 2024-02-22T12:42:25Z

We have discussed this problem with @johnniwinther a bit.

@jakemac53 Before I jump into nitty gritty details of how we could change bootstrapping I want to ask metaquestion: are we sure we want to make dart:_macros part of the platform file (i.e. core libraries)?. This seems to be compiler API rather than platform API and as such I find it a bit suspicious that we include it there. Consequently I would think it should live in a separate place.

Can we make it a builtin package instead (e.g. package the import of which is handled in a special way by the tooling)?

e.g. if you have you have import 'package:_macros/macros.dart' then

If package _macros is resolvable through package_config.json - then just use that.
Otherwise load from a special tool-specific builtin location.

Taking one step further: package_config.json could support referring to special builtin packages using custom scheme,

    {
      "name": "_macros",
      "rootUri": "dart-builtin://_macros/",
      "packageUri": "lib/",
      "languageVersion": "3.2"
    },

Then you don't need otherwise clause at all (but you would need pub changes - which I think are not that big, we already have special wiring for Flutter SDK there).

Resolving bootstrapping problem

First let me summarize the build process as we have it now:

We have prebuilt Dart SDK at version v1 which contains a dart binary which contains inside VM, as well as prebuilt CFE and platform file (core libraries in Kernel binary form).
We have CFE and Dart core libraries (aka platform) in source form checked out at version v2.

I am gonna write $\mathrm{CFE}_{v1}^S$ to denote CFE in source form ($S$) at version $v1$, similarly $\mathrm{Plat}_{v1}^K$ would mean platform precompiled to Kernel¹ ($K$) at version $v1$.

The current bootstraping process goes like this: we want to produce Kernel binary from platform sources (compile $\mathrm{Plat}_{v2}^S$ to $\mathrm{Plat}_{v2}^K$). To do that we try to execute $\mathrm{CFE}_{v2}^S$ (CFE v2 from source) on prebuilt dart binary and give it platform v2 sources:

$$\mathrm{Plat}_{v2}^K = [\mathrm{CFE}_{v2}^S / \mathtt{dart}_{v1}](\mathrm{Plat}_{v2}^S)$$

Executing $\mathrm{CFE}_{v2}^S$ on $\mathtt{dart}_{v1}$ requires compiling it to Kernel first and then running the result of that on $\mathrm{VM}_{v1}$:

$$\begin{align} \mathrm{CFE}_{v2}^K &= [\mathrm{CFE}_{v1}^K / \mathrm{VM}_{v1}](\mathrm{CFE}_{v2}^S + \mathrm{Plat}_{v1}^K) \\\ \mathrm{Plat}_{v2}^K &= [\mathrm{CFE}_{v2}^K / \mathrm{VM}_{v1}](\mathrm{Plat}_{v2}^S) \end{align}$$

Attempt to compile $\mathrm{CFE}_{v2}^S$ using $\mathrm{Plat}_{v1}^K$ is the step that fails.

The work around which I previously described would be to change this step to:

$$\begin{align} \mathrm{CFE}_{v2}^K &= [\mathrm{CFE}_{v1}^K / \mathrm{VM}_{v1}](\mathrm{CFE}_{v2}^S + \textcolor{red}{\mathrm{Plat}_{v2}^S}) \\\ \mathrm{Plat}_{v2}^K &= [\mathrm{CFE}_{v2}^K / \textcolor{red}{\mathrm{VM}_{v2}}](\mathrm{Plat}_{v2}^S) \end{align}$$

That is instruct $\mathrm{CFE}_{v1}^K$ to compile using v2 platform from source, rather than precompiled v1 platform from Kernel. At this point we would also want² to run the resulting $\mathrm{CFE}_{v2}^K$ on freshly built $\mathrm{VM}_{v2}$ rather than running it on $\mathrm{VM}_{v1}$ embedded into prebuilt $\mathtt{dart}_{v1}$ binary.

Currently prebuilt CFE is hidden inside the monolithic dart binary - it is part of kernel-service. I think we should instead produce a separate bootstrapping compiler tool $\mathtt{bc}$³ which supports compiling Dart source against source platform for bootstrapping purposes. Then bootstrapping could look like this⁴:

$$\begin{align} \mathrm{CFE}_{v2}^{K_{v1}} &= \mathtt{bc}_{v1}(\mathrm{CFE}_{v2}^S + \mathrm{Plat}_{v2}^S) \\\ \mathrm{Plat}_{v2}^{K_{v2}} &= [\mathrm{CFE}_{v2}^{K_{v1}} / \mathrm{VM}_{v2}](\mathrm{Plat}_{v2}^S) \\\ \mathrm{CFE}_{v2}^{K_{v2}} &= [\mathrm{CFE}_{v2}^{K_{v1}} / \mathrm{VM}_{v2}](\mathrm{CFE}_{v2}^S + \mathrm{Plat}_{v2}^{K_{v2}}) \end{align}$$

Here $\mathrm{Plat}_{v2}^{K_{v2}}$ and $\mathrm{CFE}_{v2}^{K_{v2}}$ are the final artifacts we want to ship with the SDK.

Strictly speaking Kernel binary format itself is versioned but we assume for simplicity that kernel format did not change between v1 and v2. ↩
The reason for running it on v2 VM is to avoid version skew between platform which was used for compilation and the VM which is used to run the resulting Kernel blob. Otherwise boostraping would break, for example, when we make changes to native parts of dart:io. This however makes an assumption that Kernel produced by v1 CFE is runnable on v2 VM. ↩
Note that we assume that v1 Kernel format can run on v2 VM. This means that bc tool itself can just be distributed as a v1 Kernel binary rather than a native executable. This unlocks bootstrapping on platforms which don't have prebuilt dart binary available. See also https://github.com/dart-lang/sdk/issues/51788 ↩
Here I made a choice to highlight that the Kernel produced by older CFE might not be the same as Kernel produced by newer CFE and we don't want to ship older Kernel with newer SDK. ↩

jakemac53 · 2024-02-22T16:12:22Z

are we sure we want to make dart:_macros part of the platform file (i.e. core libraries)?

The important part, is that the version of the library is identical between the binaries shipped with the SDK and the users local version solve. You are correct in saying that if we supported SDK vendored packages in Dart (similar to Flutter), we could achieve this.

I am trying to not invent more work than is necessary though, because the feature is already large enough, and every additional feature adds additional risk and complexity. We already have dart: libraries, and they already achieve this same goal.

Note that I would also not want this to work how Flutter SDK packages work. They allow pub package dependencies, which is IMO a mistake (massive pain for both owners and users of those packages, they all become in effect SDK vendored due to pinning). Only other SDK vendored dependencies should be allowed. We also want proper versioning, that is the whole point of having the package as a layer on top of the dart:_macros library. The current vendoring strategy that flutter uses does not allow for versioning at all.

My question would be, what advantage does being a package provide over a dart: library? I am not currently aware of advantages other than being able to have pub deps, which I strongly believe to be a mistake and wouldn't want to replicate. It seems to me to be basically just a way to always have the dependencies come from source, and not much different from providing the platform from source.

If it allowed us to use macros in the CFE or something I would possibly be in favor, but I don't believe it would, we still have the issue of the package being pre-compiled as a part of the CFE in the pre-built dart binary. So the protocol would have to match up exactly in order to compile the CFE.

mraleph · 2024-02-22T21:31:56Z

My question would be, what advantage does being a package provide over a dart: library?

My point is that it does not have to be a real package - it just that package:-scheme already has flexibility of being retargetable via package config and that is something that all tools understand. I concede that package: does scheme does not have all the functionality we need so we will have to implement some additional stuff - and it might not be worth it for this particular case (though it might be worth it as a general feature).

I think what is more important here is that _macros does not look like something that belongs to the platform (core libraries) to begin with: it is part of the compiler itself and not interface to the execution platform.

When compiling the compiler dart:_macros have to resolve to the source that lives next to the compiler. When compiling with the compiler dart:_macros have to resolve to the library shipped with the compiler. Maybe this can be just special case for dart:_macros in the CFE, but it does seem like it is loosely related to the existing concepts.

This made me think that another option is to simply redirect package:macros itself in the SDK environment, e.g. we could do:

diff --git a/sdk/lib/_macros/api.dart b/sdk/lib/_macros/api.dart
new file mode 100644
index 00000000000..e9b4f9b2ac7
--- /dev/null
+++ b/sdk/lib/_macros/api.dart
@@ -0,0 +1 @@
+export 'macros.dart';
diff --git a/tools/generate_package_config.dart b/tools/generate_package_config.dart
index 3f9a24207b0..9dee64683d1 100644
--- a/tools/generate_package_config.dart
+++ b/tools/generate_package_config.dart
@@ -148,6 +148,12 @@ Iterable<Package> makePackageConfigs(List<String> packageDirs) sync* {
     var hasLibDirectory =
         Directory(join(repoRoot, packageDir, 'lib')).existsSync();
 
+    // Hack: reroute macros package to point directly into SDK sources.
+    if (name == 'macros') {
+      packageDir = 'sdk/lib/_macros';
+      hasLibDirectory = false;
+    }
+
     yield Package(
       name: name,
       rootUri: packageDir,

And then your CL should build just fine.

jakemac53 · 2024-02-22T22:15:48Z

I think what is more important here is that _macros does not look like something that belongs to the platform (core libraries) to begin with: it is part of the compiler itself and not interface to the execution platform.

I don't know if we have anything written down for what a dart: library is appropriate to be used for. But, I think that when you have a library which only works with a certain exact SDK version, it should be appropriate? There is a lot of coupling here because the library is compiled as a part of pre-built SDK binaries and must have the same version as the user generated macro code.

This made me think that another option is to simply redirect package:macros itself in the SDK environment, e.g. we could do:

That is almost crazy enough to work.... but it would be brittle. Package:macros will have two different libraries, one for users and one for implementations. Those each export only specific things from dart:_macros. So the libraries would look a bit different even if the package URIs matched up. Unless I duplicated all the same exports in the SDK libs but that would be a lot of work to do for little value I think.

I might still try it anyways though.

mraleph · 2024-02-23T08:57:25Z

But, I think that when you have a library which only works with a certain exact SDK version, it should be appropriate?

It's just a question of what is it coupled with. It seems that platform files are more coupled with the target platform than with the compiler. When we look at dart:io it is something clearly coupled with the VM. But executing macros is the ability of CFE rather than VM - which means dart:_macros is an artifact coupled with CFE rather than VM. This makes putting into the platform awkward. But anyway - I understand that it is easier at the current stage to just put it there.

That is almost crazy enough to work.... but it would be brittle. Package:macros will have two different libraries, one for users and one for implementations. Those each export only specific things from dart:_macros. So the libraries would look a bit different even if the package URIs matched up. Unless I duplicated all the same exports in the SDK libs but that would be a lot of work to do for little value I think.

Here is another concoction which removes the need to have a copy of api.dart in the core libraries location, but this requires actual changes to prebuilt SDK to really make it work nicely:

diff --git a/pkg/macros/lib/api.dart b/pkg/macros/lib/api.dart
index 008a1da2b1d..293b61d1088 100644
--- a/pkg/macros/lib/api.dart
+++ b/pkg/macros/lib/api.dart
@@ -12,7 +12,9 @@
 // only want to expose the public portions from this library, and use explicit
 // shows to do that.
 
-export 'dart:_macros'
+// TODO: this should really be dart.library._macros, but there is no plumbing
+// in prebuilt SDK to do `--no-enable-macros`.
+export 'package:_macros/macros.dart' if (dart.library.mirrors) 'dart:_macros'
     show
         Annotatable,
         Builder,
diff --git a/tools/generate_package_config.dart b/tools/generate_package_config.dart
index 3f9a24207b0..54c5acb8c74 100644
--- a/tools/generate_package_config.dart
+++ b/tools/generate_package_config.dart
@@ -85,6 +85,12 @@ void main(List<String> args) {
     ...makeFrontendServerPackageConfigs(frontendServerPackageDirs),
     ...makePkgVmPackageConfigs(pkgVmPackageDirs),
     ...makePackageConfigs(sampleDirs),
+    Package(
+      name: '_macros',
+      rootUri: 'sdk/lib/_macros',
+      packageUri: null,
+      languageVersion: '3.4',
+    ),
   ];
   packages.sort((a, b) => a.name.compareTo(b.name));
 
diff --git a/utils/compile_platform.gni b/utils/compile_platform.gni
index 0523ac32039..ce0d26ab6f2 100644
--- a/utils/compile_platform.gni
+++ b/utils/compile_platform.gni
@@ -50,7 +50,12 @@ template("compile_platform") {
 
     outputs = invoker.outputs
 
-    vm_args = [ "-Dsdk_hash=$sdk_hash" ]
+    vm_args = [
+      "-Dsdk_hash=$sdk_hash",
+      # TODO: this should really be --no-enable-macros but we don't
+      # have plumbing for this.
+      "--no-enable-mirrors"
+    ]
 
     inputs = []
     deps = []

sortie · 2024-02-23T10:42:21Z

IMO it's a problem that package:macros is tightly with the dart sdk version:

environment:
  # Restrict the upper bound to the next release, which allows us to do breaking
  # changes if required, in minor SDK releases.
  sdk: ">=3.4.0-0 <3.5.0"

You see, when we want to bump the version number to e.g. Dart 3.5.0, we'll have to pub publish a new package:macros before we are able to even begin development of Dart 3.5.0. The moment anything serious starts to depend on package:macros, we will have to make the package:macros for the next release, before we even have begun making that release.

It's already complicated enough to bump the Dart version number due to issues like this, and we've been working hard to untangle and reduce this class of issues. The 3.2.0 version bump was delayed a week and even reverted due to a similar issue. We don't have a good way to tightly couple package versions with dart sdk versions at this time and trying to do so will complicate release engineering and doesn't lead to a nice stable design for anyone.

The good solution we have available to tightly version code with the Dart SDK is the dart: imports that are versioned with the Dart SDK. That's what is already being used with the dart:_macros except it's not public. Instead there's a package:_macros that just exports all the contents of dart:_macros without any further logic, essentially making dart:_macros public.

It would be easier and simpler to accomplish the same thing by just having dart:macros and not having a package. Then the contents are properly versioned along with the Dart SDK. To bootstrap new features, you would add them, bump the checked in sdk, and use them (just like with any other core library).

Is the concern with dart:macros that it may change and you're trying to not be subject to the breaking change policy? IMO that policy is there for a reason and we should be think through the design of new features so we can live with them for a while. We go through the policy all the time and remove smaller features which could be done for deprecated features in dart:macros too. If it's that unstable, we can also guard it behind an experimental flag. Alternatively we can amend the policy to say new libraries such as dart:macros are not fully supported across releases just yet. Or perhaps we could ship some builtin packages along with the Dart SDK although that is what the dart: imports are.

TLDR: macros being a tightly dart core library and a package complicates the design and release engineering and there are simpler options.

jakemac53 · 2024-02-23T15:24:03Z

Please read https://github.com/dart-lang/language/blob/main/working/macros/feature-specification.md#language-and-api-evolution for the justification of using a package and the tight constraints.

I do believe that it has strong benefits over being a public dart: library, even though they are subtle.

you're trying to not be subject to the breaking change policy?

We are trying to be able to version separately from the SDK. This isn't about subverting the policy, but having our own versioning which is separate.

It is very possible that this library would have breaking changes which are a result of changes to the language, not something we simply desired to do. They can essentially be forced upon us by changes that aren't otherwise breaking for the language.

jakemac53 · 2024-02-23T20:56:45Z

@mraleph I was able to get things building by just hacking the package config for package:macros (https://dart-review.googlesource.com/c/sdk/+/353561/6/tools/generate_package_config.dart).

I am not sure how palatable that solution is though, cc @johnniwinther @scheglov

jonasfj · 2024-02-26T15:10:31Z

We are trying to be able to version separately from the SDK.

Well, in effect this is giving the SDK multiple version numbers.

The SDK version number, and,
The version number for package:macro.

Which means you can do a major version bump on one without having to do it on the other.
I can see it having pros and cons, but also it's not a decision that can't be changed later, when/if the macros API becomes super stable.

On topic, have you considered doing SDK packages, similar to how Flutter does it?

So that users write:

name: my_macro_pkg
dependencies:
  macros:
    sdk: dart
    version: ^1.2.0   # Yes, you can specify version constraint on SDK packages

That would solve any chicken egg problems. It's ugly, but there isn't really much of a reason to pretend that you can get a different version of package:macro than what is in the Dart SDK.

jakemac53 · 2024-02-26T15:21:52Z

On topic, have you considered doing SDK packages, similar to how Flutter does it?

What would it take to get support for this? We could do it that way, if we have versioning support. I probably would want to write a lint or something that tells users to actually use the version too.

I would also want a lint to prohibit adding any package dependencies to this package.

jakemac53 · 2024-02-26T19:26:19Z

~~@jonasfj today if I try to have a version and sdk field, I get an error that path and version can't be used together :). Probably not hard to relax though?~~ Actually this looks to only be enforced in the test descriptor stuff, not actual pub code.

jakemac53 · 2024-03-22T18:07:07Z

We have landed on using SDK packages as a fix for this, I am working on landing that in https://dart-review.googlesource.com/c/sdk/+/359040.

add sdk_packages.yaml file (describes SDK vendored package locations) delete old macro code in _fe_analyzer_shared, move tests/benchmarks adds a top level `pkg` directory to the Dart SDK, which is where vendored packages live BUG: #54976 Change-Id: Ib3503a27fb5644fa8a39ab5a3e5b568df330cfd6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359040 Auto-Submit: Jake Macdonald <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Jonas Termansen <[email protected]> Reviewed-by: Johnni Winther <[email protected]> Reviewed-by: Bob Nystrom <[email protected]> Commit-Queue: Jake Macdonald <[email protected]> Reviewed-by: Devon Carew <[email protected]>

davidmorgan · 2024-06-04T16:22:02Z

Even though this solution works, so I'll close the issue, we are looking at decoupling by introducing a stable serialization format dart-lang/language#3706 and dart-lang/language#3872

jakemac53 added the feature-macros Implementation of the macros feature label Feb 21, 2024

github-project-automation bot added this to Static Metaprogramming - design/prototype Feb 21, 2024

github-project-automation bot moved this to Todo in Static Metaprogramming - design/prototype Feb 21, 2024

devoncarew added the area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). label Feb 21, 2024

davidmorgan closed this as completed Jun 4, 2024

github-project-automation bot moved this from Todo to Done in Static Metaprogramming - design/prototype Jun 4, 2024

Chicken and egg problem when evolving dart:_macros library #54976

Chicken and egg problem when evolving dart:_macros library #54976

Comments

jakemac53 commented Feb 21, 2024

bwilkerson commented Feb 21, 2024

Uh oh!

jakemac53 commented Feb 21, 2024

Uh oh!

jakemac53 commented Feb 21, 2024

Uh oh!

bwilkerson commented Feb 21, 2024

Uh oh!

dart-github-bot commented Feb 21, 2024

Uh oh!

jakemac53 commented Feb 21, 2024

Uh oh!

mraleph commented Feb 21, 2024

Uh oh!

jakemac53 commented Feb 21, 2024

Uh oh!

mraleph commented Feb 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Resolving bootstrapping problem

Footnotes

Uh oh!

jakemac53 commented Feb 22, 2024

Uh oh!

mraleph commented Feb 22, 2024

Uh oh!

jakemac53 commented Feb 22, 2024

Uh oh!

mraleph commented Feb 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sortie commented Feb 23, 2024

Uh oh!

jakemac53 commented Feb 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakemac53 commented Feb 23, 2024

Uh oh!

jonasfj commented Feb 26, 2024

Uh oh!

jakemac53 commented Feb 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakemac53 commented Feb 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jakemac53 commented Mar 22, 2024

Uh oh!

davidmorgan commented Jun 4, 2024

Uh oh!

Chicken and egg problem when evolving `dart:_macros` library #54976

Chicken and egg problem when evolving `dart:_macros` library #54976

mraleph commented Feb 22, 2024 •

edited

Loading

mraleph commented Feb 23, 2024 •

edited

Loading

jakemac53 commented Feb 23, 2024 •

edited

Loading

jakemac53 commented Feb 26, 2024 •

edited

Loading

jakemac53 commented Feb 26, 2024 •

edited

Loading