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

Dart: enable custom fields with defaults that do not provide const constructor to work with 'PositionalDefaults' #1632

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

pwrobeldev
Copy link
Contributor

@pwrobeldev pwrobeldev commented Nov 29, 2024

------ Motivation ------

When fields with default values were defined in a struct
and their types did not provide 'const' constructors, when
such fields were used in combination with 'PositionalDefaults',
then the generated code did not compile.

------ Designed solution ------

In order to be able to generate the code, which compiles
when 'PositionalDefaults' annotation is used together
with default values of types that do not provide const
constructors, the optional values have been used.

When Gluecodium encounters a type that does not
provide const constructor it makes it optional in
positional defaults constructor and uses null as
default.

Later, on initializer list it checks if user provided
value different than null and if not, then initializes
the field with the actual default value. This way
the limitation related to const constructors is surpassed.

However, if the user wants to have nullable type that
does not provide const constructor and uses default
value different than null, then the behavior may be surprising.
Therefore, the validator was implemented for Dart to prevent
users from such usage.

------ Contents of change ------

  1. New smoke and functional tests, which confirm invalid behavior.
  2. Implementation of designed fix.
  3. New functional tests, which confirm that the fix works as expected.
  4. Validator of default values to prevent unexpected behavior + unit tests.

@pwrobeldev pwrobeldev requested a review from Hsilgos November 29, 2024 13:27
@pwrobeldev pwrobeldev changed the title Dart: make custom structures that do not provide const constructor to work with 'PositionalDefaults' Dart: enable custom fields with defaults that do not provide const constructor to work with 'PositionalDefaults' Dec 2, 2024
@pwrobeldev pwrobeldev force-pushed the pwrobeldev/mutable-custom-structs-defaults-for-positional-defaults branch from 92fe3d3 to a7a2528 Compare December 2, 2024 09:53
When the structure uses 'PositionalDefaults' annotation
and fields that have default values, but do not provide
const constructor (e.g. custom structures defined by
the used, which are not 'Immutable'), then the generated
code does not compile.

This change showcases the invalid behavior. It adds smoke
tests, which clearly show that 'const' keyword is added
before constructor calls.

Moreover, this commit introduces new LIME input for functional
tests, which causes compilation errors.

Signed-off-by: Patryk Wrobel <[email protected]>
This change introduces the fix for the bug showcased
in the previous commit.

In order to be able to generate code, which compiles
when 'PositionalDefaults' annotation is used together
with default values of structure types that do not
provide const constructors, the optional values
have been used.

When Gluecodium encounters such type that does not
provide const constructor it makes it optional in
positional defaults constructor and uses null as
default.

Later, on initializer list it checks if user provided
value different than null and if not, then initializes
the field with the actual default value.

This way the limitation related to const constructors
is surpassed.

However, if the user wants to have nullable type with
default different than null, then the behavior may be
surprising.

Therefore, in next patches the required validation will
be implemented for DartGenerator.

Signed-off-by: Patryk Wrobel <[email protected]>
This commit introduces a new functional test, which
verifies that the newly implemented logic in generator
works as expected and non-const constructible struct
default values are correct.

Signed-off-by: Patryk Wrobel <[email protected]>
…aults

This change introduces DartDefaultValuesValidator class
which checks if the default value used for nullable
custom structs, which do not provide const constructors
are equal to 'null'.

Signed-off-by: Patryk Wrobel <[email protected]>
This change extends the documentation of 'PositionalDefaults'
annotation for Dart to describe the specific behavior in the
case of nullable types.

Signed-off-by: Patryk Wrobel <[email protected]>
The constructors of 'Uint8List' type are not const.
This type is used when BLOB is specified in LIME
file.

Therefore, the usage of BLOB type for a field of
struct that is marked as 'PositionalDefaults' and
specifying a default value of this field leads to
compilation error.

This change is used to document the invalid behavior.

Signed-off-by: Patryk Wrobel <[email protected]>
This change fixes the compilation error via
using the same approach that was used for
non-const constructible structures.

This commit extends functional tests to
confirm that BLOB can be used for a field
with default value for structure annotated
as 'PositionalDefaults'.

Signed-off-by: Patryk Wrobel <[email protected]>
Disallow users from using nullable field of BLOB
type that has default value different than null
to avoid unexpected behavior.

Signed-off-by: Patryk Wrobel <[email protected]>
This change includes the information about
usage of nullable Blob field with default
value in the structure annotated as
'PositionalDefaults'.

Signed-off-by: Patryk Wrobel <[email protected]>
Signed-off-by: Patryk Wrobel <[email protected]>
@pwrobeldev pwrobeldev force-pushed the pwrobeldev/mutable-custom-structs-defaults-for-positional-defaults branch from cf32397 to 3438558 Compare December 13, 2024 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant