dart-lang / language

Design of the Dart language
Other
2.65k stars 202 forks source link

[macros] Third party resolution. #3605

Closed modulovalue closed 3 months ago

modulovalue commented 8 months ago

Consider that, e.g., GitHub has support for surfacing resolution information https://github.blog/2021-12-09-introducing-stack-graphs/ if a name resolution spec is available in the form of a stack graph spec: https://github.com/github/stack-graphs/tree/main/languages

How are third parties, such as GitHub, expected to resolve Dart in a post-macro world? It looks like the language team expects third parties to embed a Dart compiler (+ bindings to the analyzer) into their name resolution implementation. Is that correct?

jakemac53 commented 8 months ago

It is correct that in order to understand all the references in a Dart program you will have to execute macros, absolutely.

That output is represented like a normal Dart file though, and could be serialized to disk. This is exactly how we intend to support the equivalent of what you are talking about internally in our online code browsing/searching tools.

It is a bit more complicated for GitHub projects though for sure, but not insurmountable. The files may not be the exact canonical output, because that output can be dependent on your transitive version solve at any moment. We don't have this issue internally because we have a mono_repo with pinned versions of all our deps. Checking in a pubspec.lock could help, but packages are not expected to do that today. Ultimately, it probably doesn't matter much, and tbh it might be a good thing for a dart package to know that their generated output changed as a result of upgrading some dependency?

modulovalue commented 7 months ago

It is a bit more complicated for GitHub projects though for sure, but not insurmountable.

@jakemac53 can you clarify why you believe that to be the case?

Consider, for example, an update to a package like freezed. Each update could potentially result in hundreds of thousands of macro-caches being invalidated, just for one change, to one package. Since macros are turing complete, there's, in general, no way to know what is being invalidated without running the macro.

Given the fact that no pubspec.lock files are checked into source control by default, things are getting much worse. The worst case of possible configurations is the cartesian product of all versions of all packages that a project depends on.

I do not see how this exponential blowup in possible configurations can be remedied with the current design. The issue does appear to be insurmountable to me.

jakemac53 commented 7 months ago

It is really no different than build_runner today. That has exactly the same properties, the only real difference being the files are more typically checked in. The reason this works out fine, is that most of the time an update to a dependency does not change your generated code, even though it could.

It is not the end of the world if your generated code that you have checked into your repo is slightly out of date, either. It is there just to be an aid, and technically it is accurate for some version solve in any case.

If you want your generated code to be kept up to date, you can run a job that simply regenerates it with your latest deps, and checks if there is any diff, failing (and notifying you via some mechanism) if it did change. This is equivalent to the build_verify package today for build_runner.

modulovalue commented 7 months ago

@jakemac53 I don't feel like you've properly addressed my previous comment.

Your claim was:

It is correct that in order to understand all the references in a Dart program you will have to execute macros, absolutely. [...] It is a bit more complicated for GitHub projects though for sure, but not insurmountable.

I disagree. I claim that this problem is insurmountable and have provided an example situation in my previous comment https://github.com/dart-lang/language/issues/3605#issuecomment-1929599824

To be clear, do you still claim that it isn't an insurmountable problem or do you agree that having macros (as a first-class language feature) makes it impossible to guarantee that fast, efficient and correct resolution across a large corpus of code (that does not employ the google-monorepo-model) becomes insurmountable?


It is really no different than build_runner today. That has exactly the same properties, the only real difference being the files are more typically checked in.

That sounds very different to me. build_runner is an opt-in tool, whereas the macro proposal would introduce a language feature without any opt-out mechanisms.

Any project using build_runner, that chooses not to check in generated code, checks in an incomplete program that does not resolve until a third party tool is run.

In a post-macro-world, any project using macros that chooses not to check in generated code becomes a complete program (until proven otherwise) that can't run.

If we assume that resolution is decidable, then it is decidable to say whether a program is "complete" by trying to resolve it. This is an excellent property to have, because it guarantees that compilation + the analyzer can be guaranteed to always be fast and efficient. In a post-macro-world, it becomes undecidable whether a program is "complete" because resolution itself becomes undecidable due to the new dependency on a turing complete step.

The current macro proposal would remove desirable properties that Dart already has. We could no longer guarantee efficient resolution.


It is not the end of the world if your generated code that you have checked into your repo is slightly out of date [...] If you want your generated code to be kept up to date [...]

