C#: add option and code generation for null reference types

[tonydnewell]

This replaces the closed PR #13218

It adds an option for generating C# code that handles null reference types.
See https://github.com/grpc/proposal/blob/master/L110-csharp-nullable-reference-types.md

Further work could be done to:

• add tests to check the generated code is as expected - there doesn't appear to be these kind of tests for other code generation
• change #nullable enable annotations to #nullable enable - but this requires further codegen changes
  • to annotate handling calling the Google.Protobuf.Reflection API that doesn't yet handle null reference types
  • to handle types such as string and bytes that use null to indicate presence

[tonydnewell]

The code generated by this PR may be good enough - however to be able to change #nullable enable annotations to #nullable enable the following needs to be addressed:

• Calling Google.Protobuf.Reflection.GeneratedClrTypeInfo()
  GeneratedClrTypeInfo() doesn’t support NRT, thus need to add null forgiving operator in some calls: e.g. new pbr::GeneratedClrTypeInfo[] { !null, }

• Checking for null in message fields uses the field name for the check but the property name to get the value
  e.g. in the generated GetHashCode() this idiom is used: if (someMessge_ != null) hash ^= SomeMessage.GetHashCode();
  Similar in MergeFrom().

• Optional message, string, and bytes fields need to be declared as nullable otherwise the constructor won't compile

• oneof fields need to be declared nullable
  code accessing those fields needs to be annotated or errors suppressed, e.g.

  public int OneInt {
    get { return HasOneInt ? (int) myOneofField_ : 0; } // Error CS8605 unboxing null value
    set {
      myOneofField_ = value;
      myOneofFieldCase_ = MyOneofFieldOneofCase.OneInt;
    }
  }

[jskeet]

@tonydnewell Sorry for not reviewing this for a while - is this now done and complete, ready for review and possible merging?

[tonydnewell]

@tonydnewell Sorry for not reviewing this for a while - is this now done and complete, ready for review and possible merging?

It is ready. However my contract has now ended so I'll only be able to fix any problems on an ad hoc basis.

[jskeet]

@tonydnewell Sorry for not reviewing this for a while - is this now done and complete, ready for review and possible merging?

It is ready. However my contract has now ended so I'll only be able to fix any problems on an ad hoc basis.

Righto - if I spot anything, I could quite possibly fix it myself instead. Will attempt to find headspace to review this soon. Thanks!

[jskeet]

(It's actually much shorter than I'd have expected, so it might not be too hard to review after all...)

[jskeet]

I think this looks mostly fine - but I don't see anything to handle the wrapper types for string and ByteString. I think those properties would need to be nullable as well.

@tonydnewell is that a change you think you'd be able to get to in spare time, or would you like me to try to get this merged and then do a fast-follow change for that?

[jskeet]

I don't think we need this line, as we're not using the file_name variable anywhere. (It doesn't do any harm though, so we could merge and then fix.)

[jskeet]

Nit: extra space at start (could fix after merging).

[Falco20019]

@tonydnewell Is there anything I can do to help get traction on this?

[jskeet]

I suspect the next step is mine - it's a matter of finding the time.

[Falco20019]

@JasonLunn I think we can close this one in favor of the extended one. If you want to keep it until the other is handled, I added close-keywords to my PR instead :)

[JasonLunn]

@JasonLunn I think we can close this one in favor of the extended one. If you want to keep it until the other is handled, I added close-keywords to my PR instead :)

Yes let's keep this one open until the other merges.

[JasonLunn]

Marking this blocked until we make a decision on #20242