Skip to content

ifdefs or equivalent #35718

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

Closed
Hixie opened this issue Jan 20, 2019 · 7 comments
Closed

ifdefs or equivalent #35718

Hixie opened this issue Jan 20, 2019 · 7 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). closed-stale Closed as the issue or PR is assumed stale type-enhancement A request for a change that isn't a bug

Comments

@Hixie
Copy link
Contributor

Hixie commented Jan 20, 2019

I'd like to be able to check, at compile time, whether an environment variable is set, and if so, I'd like to declare a particular identifier as a constant whose value is derived from that environment variable, and if not, I'd like it to be a mutable variable.

Specifically, I'd like defaultTargetPlatform to be const in release builds so that we can tree-shake code that depends on it (and not include iOS logic in Android apps), while in debug builds it remains a variable whose value we can dynamically change via the service protocol extension we have today.

@lrhn lrhn added area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). type-enhancement A request for a change that isn't a bug labels Jan 21, 2019
@Hixie
Copy link
Contributor Author

Hixie commented May 30, 2019

Another use case that's becoming more and more of an issue is developing packages and plugins that need to target multiple versions of Flutter or Dart. For example, we have a policy that our plugins must be compatible with the latest stable release, but we develop against master. This means that when we make a breaking change on master, we have a lot of difficulty. It would be much easier if we could ifdef-out some code for when we're targeting stable and #ifdef-out the equivalent code for when we're targeting master. Or, rather, in practice, mark code blocks as targeting one version of flutter vs another version of flutter. Maybe this means conditions should allow for a syntax similar to pub version constraints, applied against packages that the code is being compiled with.

cc @leafpetersen @munificent @mit-mit

@Hixie
Copy link
Contributor Author

Hixie commented Sep 5, 2019

Here's a straw-man proposal that does not solve the two problems described above but might be a starting point for ideas that do solve those problems:

We allow tags to be declared. Tags are defined in terms of enums, and must be given a type and a name, and optionally a default value. For example:

enum BuildMode { debug, profile, release }
enum BuildPlatform { android, iOS, fuchsia, web, windows, macOS, linux }
enum BuildSize { demo, full }
tag BuildMode buildMode;
tag BuildPlatform target;
tag BuildSize game = BuildSize.full; // with default

The compiler can be given a value for each declared tag. It is an error for a tag to be declared without a default value if the compiler was not explicitly given a value for that tag on the command line. It is an error for a tag to be given to the compiler on the command line if no code declares that tag.

Declarations and blocks can be annotated to specify that they only apply when a tag has a particular value or when it does not have a particular value.

class Foo {
  @if(buildMode == BuildMode.debug)
  final Bar baz;
  @if(target != BuildPlatform.web)
  final Bob quux;

  void hello() {
    print('Welcome to the game!');
    @if(game == BuildSize.demo) {
      print('(This is a demo.)');
    }
  }
}

Every declaration and code block implicitly has a list of values that it applies for. By default, code applies to every value of every tag. For example, the class Foo above applies to all three BuildMode values, all seven BuildPlatform values, and both BuildSize values.

Each @if annotation excludes some values from the declaration or code block that it labels. For example, the "baz" field above only applies to the "debug" BuildMode, not the "profile" and "release" build modes. Annotations are of the form "tag != TagEnum.value" or "tag == TagEnum.value", either excluding the specified value or all but the specified value, respectively.

The compiler excludes code that doesn't apply given the values it was given for each tag when compiling.

The analyzer verifies that code only refers to members tagged with a subset of the tags that apply to the code. For example:

enum Foo { a, b }
tag Foo tag1;
tag Foo tag2;

void main() {
  @if(tag1 == Foo.a)
  bool q = true;
  @if(tag1 == Foo.a) {
    q = false;
  }
  @if(tag2 == Foo.a) {
    q = false; // error 1
  }
  print(q); // error 2
}