I really don't like the point of view of calling checked in code "out of date" because build_runner might produce something different the next time it is run. build_runner is a tool that might update the code. It only starts to become out of date once build_runner outputs something different. build_runner, or any other code generator, doesn't have to always be run. That's just a preference. It's not out of date until the code generator produces a different output.

jakemac53 commented 7 months ago

I agree that it is not practical, or even possible, to provide a perfect view of macro generated code as text, on github. This is a fundamental truth as long as versions of dependencies are not pinned, because there are many different possible valid versions of the code, based on all the possible valid version solves.

That does not mean however that it is insurmountable to provide a good experience in the face of this truth. As long as you allow yourself to squint a little bit, I think you will realize that the vast majority of real world use cases will end up perfectly fine (assuming users are checking in generated code and periodically updating it).

I would even go far as to suggest that our existing semantic versioning, if followed properly, solves this problem for us almost entirely (there may be some edge cases, but they should be rare). This is because:

There are some obvious failure modes here, but even when they do occur the result is not very bad:

I put "out of date" in quotes above because in these scenarios, it really isn't "out of date", because it is only so for the absolute latest version solve. It is still accurate for some previous version solve. This whole scenario is generally problematic, and macros which encounter these scenarios will generally not behave well, and any published package using a macro which behaves this way is going to get in all kinds of trouble, since it means their API is actually fluid, so they can't use semver properly at all. This might sound scary, but in practice I do not think it is worth blocking all the valid use cases of macros just to try to prevent these bad uses. The ecosystem will figure it out, and macros like that will not be popular.

Regarding the efficiency of compiling/resolving Dart code, the feature has been designed as much as possible to not affect things more than necessary, while providing enough power to be a useful feature. I don't have much else to say about that, it will be up to macro authors not to write macros that do unbounded amounts of work.

leafpetersen commented 7 months ago

How are third parties, such as GitHub, expected to resolve Dart in a post-macro world? It looks like the language team expects third parties to embed a Dart compiler (+ bindings to the analyzer) into their name resolution implementation. Is that correct?

I have to admit I'm a little lost as to what the goal is here. You flat out cannot correctly resolve symbols in Dart without implementing the full type system. Type inference, type promotion, and extension method resolution are all intertwined, and all of them are necessary in order to determine what symbols resolve to. So yes, if you want to correctly resolve symbols (with, or without macros) you must embed an implementation of the Dart type system. What am I missing?

jakemac53 commented 7 months ago

Note that all the concerns here also apply to dartdoc API docs on pub, cc @sigurdm @jonasfj.

It is technically possible, that any time a package is published, that could affect the API of any other package that depends on it, if they are using particularly finicky macros.

I don't know that we should directly do anything about that, because any time it happens it really indicates a much larger problem. But it is a thing, technically. Maybe, giving authors the ability to regenerate their API docs on demand would be enough to cover some of these rare scenarios?

jonasfj commented 7 months ago

Note that all the concerns here also apply to dartdoc API docs on pub,

If package foo depends on package bar we will also regenerate documentation for foo when a new version of bar is published. There are some limits to this logic:

We do periodically regenerate API docs for a package after 31 days. So eventually, things will become consistent. We could probably regenerate more frequent, but current logic seems to be working reasonably.

jakemac53 commented 7 months ago

Interesting ok, it sounds like this won't be an issue then 👍

munificent commented 6 months ago

It is technically possible, that any time a package is published, that could affect the API of any other package that depends on it, if they are using particularly finicky macros.

I wonder if we should consider explicitly prohibiting this. We could restrict the introspection API such that you aren't allowed to introspect on declarations outside of the package containing the macro application. We'd probably also want to give you the ability to introspect on stuff in dart: libraries and the package where the macro itself is defined.

That would rule out possibly valid things a macro author might want to do. But it would mean that the results of macro applications are always local to a package, independent of its dependencies, and potentially serializable with with the package.

It's probably too much of a restriction, though.

Another approach might be to dynamically track during macro execution what declarations the macro introspects over and where they come from. If we see that the macro applications in a package don't introspect on any declarations from outside of that package, then we know it's macro-generated code is local to the package.

I'm not sure if there's anything useful here, but I wanted to write it out just in case.

jakemac53 commented 6 months ago

I don't think it would be a good trade-off to block introspection of things from other packages. That would punish you for splitting your app up into small packages, like how we build apps internally. And yet there would be no benefit at all for internal users as this problem doesn't exist in a mono repo scenario.