Skip to content

Nullable reference types support#129

Merged
andrewvk merged 86 commits intomasterfrom
feature/nullable_v3
Jan 18, 2021
Merged

Nullable reference types support#129
andrewvk merged 86 commits intomasterfrom
feature/nullable_v3

Conversation

@andrewvk
Copy link
Copy Markdown
Member

No description provided.

andrewvk and others added 10 commits August 30, 2020 19:31
# Conflicts:
#	Build/JetBrains.Annotations.cs
#	CodeJam.Experimental.Tests/UseCases/TempDataUseCases.cs
#	CodeJam.Main/Arithmetic/OperatorsFactory.cs
#	CodeJam.Main/Assertions/Code.cs
#	CodeJam.Main/Assertions/CodeExceptions.cs
#	CodeJam.Main/Assertions/DebugCode.generated.cs
#	CodeJam.Main/CodeJam.Main.csproj
#	CodeJam.Main/Collections/SuffixTree/SuffixTreeBase.cs
#	CodeJam.Main/Expressions/ExpressionExtensions.cs
#	CodeJam.Main/Reflection/MemberAccessor.cs
#	CodeJam.Main/Strings/StringExtensions.cs
#	CodeJam.Main/Threading/ParallelQueue.cs
#	CodeJam.sln.DotSettings
@andrewvk andrewvk requested review from NN--- and ig-sinicyn November 21, 2020 10:46
Comment thread Build/Build.csproj Outdated
@andrewvk
Copy link
Copy Markdown
Member Author

Feel free to apply changes from previous PR.

@andrewvk
Copy link
Copy Markdown
Member Author

One more interesting point. R# automatically removes its own CanBeNull markup when adding "?" to a type. @NN---

@NN---
Copy link
Copy Markdown
Member

NN--- commented Nov 25, 2020

I paid attention to that:)
As I said my concern is old ReSharper versions without good NRT support.

@NN---
Copy link
Copy Markdown
Member

NN--- commented Jan 11, 2021

@andrewvk So, do we agree that all ReSharper nullability attributes will be replaced with .NET nullability attributes ?

@NN---
Copy link
Copy Markdown
Member

NN--- commented Jan 17, 2021

Btw, IDictionary doesn't require "TKey : notnull" according to sources.
https://github.com/dotnet/runtime/blob/master/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/IDictionary.cs

It means adding this constraint is not correct.

@andrewvk
Copy link
Copy Markdown
Member Author

You mean that NUnit still doesn't have a NotNull attribute markup? Yeah, that's pretty weird. Of course, in this case it's worth adding your own variant.

@andrewvk
Copy link
Copy Markdown
Member Author

image

@NN---
Copy link
Copy Markdown
Member

NN--- commented Jan 17, 2021

Yeah. I tried to reproduce this in a separate project but everything compiled.
Probably it requires this constraint only for specific framework.
Currently everything compiles but not in appveyor:(
I’ll check how to improve it.

@andrewvk
Copy link
Copy Markdown
Member Author

Core 3.0 library
image
Core 5.0 library
image
Shall we try to hide it under a compiler directive?

@andrewvk
Copy link
Copy Markdown
Member Author

#if NETCOREAPP3_1 where TKey : notnull #endif
Build OK

@NN---
Copy link
Copy Markdown
Member

NN--- commented Jan 17, 2021

It is better to not introduce additional constraint if possble

@NN---
Copy link
Copy Markdown
Member

NN--- commented Jan 18, 2021

Fixed recursion and now the build works :)
@andrewvk Is there anything left in PR ?

@andrewvk andrewvk merged commit 297a838 into master Jan 18, 2021
@NN--- NN--- deleted the feature/nullable_v3 branch July 13, 2021 05:17
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.

4.0 roadmap

3 participants