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

Switch to C# 7.3 #698

Closed
Barsonax opened this issue Oct 29, 2018 · 10 comments
Closed

Switch to C# 7.3 #698

Barsonax opened this issue Oct 29, 2018 · 10 comments
Labels
Core Area: Duality runtime or launcher Discussion Nothing to be done until decided otherwise
Milestone

Comments

@Barsonax
Copy link
Member

Barsonax commented Oct 29, 2018

Summary

Currently duality uses C# 4.0. This means we are missing the newer features that C#7.3 brings. Consider upgrading to a newer C# version.

Analysis

  • Will enable us to write cleaner more readable code. For instance C#6 has null propagation. This is a pretty long list of features so its better to actually read up on the newer C# versions than to post them here.
  • We would be able to do Investigate By-Ref Vector and Matrix Operators in C# 8.0 #600
  • Will make duality incompatbile with older visual studio versions. Only applies to ppl working on the source code itself.
  • Even if we don't upgrade .NETstandard will make the solution incompatible with older visual studio versions anyway.
@ilexp ilexp added this to the General milestone Oct 29, 2018
@ilexp ilexp added Core Area: Duality runtime or launcher Discussion Nothing to be done until decided otherwise labels Oct 29, 2018
@Barsonax
Copy link
Member Author

Barsonax commented Dec 9, 2018

C# 8.0 among other features will bring nullable references. This is a opt in feature as the new syntax is a breaking change. It is backwards compatible once compiled though. You do need Visual studio 2019 which is currently still in preview.

Even if nullable references are not directly beneficial to duality it is to users that use duality as they can see when returns/parameters etc can be null or not. This will help users write more robust code because null reference exceptions will now be pointed out to them at compile time (warning by default but can be turned to errors).

It might be worth looking into once C# 8.0 is stable.

@ilexp
Copy link
Member

ilexp commented Jun 10, 2019

Alright, let's get things started. Is there a feature-by-feature list of changes to C# by language version, as well as minimum required VS toolchain and its release date? If not, can we compile one as a basis for deciding which version to upgrade to?

@Barsonax
Copy link
Member Author

Barsonax commented Jun 10, 2019

From what I know:
VS2015 C#6

  • Static imports
  • Exception filters
  • Auto-property initializers
  • Expression bodied members
  • Null propagator (this is the ? operator for easy null checks)
  • String interpolation (alot more readable than string.format)
  • nameof operator (this is a very nice one to reduce the amount of magic strings)
  • Index initializers

VS2017 C#7

VS2019 C#8

  • Readonly members
  • Default interface members (needs .NET core 3.0)
  • Pattern matching enhancements:
    • Switch expressions
    • Property patterns
    • Tuple patterns
    • Positional patterns
  • Using declarations
  • Static local functions
  • Disposable ref structs
  • Nullable reference types (probably the killer feature of C# 8)
  • Asynchronous streams
  • Indices and ranges

Since you need C#8 for nullable references I say we go for C#8. VS2019 is out already and it's stable (and ALOT faster I have to say).

Appveyor has a VS2019 image so the CI part should work:
image

Jup this does mean that ppl with VS2015 or VS2017 that want to work on the source of duality are out of luck but I don't believe this is a big problem as you can simply install multiple VS versions on one machine nowadays.

EDIT: enriched the list with some info about the language features

@Barsonax
Copy link
Member Author

As a first step I say we just switch all projects to C# 8.0 and get appveyor working with it. After that we can start working on actually using the new features and changing the coding style where needed.

@ilexp
Copy link
Member

ilexp commented Jul 28, 2019

Not new info, but for the record, as I didn't see it mentioned here: We can't upgrade to C# 8.0 yet, as VS 2019 doesn't support building PCL libraries by default and the AppVeyor 2019 image doesn't have the .NET Portable Library Targeting Pack.

So we can either do the switch to .NET Standard first (#573), or just upgrade to C# 7.3. I've created a new discussion on the AppVeyor support in the mean time.

I'll pick this up to upgrade to C# 7.3 and .NET 4.7.2.

@ilexp ilexp self-assigned this Jul 28, 2019
@ilexp ilexp modified the milestones: General, C# / .NET Upgrade Jul 28, 2019
@ilexp
Copy link
Member

ilexp commented Jul 28, 2019

Progress

  • Updated all Duality projects using the .NET Framework to 4.7.2
  • Updated all Duality projects to use C# 7.3.
  • Triggered a binary release and did a quick test. Everything seems to be in order.

ToDo

  • Wait for the VS 2019 issue to be addressed or for the .NET Standard switch, then revisit this issue to consider upgrading to C# 8.0.

@ilexp ilexp removed their assignment Jul 28, 2019
@Barsonax
Copy link
Member Author

Updated all Duality projects to use C# 7.3.

#600 is now no longer blocked

@ilexp
Copy link
Member

ilexp commented Aug 2, 2019

So we can either do the switch to .NET Standard first (#573), or just upgrade to C# 7.3. I've created a new discussion on the AppVeyor support in the mean time.

Discussion was moved to GitHub for further updates. See here: appveyor/ci#3041

@ilexp
Copy link
Member

ilexp commented Aug 2, 2019

According to this thread, C# 8.0 will only be available on .NET Core 3.0 and .NET Standard 2.1, but not on .NET Framework. That means further work on this is blocked until we eventually switched to .NET Core, which probably happens separately, after the anticipated .NET Standard switch.

@ilexp
Copy link
Member

ilexp commented Aug 2, 2019

Because this will probably remain blocked for a bit and we've already done the upgrade to C# 7.3, I've made a new issue for C# 8.0 in #738. Closing this.

@ilexp ilexp closed this as completed Aug 2, 2019
@ilexp ilexp changed the title Consider upgrading the C# version Switch to C# 7.3 Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Area: Duality runtime or launcher Discussion Nothing to be done until decided otherwise
Development

No branches or pull requests

2 participants