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

Class modifiers in multi-library packages #3113

Open
nex3 opened this issue May 26, 2023 · 9 comments
Open

Class modifiers in multi-library packages #3113

nex3 opened this issue May 26, 2023 · 9 comments
Labels
request Requests to resolve a particular developer problem

Comments

@nex3
Copy link
Member

nex3 commented May 26, 2023

Class modifiers are a very intriguing feature of Dart 3, but at present they're difficult to use in a project that otherwise follows general Dart best practices of separating concerns into different libraries that only expose the necessary APIs at library boundaries. If you want to get the benefits of exhaustiveness checking for a given class hierarchy, for example, you either have to include the entire hierarchy in one massive library, or use part of which obfuscates loads and references. Both of these solutions prevent effective cross-library encapsulation.

This is, admittedly, a difficult problem given that the Dart language doesn't have any notion of multi-file groups like "packages". That said, I'm hoping there's a middle ground that could be reached to allow these modifiers to be useful without pulling everything into the same file. As a straw example, you could treat libraries that are imported by a class's library as "in the same library" for class modifier purposes. This would ensure that you can still determine the full scope of a sealed class by resolving its library.

@nex3 nex3 added the request Requests to resolve a particular developer problem label May 26, 2023
@lrhn
Copy link
Member

lrhn commented May 27, 2023

Allowing libraries imported by a library to skirt its class modifier rules will also require that library to have imported the first library back (otherwise it can't subclass anyway, so it shouldn't care about modifiers).

So a more precise description might be to treat "strongly connected components" specially, libraries which all import each other cyclically, directly or indirectly (a.dart imports b.dart, b.dart imports c.dart, c.dart imports a.dart would work too).

That should ensure that the libraries are always part of the same compilation unit, so the compiler will see all of them at the same time, which is what we require for sealed.
I'd also require them to be in the same package (so I'd have to formally introduce the notion of belonging to a package, using the underlying functionality we use for assigning language version).

It's still a derived property of something which may be accidental. Just because two libraries depend on each other, it doesn't mean they're that kind of friend libraries. But if they're in the same package, at least you're not messing with anybody else's code, just your own.

It would still not allow tests to ignore class modifiers, because your package libraries will not depend on the test libraries.

(Personally, I'd just add imports to part files instead, and use those.)

@nex3
Copy link
Member Author

nex3 commented May 30, 2023

If the language-version machinery already requires a notion of "package", then maybe it would be clearer to use that as the boundary here.

@lrhn
Copy link
Member

lrhn commented May 30, 2023

Allowing every file in "the same package" to be able to contribute to a sealed class isn't viable. The compiler needs to know every direct subclass in order to decide whether a switch expression is exhaustive, and if that can depend on files that are not even part of the same program, it's just not possible. And every test in test/ is a separate program, even if it's in the same package.

A package is simply too big a unit to be useful for this.

@nex3
Copy link
Member Author

nex3 commented May 30, 2023

Sure, but couldn't you solve that issue by using the intersection of the package and the program (that is, the files within the package that are actually loaded)?

@jakemac53
Copy link
Contributor

Sure, but couldn't you solve that issue by using the intersection of the package and the program (that is, the files within the package that are actually loaded)?

You generally don't want the compilation/analysis of a library to be dependent on how it is used. That makes invalidation poor (it depends on not only deps but reverse deps). For whole world compiles it could be ok but we have to consider modular compiles as well, where we don't compile whole packages but individual libraries from the package, without visibility into the rest of the package.

@lrhn
Copy link
Member

lrhn commented May 30, 2023

As @jakemac53 says, it makes it very hard to say something definitive about a program if its behavior depends on which other files are part of the program.

Whether a sealed class hierarchy has a class more or less determines whether a switch is exhaustive. Whether the switch is exhaustive may determine whether a variable is promoted by the switch. That means that the type of a local variable can depend on whether another file is part of the same program or not. That makes it very hard to analyze the file by itself.

Example:

sealed class A {}
final class B extends A {}
void foo(A a) {
  int? x = null;
  switch (a) {
    case B _: x = 42;
  }
  print(x.toRadixString(16)); // Valid or not?
}

Whether this code is correct or not depends on whether A has any more subtypes anywhere in the program. Which cannot be answered without knowing the entire program, and it precludes analyzing the file by itself without knowing the program entry point, and all files included in the program.

So, not viable.

@nex3
Copy link
Member Author

nex3 commented May 30, 2023

Ah, yeah, given modular compilation that restriction does make sense.

@rrousselGit
Copy link

A way to make a class sealed while implemented across multiple files would be nice.

Using "part" may require significant refactoring of existing code to do that.

For example, I'm thinking about the analyzer API, which would benefit quite a lot from making classes sealed.
Yet the public API file is nothing but interfaces, and the implementations are somewhere else.

@munificent
Copy link
Member

One of the things we've talked about is "friend libraries" where library A could explicit say that library B is a "friend", which would then allow B to ignore the class modifier restrictions.

Or we could consider an approach like Java's permits clause where the sealed class explicitly reaches out to mention the allowed subtypes, which may be declared in other libraries.

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