In this code, the errors happen because those blocks are tagged with tag1 values a and b, whereas q is only tagged with tag1 value a.

A more elaborate example with nested scopes of tagging;

enum Foo { a, b }
tag Foo tag1;
tag Foo tag2;

@if(tag1 == Foo.a)
class Bar {
  @if(tag2 == Foo.b)
  int q = 0;

  @if(tag1 == Foo.b)
  int r = 1; // not an error, but could be a lint (1)

  @if(tag2 == Foo.a)
  void test() {
    print(q); // error (2)
  }
}

void main() {
  print(Bar().q); // error (3)
  @if(tag1 == Foo.a) {
    print(Bar().q); // error (4)
    @if(tag2 != Foo.a) {
      print(Bar().q); // (5)
    }
  }
}

In this example, Bar.r can never be referenced, because it doesn't apply to any value of tag1. This could be a lint (it probably should not be an outright error, because it's harmless enough).

The error marked 2 is invalid because that block applies to value "a" of tag2, whereas "q" is only declared for value "b".

The error marked 3 is invalid because that block applies to all values of tag1 and tag2, whereas Bar is only available for one value of tag1, "a". The error marked 4 is similar; while the call to Bar() is now valid because of the outer @if, the use of Bar.q is invalid since in that block, all values of tag2 apply. The statement in the innermost block, labeled 5, is fine because it only applies for tag1 "a" and tag2 "b".

Adding a new enum value to Foo would make the line marked 5 into an error, because the innermost @if would not exclude this new value (it uses !=) whereas Bar.q is declared using an @if that only allows one value (it uses ==).

For the purposes of control-flow analysis, each @if-annotated code block can be treated like an if statement expressed in terms of a global variable. One possible additional improvement here would be to notice a chain of @if-annotated blocks that cover the full spectrum of values for a particular tag, and treat that as an if-else chain.

Again, this is not a concrete proposal as it does not solve the problems listed for this issue. It's only meant as a starting point for discussion.

@munificent
Copy link
Member

We've discussed features similar to this before. I'll quickly walk through the two main problems we ran into that stalled progress on the design. That's not to say that those are necessarily fatal or the only problems, just to give you some context that we have explored this space a little (which implies that your idea has merit) but also that our lack of progress may be because the problem is harder than it first appears.

What is the static analysis user experience?

Allowing "control flow" at the declaration level means that the static shape of a program is no longer, well, static. In some configurations you may get static shape A and in others B. But the user's IDE and static analysis experience is currently "modeless" or configuration independent. When I'm looking at a Dart source file in my editor, I'm not looking at it "in release mode". That's a choice that happens farther downstream. But if the set of static errors that are reported depends on choices like release mode versus debug mode, then the idea of a modeless IDE experience is no longer meaningful.

Concretely, say I had:

enum BuildMode { debug, profile, release }
tag BuildMode buildMode = BuildMode.debug;

class Foo {
  @if(buildMode == BuildMode.debug)
  String field = "value";

  @if(buildMode == BuildMode.release)
  int field = 123;
}

main() {
  String s = Foo().field; // <--
}

Should analyzer report an error on the marked line?

One approach would be to actually make the IDE modal. Have some combo boxes or something where the user picks a specific configuration that they want to view the program as. The two main problems with that are:

  1. For package authors, it means they need to remember to check out their package in all configurations. Otherwise, they risk shipping a package that has compile errors in an overlooked configuration. We basically require users to brute force check all of the combinations.

  2. The set of configurable options (tags in your proposal) is potentially large and confusing. In theory, the IDE would need to give you a dropdown for every single tag in every single package your app transitively depends on. That could scale poorly and be confusing when you see flags that aren't meaningful to you.

Granted, this is sort of what C++ IDEs do, so it's not intractable, but also, uh, it ends up being kind of a crappy user experience. I have repeatedly committed bugs in C programs without realizing because I didn't realize XCode was only showing me the errors for a certain configuration.

