[pigeon] Improve C++ field handling#2097
Conversation
| /// | ||
| /// See [_writeDataClassImplementation] for the corresponding declaration. | ||
| /// This is intended to be added to the header. | ||
| void _writeDataClassDeclaration(Indent indent, Class klass, Root root, |
There was a problem hiding this comment.
I moved these without changes as the very first commit, so excluding the first commit from the review will make it easier to see the substantive changes.
| } | ||
|
|
||
| // TODO(stuartmorgan): Audit all uses of this and convert them to context-based | ||
| // methods like those above. Code still using this method may well have bugs. |
There was a problem hiding this comment.
I suspect that there are at least some bugs similar to the ones I fixed for field types in other places, but I wanted to limit the scope of this PR. We should get more of the native tests online in follow-ups to flush out other bugs.
There was a problem hiding this comment.
Yes. This PR removes fields from being unique_ptrs, so the use of this method should be greatly reduced here.
| } | ||
|
|
||
| // TODO(stuartmorgan): Audit all uses of this and convert them to context-based | ||
| // methods like those above. Code still using this method may well have bugs. |
There was a problem hiding this comment.
Yes. This PR removes fields from being unique_ptrs, so the use of this method should be greatly reduced here.
|
@stuartmorgan everything is working fine on my end. |
I assume you mean the non-null POD case? For the declaration it doesn't do anything (https://abseil.io/tips/109), and for the definition it seemed like pointless boilerplate in the generated code (not to mention extra work in the generator) given that it's a one-line implementation, so there's not really any room to accidentally modify the parameter. |
I was mostly thinking about compiler optimizations due to it being const, but I don't think it would change much, if at all. |
This implements non-nullable field support in the C++ generator, and fixes some cases where the C++ generation for nullable fields was assuming non-nullable values.
As part of this:
isNullabletoHostDatatypeto make it easier to track nullability across translation from Dart to host datatypes.friendfor a special namespace to facilitate these tests.Fixes flutter/flutter#104278
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).