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

Equals initializer syntax #1580

Merged
merged 60 commits into from
Mar 27, 2023
Merged

Equals initializer syntax #1580

merged 60 commits into from
Mar 27, 2023

Conversation

oowekyala
Copy link
Collaborator

@oowekyala oowekyala commented Feb 2, 2023

  • Introduce the new = initializer syntax to replace the parens syntax in all targets except C++. This works exactly like parentheses do, except in C++ it calls the assignment operator.
  • Using the parens syntax emits a warning now
  • The formatter converts to the new syntax automatically for ease of migration (for C++ it doesn't touch anything).

Updated 2023-03-02 This does in fact add a syntax for C array initializers (which can be used also for struct initializers). Summary:

  • Using a: int(0) or a: int(0,0) produces a warning (except in C++). they can be replaced with a: int = 0 resp. a: int = {0, 0} (the latter only in C/C++).
    • the braced list syntax is not available in Python or TS, so if you were declaring a python list/tuple with a(1,2), you can convert it to a = {= [1, 2] =} to remove the warning, or just wait until we allow writing a = [1, 2] directly.
  • In all targets, reactor instantiations have to use the equal syntax: write new R(a = 1) instead of new R(a (1)).
    • In C++, if you want to call the constructor with parameters, then you have to write out the type of the constructed object explicitly: new R(a = {= std::vector<int>(1,2) =} or so (whereas you don't need to repeat the type when you write a default parameter value or a state var initializer). This is so because we would get problems with generic type parameters otherwise (and C++ itself has the same behavior).
    • In C, if you were writing new R(a(1,2)) or new R(a = (1,2)) to mean an array, then use new R(a = {1, 2})
    • In Python and TS, the same caveat applies, that you can't write a python/TS bracket-delimited list without target escaping yet.

Remaining action items:

Fixes #1650 and #1651

@lhstrh
Copy link
Member

lhstrh commented Feb 24, 2023

Only 654 files changed? 🤣

@lhstrh
Copy link
Member

lhstrh commented Mar 2, 2023

This looks good to me, though it would be great to get some more 👀 on this. My main concern is that this does not only affect our tests, but also our examples. We should probably set up CI in https://github.com/lf-lang/examples-lingua-franca first, and prepare a PR that makes all the required adjustments...

@lhstrh
Copy link
Member

lhstrh commented Mar 2, 2023

Just to clarify, does using braces now trigger a warning or an error?

@edwardalee
Copy link
Collaborator

This also requires an update of all the examples and text on the website.

@oowekyala
Copy link
Collaborator Author

Just to clarify, does using braces now trigger a warning or an error?

Using parentheses triggers a warning (except in C++), braces is still an error in all targets except C++. Also in all targets including C++, writing new R(a(1)) or a {1} instead of a = 1 or eg a = {= std::vector<int>(1) =} is now a warning.

This also requires an update of all the examples and text on the website.

I've been working on this and will submit a PR today.

@cmnrd cmnrd linked an issue Mar 24, 2023 that may be closed by this pull request
@cmnrd cmnrd linked an issue Mar 24, 2023 that may be closed by this pull request
@cmnrd
Copy link
Collaborator

cmnrd commented Mar 24, 2023

Looks like all the tests pass now. This should be ready to be merged. Should we go ahead or does anyone else want to review the code?

@petervdonovan
Copy link
Collaborator

I do not object to merging this right away even though I have not reviewed the code. If other people think that more review is important and can wait for a few days for me to do it, I can, but I also do not mind trusting you and Clement so that this big PR does not linger any longer.

@petervdonovan
Copy link
Collaborator

The one failing test, NativeListsAndTimes, relies on a C23 feature, so it shouldn't be surprising that it does not pass on Windows. (The failing CLI tests are just a network issue.)

In C, I am not aware of any use cases for zero-length arrays except in the case when you are code-generating C, using macros, etc., and the array of length zero is the continuation of some pattern. I am not sure I can envision such situations cropping up in LF programs. For sure it is awkward and weird that C does not permit initializers of length zero before 2023, but maybe that is C's fault, not ours. We have the ability to patch it up in LF, but I am not sure that that is our responsibility.

Copy link
Collaborator

@petervdonovan petervdonovan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I have no important comments. Looking forward to seeing this get merged soon!

cmnrd and others added 3 commits March 27, 2023 11:14
Co-authored-by: Peter Donovan <33707478+petervdonovan@users.noreply.github.com>
Co-authored-by: Peter Donovan <33707478+petervdonovan@users.noreply.github.com>
@cmnrd cmnrd merged commit 6547cea into master Mar 27, 2023
@cmnrd cmnrd deleted the clem.new-equals-initializers branch March 27, 2023 17:55
@cmnrd cmnrd added enhancement Enhancement of existing feature feature New feature and removed enhancement Enhancement of existing feature feature New feature labels Mar 27, 2023
@petervdonovan petervdonovan changed the title Add equals initializer syntax Equals initializer syntax Aug 25, 2023
@petervdonovan petervdonovan added feature New feature and removed enhancement Enhancement of existing feature labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Valid time value rejected as invalid CLI gives null-pointer exception
5 participants