The set of configurations is potentially self-referential

One desirable use for ifdefs is applying them to imports. That way, for example, you could only import dart:io on a platform where it's available. Or only import dart:developer in debug builds.

But imported libraries are also the things that define these tags. So you can get into a weird situation where:

  1. In order to process an @if, you need to know the values of all tags.
  2. Which means you need to traverse all the imports to find the tags.
  3. But you don't know "all the imports" until you start evaluating tags.

I believe this means you've have to do some sort of incremental, ordered traversal of imports. That might rule out circular imports (which are very common in Dart today) or mean that the order of import directives in a file has semantic meaning (which means that sorting your imports could break your app). That would be a really big conceptual change in how the library system behaves and not necessarily for the better.

The analyzer verifies that code only refers to members tagged with a subset of the tags that apply to the code.

This is a good attempt to untangle some of the difficulty statically analyzing code that does "static control flow". I could be wrong, but my hunch is that this kind of analysis may actually be intractable/undecidable/Turing-complete/what have you.

If we allow arbitrary Boolean expressions in @if conditions, then I think that means you could express any 3-SAT problem as a set of tags and @if blocks. That implies that this analysis is NP-complete.

I may be wrong about this. Even so, I suspect there are still other cases where that analysis gets really fishy once you take into account inheritance, overrides, const, default values, etc. My spidey sense tingles.

For example, in your initial request, you want to have a variable that is const in some configurations but not in others. Can that variable be used as a default value? Can the analyzer reliably ensure that another member that uses that variable as a default value can't be reached unless the proper tags are set? If that member is in a class that is never used when the variable is non-const, is that sufficient? Does the class itself need to be guarded by @if? If so, does that mean every single place that uses that class also needs the guard? Is there any way to encapsulate a tag such that an @if doesn't bleed out like this?

@Hixie
Copy link
Contributor Author

Hixie commented Sep 12, 2019

The strawman above doesn't solve the original problem described at the top of this issue, but FWIW, it doesn't have the problems you list:

  • in your code snippet, there would be an error because field is declared twice, which isn't valid.

  • for the imports, you can find all the tags in one pass at the same time as parsing, and then do the analysis thereafter; the tags can't impact the parsing, can't impact the imports, and can't impact other tags.

  • the @if conditions are explicitly not arbitrary, they are only of the form tag == value or tag != value where value is only allowed to be a literal enum value of the right type.

For example, in your initial request, you want to have a variable that is const in some configurations but not in others

Yeah, I don't know how to solve that. The strawman proposal above doesn't attempt to solve it. I'm still thinking about it. :-)

@Hixie
Copy link
Contributor Author

Hixie commented Sep 12, 2019

For the const thing, maybe the solution is to write the code as follows:

Foo get bar => _bar;
Foo _bar = /* a compile-time evaluatable expression */;
@if(buildMode == BuildMode.debug)
set bar(Foo value) {
  _bar = value;
}

The compiler can then be made clever enough to notice that _bar never changes value in release builds, and can therefore be treated like a constant, and that can propagate to the getter, which can then be inlined everywhere, and so on, which gives us the original intent, which is to tree-shake away code that can't be reached given the immutable value of this field in that mode.

This avoids the problems listed above about actually having the field be actually both const and non-const. It's unambiguously non-const, it's just that the compiler can figure out that it might as well be and it can thus get all the benefits of being const.

@Hixie
Copy link
Contributor Author

Hixie commented Sep 12, 2019

For the "targeting two different versions of a library" thing, this doesn't seem to really help at all.

@yjbanov
Copy link

yjbanov commented Sep 12, 2019

I proposed something similar over in the language repo:

@lrhn lrhn added the closed-stale Closed as the issue or PR is assumed stale label Apr 9, 2025
@lrhn lrhn closed this as not planned Won't fix, can't repro, duplicate, stale Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). closed-stale Closed as the issue or PR is assumed stale type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants