[pigeon] Improve C++ data class constructors#3585
[pigeon] Improve C++ data class constructors#3585auto-submit[bot] merged 9 commits intoflutter:mainfrom
Conversation
tarrinneal
left a comment
There was a problem hiding this comment.
lgtm. A couple very minor and optional nits.
|
|
||
| indent.format(''' | ||
| ${klass.name}::${klass.name}($paramString)$initializerString {} | ||
| '''); |
There was a problem hiding this comment.
Seems like this could be done with a non-format method that would be easier to maintain.
There was a problem hiding this comment.
That's what I thought at first too, but this was actually the best option I found. The main issue is that you can't have trailing commas in parameter lists or initializer lists in C++, which means that the options are either join (as I've done here, which then requires format), or an indexed for loop with a conditional on each line to add or not add a comma at the end, and that's just much messier.
And then it gets even uglier if you want to make the empty version:
Foo::Foo() {}
rather than
Foo::Foo(
) {}
because you have to duplicate the entire statement in a conditional, one printing the complete line with no params, and one printing only up to the (, then doing the for loop, then doing the ) [...] {}.
I don't love this version, but I liked the other way a lot less when I initially tried it.
(Longer term, I would like to make a helper method for the C++ generator for printing function declarations and definitions, and refactoring everything to use it, to reduce the need for this kind of thing inline.)
[pigeon] Improve C++ data class constructors
Instead of generating a default constructor for C++ data classes that will leave non-nullable POD fields uninitialized (and other non-nullable fields default-initialized, which doesn't match what we do for other languages), this creates either one or two initializers:
Other changes that followed from this:
Tthat became an error once the default constructor for data classes went away.Fixes flutter/flutter#104653
Fixes flutter/flutter#104286
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.///).