Dart 3.0 class modifiers

[dcharkes]

When code opts into Dart 3.0, we need to add class modifiers.

All subtypes of Struct, Union, Opaque, and AbiSpecificInteger need to become final class instead of class.

If the code is being generated for pre-3.0, we need to generate class, because final class is not accepted by the parser.

This means we have two options:

1. Tell people to use an older version of FFIgen if they want to target an older version of the SDK. (We often bump the lower SDK constraint when targeting new dart:ffi features anyway.)
2. Add an optional language-version config var, which determines what we output.

I'm not sure of option 2 is worth it though. For example VarArgs only exists in 3.0 and we will add that as well when Dart 3.0 is out.

[dcharkes]

I'll go for option 1.

We could bump FFIgen to 8.0.0 so that we can still release legacy versions on 7.x. However, that would mean that users need to opt in by explicitly upgrading FFIgen.
Using 7.3.0 instead would mean it auto-upgrades for users. For users on old SDKs the pub version solver would keep solving to 7.2.x because of the 3.0 sdk constraint in 7.3.0.

@leafpetersen is bumping the SDK constraint to 3.0 and adding class modifiers considered a breaking change? Should it we be major version bumping FFIgen?

[leafpetersen]

Bumping the SDK constraint should be fine - the new version will only get picked up by people on 3.0 sdks. Note though that code generators have some additional complexity:

• If you are generating code that will be interpreted as being part of a different package, you need to ensure that the generated code explicitly sets its language version, since the package you are generating into still may be on 2.X (and hence by default your generated code would be interpreted in version 2.X).
• If you are generating code into other libraries as part files, you need to match the language version of the library into which you are generating

As for whether adding final to the generated classes is breaking, it likely is technically breaking in that someone could be extending/implementing the generated classes. If you feel that someone doing so is clearly violating the intent of your API (and especially if that intent is documented somewhere) you could make the argument that this should not be treated as a breaking change. As always, I would focus on the actual practical impact on users here.

cc @natebosch @devoncarew @jakemac53 who might have additional advice and opinions about how we want to manage the roll out of class modifiers in dart owned packages and code generators.

[natebosch]

I think it is sensible to only support generating the new language version, and asking folks who need backwards compatibility to stay on an older version of ffigen. Publishing this as a major version bump is a good idea - folks opt in to picking up the new codegen.

I think projects should see analysis errors if we are generating code with final and they haven't upgraded their language version, but we could consider checking the language version during generation and giving a concrete error and instructions when they aren't on a new language version yet.

If the code wouldn't have supported subclasses in a meaningful way before and we can reasonably assume there are no (or very very few) examples in the wild of extends, then it's probably OK to publish with the newly generated code without a major version bump - but I think this should be covered by our other guidance.

[dcharkes]

• If you are generating code that will be interpreted as being part of a different package, you need to ensure that the generated code explicitly sets its language version, since the package you are generating into still may be on 2.X (and hence by default your generated code would be interpreted in version 2.X).

Packages have a dev dependency on FFIgen.

• With FFIgen 7.3.0 requiring Dart 3.0: The behavior will then be that once users have a 3.0 SDK, regenerating the code will add the keywords, which will then lead to error messages because they haven't bumped their SDK constraint.
  They then have two options: either pin their FFIgen to 7.2.x and regenerate without keywords, or bump their projects' SDK constraint to 3.0.
• With FFIgen 8.0.0 requiring Dart 3.0: The behavoir will then be that once users have a 3.0 SDK, nothing happens. Regenerating code will not add keywords because users will stay on 7.x. Once users bump their SDK constraint they will have errors in the FFIgenerated code. At which point they will likely go to the FFIgen repo and realize there's a 8.0.0. -> Maybe this is slightly better, the users will run into this when upgrading their project to 3.0 rather than when upgrading their SDK to 3.0.

Publishing this as a major version bump is a good idea - folks opt in to picking up the new codegen.

I'm also leaning in to this considering the above user flows.

As for whether adding final to the generated classes is breaking, it likely is technically breaking in that someone could be extending/implementing the generated classes.

It's not a breaking change, we already have custom error messages pre 3.0 that prevent Struct subtypes from being extended/implemented/mixed-in. With the new base modifier on Struct the SDK mandates struct subtypes from not being mixed in or implemented. And generating final on the Struct subtypes 'documents'/'duplicates' the already existing manual check in the FFI transform (CFE) / FFI verifier (analyzer).

If the code wouldn't have supported subclasses in a meaningful way before and we can reasonably assume there are no (or very very few) examples in the wild of extends, then it's probably OK to publish with the newly generated code without a major version bump - but I think this should be covered by our other guidance.

The packages using FFIgen, if they export the generated code in their public API, can indeed publish their package without a major version bump. Adding final to Struct subtypes is non-breaking as we were already disallowing subtypes of struct subtypes with custom FFI checks.

we could consider checking the language version

We have considered this, but not done it (yet).

