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

Nullable Reference Types, setup Attributes vs Constructor+IDisposable #1551

Closed
virzak opened this issue Oct 2, 2020 · 4 comments
Closed

Comments

@virzak
Copy link

virzak commented Oct 2, 2020

Trying to convert a benchmarking sample to be nullable by setting <Nullable>enable</Nullable>, and a warning shows up for this line:

warning CS8618: Non-nullable field 'data' is uninitialized. Consider declaring the field as nullable.

What's the strategy to deal with NRT for a case like this?

Could the xUnit's approach to setup and teardown be leveraged? Instead of attributes Constructor+IDisposable would be used.

@adamsitnik
Copy link
Member

Hi @virzak

Trying to convert a benchmarking sample to be nullable by setting

Why are you trying to do it? Our CI uses .NET Core 2.1 and we don't plan to use the Nullable feature in the Samples project (we want to keep samples as simple as possible to focus on BDN)

What's the strategy to deal with NRT for a case like this?

I don't know. Please ask a question on StackOverflow.

Instead of attributes Constructor+IDisposable would be used.

We decided to go with attributes a long time ago and unfortunately, we have to follow this strategy. Otherwise, it would be a big breaking change for our users.

There is nothing actionable on BDN side, so I am closing this issue.

Have a nice day,
Adam

@virzak
Copy link
Author

virzak commented Oct 5, 2020

@adamsitnik The project I'm working on uses the latest .Net along with latest features like nullable. Modifying your sample was an exercise in upgrading. Submitting a PR or messing up the CI wasn't the intent.

Since nullable will be enabled by default in future releases of .Net, this warning will eventually become a part of this repo.

Asking this question on github seems like an appropriate place. Here are issues 1 and 2 and docs on the same topic from EF Core team.

The obvious answer to my question would be to use ! as in:

private int[] data = null!;

Just was wondering if there's anything better.

@aelij
Copy link

aelij commented Jun 5, 2024

@adamsitnik Would it be possible to support the required property modifier?
Currently it causes a build error as the generated code doesn't use object initializers:

BenchmarkDotNet.Autogenerated.Runnable_2 instance = new BenchmarkDotNet.Autogenerated.Runnable_2(); // do NOT change name "instance" (used in SmartParamameter)
instance.Value = ...

To support required it would need to be:

BenchmarkDotNet.Autogenerated.Runnable_2 instance = new BenchmarkDotNet.Autogenerated.Runnable_2 { Value = ... };

@adamsitnik
Copy link
Member

adamsitnik commented Jun 5, 2024

Would it be possible to support the required property modifier?

Yes if you are willing to provide tests and implementation.

The template is located here:

https://github.com/dotnet/BenchmarkDotNet/blob/master/src/BenchmarkDotNet/Templates/BenchmarkType.txt#L6-L7

And the actual content is generated here:

.Replace("$ParamsContent$", GetParamsContent(benchmark))

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

3 participants