dart-lang / language

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

Support for Rust like #[cfg()] attributes #3972

Open scheglov opened 1 month ago

scheglov commented 1 month ago

Maybe instead of adding configurable URIs for parts (in addition to imports and exports), we can add a more flexible feature?

As you can see below, it can be used not only for "part" (modules in Rust), and "imports"; but also can be used to conditionally compile statements. Which might be convenient - declare much platform specific logic in conditional parts, and then conditionally use it.

The main point here is that in Rust this is a universal feature - you can condition anything. And in Dart currently we have push this feature into every directive separately.

#[cfg(target_os = "windows")]
mod my_windows;

#[cfg(target_os = "macos")]
mod my_macos;

fn main() {
    #[cfg(target_os = "windows")]
    my_windows::say_hello_windows();

    #[cfg(target_os = "macos")]
    my_macos::say_hello_macos();
}

my_macos.rs

pub fn say_hello_macos() {
    println!("Hello, macOS!");
}

my_windows.rs

pub fn say_hello_windows() {
    println!("Hello, Windows!");
}
munificent commented 1 month ago

Related: Conditional attribute in C#.

lrhn commented 1 month ago

We can already do conditional compilation using if (const String.fromEnvironment("dart.config.os" == "windows")). We'd just have to have the compiler add that information to the compilation environment. (We should, https://github.com/dart-lang/sdk/issues/54785.)

We could allow "anything" to be conditional. I'm not sure it's going to make programs easier to read, or analyze using the analyzer or any other tool that doesn't actually have a designated configuration. We'd definitely need to ensure that all parts of a program are compiled in the same compilation configuration. If one class can conditionally have a member in its interface, and another can conditionally call that member, they better agree on which configuration they're compiled as. (I don't think it'll give any power that we won't have today with configurable imports and parts, and constant if conditions. You can do pretty much anything with the imports, the only reason to add conditional parts is to have conditional augmentations, of privately named members, which need to be inside the same library. But then, augmentations aren't really needed except by macros.)

Wdestroier commented 1 month ago

Could this allow a test function (or many) to be appear right after the target function?

trimmed(string) => string.trim();

@test
trimRemovesSurroundingWhitespace() {
  var string = '  foo '
  expect(trimmed(string), equals('foo'))
}

Would be awesome.

lrhn commented 1 month ago

I guess it could make visibleForTesting really mean "only exists when testing", if testing is a flag that the compiler can switch on.

jakemac53 commented 1 month ago

Could this allow a test function (or many) to be appear right after the target function?

Even if that code is omitted reliably in the final program it does still add a parsing cost to all downstream dependencies. This adds up to a significant cost on larger programs, so I would strongly advise against this.

It also adds a cost in terms of extra dependencies, unless possibly the imports for those are also trimmed in a similar way, but pub would have to know to trim them as well.

It is much better to just keep the tests separate.

munificent commented 1 month ago

We can already do conditional compilation using if (const String.fromEnvironment("dart.config.os" == "windows")).

That only works inside imperative code. There's no way to conditionally include or exclude an entire declaration, which is what the Rust thing and the C# Conditional attribute can do. C#'s attribute has even more power in that when a conditional method is excluded, all calls to it are silently discarded along with their argument list expressions. This is arguably too much magic, but it does make for a powerful, useful feature.

mcmah309 commented 1 month ago

Even if that code is omitted reliably in the final program it does still add a parsing cost to all downstream dependencies. This adds up to a significant cost on larger programs, so I would strongly advise against this.

It also adds a cost in terms of extra dependencies, unless possibly the imports for those are also trimmed in a similar way, but pub would have to know to trim them as well.

It is much better to just keep the tests separate.

It's worth noting that in larger dynamic projects like applications, especially on teams, constant refactoring is inevitable. Trying to maintain two separate identical trees (src and test) is a development bottleneck. Ideally the project structure is well defined and rarely changes but that is not always true in practice. Also, it is often annoying to create a separate file/structure just to write a test, developers being lazy, will just not write tests for this reason :shrug:. Finally, keeping the tests near the code can help explain what the code does faster. I'd happily take a few millisecond longer compilation for these benefits.

lrhn commented 1 month ago

Not sure it'll only be "a few milliseconds". If your test code is inside lib/, then it's available to other packages. That means its dependencies must also be available, so every dev-dependency becomes a normal dependency.

That can increase the number of packages to solve for, and makes it more likely to get a version conflict when solving.

lrhn commented 1 month ago

That only works inside imperative code. There's no way to conditionally include or exclude an entire declaration,

Three will be with conditional parts and augmentations.

It's not as convenient as Rust's feature, but with conditional imports and parts, and constant condition expressions inside body code, you should be able to have both conditional code and conditional declarations. (The only thing missing is conditional local declarations, and I think it should be possible to work around that.)

If we expect users to want to have large chunks of conditional code, we may want to make it more convenient to write. If it's just going to be a few more-level libraries, then I think the currently planned support will be enough.

mcmah309 commented 1 month ago

If your test code is inside lib/, then it's available to other packages. That means its dependencies must also be available, so every dev-dependency becomes a normal dependency.

That can increase the number of packages to solve for, and makes it more likely to get a version conflict when solving.

Yes given the current Dart dependency system this is not ideal. A possible solution would be introducing a module system where test modules can be declared inside lib. e.g. inside lib

trimmed(string) => string.trim();

test {
  import 'package:test/test.dart';

  @test
  trimRemovesSurroundingWhitespace() {
    var string = '  foo '
    expect(trimmed(string), equals('foo'))
  }
}

Thus the parsing cost would essentially become nothing since all code inside a test { ... } would be ignored.

We could generalize this approach to work with more modules e.g. module "test" { ... }. Thus anything in module "not_the_current_module" { ... } would be ignored. Though I don't believe using an "integration_test" inside lib would be wise. Also since Dart already supports conditional export statements that makes a lot of the general uses of module mute.