• Check generated features against SDK constraint of user #361

Thanks for helping me think through this! 🚀

[oprypin]

• 3.0 Class modifiers dart-archive/ffigen#553

This current taken approach of an instant flip is untenable at least for google3. It means that we will have to flip the language version for everyone and import third_party ffigen atomically. Among who knows how many other things that will want to be atomic.

This also means that any updates of third_party ffigen in google3 are blocked until the flip.

Instead, generating different code based on the target language version would be very very nice.

• Check generated features against SDK constraint of user #361

built_value already went for this approach and it's great

• Prepare codegen for Dart 3. google/built_value.dart#1225

[oprypin]

Actually a very easy option would be to undo this change and instead just add // @dart=2.13 into the generated files.

This works (and I confirmed that it does) because ffi is part of the SDK and SDK pretends that class modifiers aren't there if used from files pinned to an older language version.

[dcharkes]

This current taken approach of an instant flip is untenable at least for google3. It means that we will have to flip the language version for everyone and import third_party ffigen atomically. Among who knows how many other things that will want to be atomic.

I believe we do not run any ffigen as part of build steps yet. So currently it's a manual run by users to run ffigen. Or is FFIgen already integrated in build steps? @TzviPM

This also means that any updates of third_party ffigen in google3 are blocked until the flip.

That only holds true if FFIgen is run automatically.

Instead, generating different code based on the target language version would be very very nice.

FFIgen currently does not try to parse the the parent project package_config/pubspec, but conceivably we could do so. (With a fallback that if you run it in a vaccuum and there is no pubspec it just assumes newest version of Dart.)

Actually a very easy option would be to undo this change and instead just add // @Dart=2.13 into the generated files.

2.16 likely, as that's the lower bound currently on the last stable release.

However, wouldn't this prevent users from having their whole project in 3.0 mode? I guess // @dart=2.16 or // @dart=2.19 isn't as bad as having // @dart=2.11 because it doesn't prevent running in sound null safe mode.

[dcharkes]

Also, looking for a pubspec to see if we're in 3.0 or not will likely not work in g3 where language versions are defined in a different way?

• Prepare codegen for Dart 3. google/built_value.dart#1225

This PR doesn't show how the language version of the surrounding project is obtained.

Any suggestions on how to do that reliably?

[oprypin]

I believe we do not run any ffigen as part of build steps yet. So currently it's a manual run by users to run ffigen.

In g3 I see both kinds of usages - checked in and auto generated. So there is at least 1 place where this is integrated into the build, haven't checked how many places there are that use this.

Also, looking for a pubspec to see if we're in 3.0 or not will likely not work in g3 where language versions are defined in a different way?

Yes, in g3 we are not looking at pubspec. But somehow built_value figures out the language version anyway. I don't know how it is done - neither in g3, nor normally.

---

Anyway I realized that for g3 we will have a way forward: I'll have to proceed with adding // @dart=2.19 (as a temporary patch) to the generator, whether or not this path will be taken in this upstream repository.

[oprypin]

This PR doesn't show how the language version of the surrounding project is obtained.

Ah but I think I have a pointer towards the answer. built_value is always generated based on another source file. So I'm pretty sure it looks at the resolved language version of the invoking Dart file / library. Not of the overall project.

[dcharkes]

Anyway I realized that for g3 we will have a way forward: I'll have to proceed with adding // @dart=2.19 (as a temporary patch) to the generator, whether or not this path will be taken in this upstream repository.

That still means

This also means that any updates of third_party ffigen in google3 are blocked until the flip.

Which will most likely block @TzviPM's work.

An alternative to detecting the language version from a pubspec is to declare the target language version in ffigen.yaml, but that requires users to configure it.

[TzviPM]

let's not guide the entire ecosystem based on my work in google3. That said, I imagine a configurable version would be good to maintain at least for a short period of time for others as well. I'm fine with making dart 3 the default, as long as we can opt out for some period of time. Opting out in the config is no issue for us, because we auto-generate that config in google3. I'll gather some additional context from @oprypin and update this issue. We don't currently support using ffigen automatically, but I am hoping to support that use case by EOM, so this might block my work depending on the timelines of dart 3, etc.

[jakemac53]

Does ffigen import any user code in its generated files, or is it all fixed dependencies? If the latter, we could also consider adding // @dart=3.0 unconditionally.

[oprypin]

Apologies, I must have made a mistake in my observations.

ffigen is not used at build time anywhere in google3 (yet?), so indeed it's only checked-in files, which of course cannot affect a roll of third_party ffigen.

So maybe we don't have a problem after all.

These are many possible ways of going forward:

• Keep final class generation as is (with the recent PR)
• Add an opt-out flag to not generate final
• Add // @dart=2.16 and revert the addition of final
• Add // @dart=3.0 and keep final

But in google3 I think we're not blocked either way.

[dcharkes]

Add // @dart=3.0 and keep final

This is done as a patch in google3 now, so I believe we can close this.