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

Enhancements to the C# runtime #3614

Open
espenrl opened this issue Mar 31, 2022 · 6 comments
Open

Enhancements to the C# runtime #3614

espenrl opened this issue Mar 31, 2022 · 6 comments

Comments

@espenrl
Copy link

espenrl commented Mar 31, 2022

Hi, I love and have used ANTLR 4 for a long time and I want to contribute to the C# target.

I would like to contribute to

  • adding nullable annotations in runtime and produced parser code
  • performance enhancements (mainly improving memory allocation)

...if that is something you guys see valuable.

And that takes me to the current framework targets

<TargetFrameworks>net45;netstandard2.0</TargetFrameworks>

Doing the contributions is easier if one could bump up the target frameworks. Is that something that is up for discussion?

@kaby76
Copy link
Contributor

kaby76 commented Apr 1, 2022

What frameworks are you proposing? Can you give specific examples of what would change?

@KvanTTT
Copy link
Member

KvanTTT commented Apr 1, 2022

I consider nullable annotations and performance enhancements are valuable.

But I'm not sure it's the right time to drop net45 version, some users still use old frameworks, see #3212

@kaby76
Copy link
Contributor

kaby76 commented Apr 1, 2022

  • I think changing to nullable (e.g., int LA(int) => int? LA(int)) needs to be investigated. Apparently, one can generate net45 assemblies with ? nullable tagged value types using the net6.0 SDK. That SDK will generate CIL code that appears to convert the syntax to Nullable<...>. For unboxing, there will be generated call to get_Value(), which I really don't like--that's the whole point of a value vs reference type. I just don't understand why nullable value types are "so much better", to what, save on a few extra "if-thens"?
  • But, if the signature changes in a public method in the runtime, it should trigger a major version change because, presumably, there are people who carefully wrote parser and lexer base classes, listeners and visitors (it's an API change). But, here we find the Dart target was changed March '21 going from 4.9.2 to 4.9.3 Nov '21. Why didn't it trigger a major version number change all around? Where's the oversight?

@espenrl
Copy link
Author

espenrl commented Apr 1, 2022

Nullable annotations for reference types is basically a compiler trick. It doesn't change the API surface or have any runtime implications at all, but it adds some attribute metadata to the assembly. You need to use a compiler for C# 8 or later.

Nullable value types though is a different thing. That is another type behind the scenes Nullable<T> which will have a performance overhead. To be clear I'm not talking about using nullable value types.

Thanks for all the feedback. I'll look into how this all can be done and still target net45.

Maybe there can be a switch to turn on and off nullable annotations in the generated parser code. What do you think about that? As far as the runtime goes it's just some additional attribute metadata in the assembly that older compilers won't care about.

@espenrl
Copy link
Author

espenrl commented Apr 5, 2022

This dropped in my feed .NET Framework 4.5.2, 4.6, and 4.6.1 will reach End of Support on Apr 26, 2022.

@lingyv-li
Copy link
Member

But, here we find the Dart target was changed March '21 going from 4.9.2 to 4.9.3 Nov '21. Why didn't it trigger a major version number change all around? Where's the oversight?

It might be an oversight, but we didn't push for it either. As Dart was (still is) a very young runtime that's not used by many, asking everyone to re-generate code and do minor version upgrade is a bit unnecessary. Also the whole Dart community was going through the null-safety migration at the time, people all have the expectation when seeing the Null safety tag in pub.dev.

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

No branches or pull requests

4 participants