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

Towards a consistent handling of constant environment variables across all backends #50535

Open
joshualitt opened this issue Nov 22, 2022 · 6 comments
Labels
area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …).

Comments

@joshualitt
Copy link
Contributor

Users currently rely on constant environment variables(CEVs) for configuring their applications, as well as for ensuring tree shaking on some backends. These CEVs are currently not consistently handled across all backends. In particular, some backends support unevaluated constants and some do not. Furthermore, some backends support non-constant environment variables, but these are outside the scope of this issue.

This lack of consistency negatively impacts some of our users, particularly those who wish to invoke the CFE in a modular fashion. While the number of users negatively impacted by this issue is small, nonetheless for those users affected this is a very significant problem.

There are a number of different approaches we can take, some possibilities are:

  1. Status quo, users will have to deal with this pain. Some backends will support unevaluated constants, some won't.
  2. All backends support unevaluated constants. Users have the flexibility to invoke the frontend anyway they wish, but backend engineers bear the pain.
  3. We investigate some alternative to environment variables, that is handled consistently across all backends. For example, we can introduce a notion of a 'backend flag'. This would require a language change as well as backend work, but this could decouple us from all the legacy baggage around environment variables.
  4. We change the CFE to support partial environments, and environment reconciliation. This might be the most work, but it would be isolated to the CFE, and backends could be cleaned up.

@sigmundch @johnniwinther @eyebrowsoffire for FYI

@joshualitt joshualitt added the area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …). label Nov 22, 2022
@johnniwinther
Copy link
Member

@joshualitt Can you elaborate a bit on the 4th option?

copybara-service bot pushed a commit that referenced this issue Nov 23, 2022
Bug: #50535
Change-Id: Iabdaa1460c74d70a0ae44b60c5ecc9287a9740cf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/271562
Commit-Queue: Joshua Litt <joshualitt@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
@joshualitt
Copy link
Contributor Author

I'm no CFE expert, so take everything I'm about to say with a grain of salt.

What I'm imagining for option 4 is that when the CFE creates a dill, either outline or full, it will generate some metadata about the environment of the dill. Specifically, this metadata will include the defined CEVs and their values. When a dill has dependencies, the CFE will check the metadata of the dependencies against the current environment to ensure consistency.

To make this work, CEVs will have to have two states: defined and undefined. When a CEV is explicitly defined in some environment somewhere, it is considered to be defined, and any downstream consumers of that dill must either not explicitly define the CEV, or they must define it to the exact same value. When a CEV is not defined anywhere, it will remain undefined, but when evaluated by a backend's constant evaluator it will evaluate to it's default value.

Thoughts on the overall approach? I know I'm handwaving here, and what I'm proposing is probably fairly complex. I'm also happy to discuss offline if what I'm describing is unclear.

@sigmundch
Copy link
Member

I'd recommend taking another look at dart-lang/language#304 for context as well.

It took me a long time to find that issue unfortunately (issue searches are so hard!), but I believe it is very relevant to the discussion here. My guess is that (4) specifically would entail revisiting some parts of the discussion there, for example see dart-lang/language#304 (comment).

@joshualitt - could you clarify how something like (4) addresses the complexities here? What part of the backend inconsistency it addresses and how it alleviates the problems we are trying to solve?

@joshualitt
Copy link
Contributor Author

@sigmundch If the CFE can support partial environments, and environmental reconciliation, then we can remove unevaluated constants from the CFE, at least as a backend facing concept. As a result, all constants, including CEVs will appear from a backend perspective to be fully evaluated. This will enable clients, such as Flutter Web, to build a single dill with a partial environment defining only those constants they care about. Other constants can remain fully undefined, or defined in downstream consumers of the Flutter Web SDK, and the CFE will ensure everything is consistent.

Furthermore, by removing support for unevaluated constants, the handling of CEVs will be fully consistent, indeed fully opaque, to all backends. One notable exception to this is modular compilation. Modular compilers might need to handle undefined vs. defined constants differently. In the latter case, a modular compiler could leverage the value of the constant, while in the former case a backend would have to avoid reasoning about a constant until a definition for it appears.

That said, I believe our only modular compiler right now is DDC, and with this technique DDC would be able to support CEVs in the same manner as other backends. However, we might need to add runtime support for environments to make this work.

W.R.T. to the linked comment, I believe the method I propose fulfills the spirit of that comment. It will be the CFE's job to ensure environments are consistent. Practically speaking a given backend will only see a single consistent environment, regardless of how piecemeal that environment is defined.

@joshualitt
Copy link
Contributor Author

Just so we don't lose sight of the other alternatives, before we actually try and solve this issue we should investigate whether or not constness actually has some benefit here.

Practically speaking, I've only seen two uses of CEVs:

  1. Configuring an option.
  2. Tree shaking

Do either of these use cases benefit from constness?

For the first use case, users might want a more flexible mechanism than CEVs, perhaps even something they can configure at runtime. In the second use case, I can imagine an alternative mechanism, such as compiler directives or flags, that practically speaking might cover all of the use cases.

As a result option 3 proposed above might be a better approach.

@yjbanov
Copy link

yjbanov commented Nov 29, 2022

If we only have two use-cases for constants, one for early evaluation (analysis, modular compilation), and one for late evaluation (linking, tree-shaking, optimization), then perhaps we could have const and late const?

  • const - evaluated early and modularly; does not depend on environment; can participate in const and late const expressions. The implementation detail here is that evaluation takes place in the frontend.
  • late const - evaluated late; can participate in late const expressions only. The implementation detail here is that evaluation takes place in the backend.

The keywords already exist, and given the existence of late final, could be easily understood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-sdk Use area-sdk for general purpose SDK issues (packaging, distribution, …).
Projects
None yet
Development

No branches or pull requests

4 participants