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

How do we restrict core libraries for macros? #1916

Open
munificent opened this issue Oct 20, 2021 · 5 comments
Open

How do we restrict core libraries for macros? #1916

munificent opened this issue Oct 20, 2021 · 5 comments
Labels
request Requests to resolve a particular developer problem

Comments

@munificent
Copy link
Member

The macro proposal says:

Only the core libraries which don't violate the above rules are allowed... All other SDK libraries are not available.

What does this mean precisely? Some options:

  1. It is a compile-time error for the macro class created by a macro application to be declared in a library that contains an import of any prohibited library, imports any other library (transitively) that contains a prohibited import.

  2. It is a compile-time error for a macro class to refer to anything imported from a prohibited library. In other words, it's like the libraries exist but are completely empty.

  3. It is a macro execution-time error to call any function or method in a prohibited library. In other words, it's like the libraries exist but all function and method bodies are throw UnsupportedError().

I think we will also want to restrict some functionality in libraries that are otherwise allowed. For example, dart:core is allowed, but DateTime.now() should not be (#1915). Given that, option 3 might be the most consistent choice. In other words, all libraries are available but some functionality throws a runtime error. And it just happens to be that in some libraries like dart:isolate that all functions throw runtime errors.

I think there is prior art here for how things like dart:io are handled on Flutter that we can consult.

@munificent munificent added the request Requests to resolve a particular developer problem label Oct 20, 2021
@Levi-Lesches
Copy link

Options 1 and 2 definitely sound more "proper" than number 3. Making every function throw a runtime error, while maybe consistent with the potential DateTime.now() and Random behavior, doesn't indicate at all that those seemingly-normal core functions will throw. So not only do you lose static errors about imports, but you don't gain any new information, and now have to treat every function as though it may throw.

Based on that I'd say option 1. Option 2 is safe, but then writing an import line is essentially dead code, so option 1 would cover that base too.

@jakemac53
Copy link
Contributor

jakemac53 commented Oct 20, 2021

I have always assumed we would do option 1, and use the existing libraries.json files to support it, so its essentially just another platform, with its own set of available libraries.

In practice though, if we run macros inside the compiler isolate itself, that might be a bit weird to deal with enforcing.

@munificent
Copy link
Member Author

Option 1 works for me too if we can pull it off. I do worry that it can end up being too conservative. If any library in the entire transitive import graph from the macro definition imports a forbidden library—even if it doesn't use it—the macro won't compile.

That's more or less the situation with libraries like dart:html and dart:io for their unsupported platforms and we seem to get by, so it's probably doable for macros. On the other hand, config-specific imports do hint that the solution doesn't work very well either.

@cedvdb
Copy link

cedvdb commented Oct 20, 2021

Disabling DateTime.now() and Random() seems like a safer option in the sens that if there is a need for those when the feature is introduced, it won't be a breaking change to "whitelist". Allowing those from the get go sets a precedent.

From my naive point of view, static meta programming will be mainly used by code that is not complex but solves repetitive tasks, which does not require external libraries. That's a big assumption on my part but you could judge the need for the unrestrictiveness better by first being overly restrictive.

@lrhn
Copy link
Member

lrhn commented Oct 25, 2021

I think option 1 is the right (and achievable) approach. A platform library is either available or not, and if we want to prevent dart:ffi (Do we?), then we can. Code written to be used as a macro will know its restrictions. Other libraries should not import platform libraries unless they need them.

I'd very much prefer if the core platform libraries are available: core, math, convert, collection, async, typed_data.

We possibly want developer available so you can profile your macro code.

I fully expect people to request dart:ffi so they can use native code to provide data for the generated code (say a tensor-flow compiler for generating tables).
(I'd probably be happy to allow any library supported by the stand-alone VM, or which is already used by our compiler.)

On the other I'd be very worried about adding restrictions on the features of individual platform libraries (option 3).

I see no good way (or reason) to disable DateTime.now or Random.
Trying to prevent that seems like a game of Sisyphusian whack-a-mole (for example, can you even use List when List.shuffle uses Random?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

5 participants