Code cleanup and formatting for System.Memory files. by ahsonkhan · Pull Request #17451 · dotnet/coreclr

ahsonkhan · 2018-04-06T03:38:04Z

I tried to limit the changes mainly to files shared between coreclr/corefx relevant to System.Memory.

Each set of changes is a separate commit.

The IDE00XX errors were fixed by VS, so all instances of the errors within the solution got fixed.

Related PR on the corefx side: dotnet/corefx#28875

cc @dotnet/corefxlab-contrib, @stephentoub, @jkotas

This should get mirrored - cc @Anipik, @safern

jkotas · 2018-04-06T04:24:09Z

src/mscorlib/src/System/Reflection/Emit/ModuleBuilder.cs

            ModuleBuilder refedModuleBuilder = refedModule as ModuleBuilder;

-            String strRefedModuleFileName = String.Empty;
+            String strRefedModuleFileName = string.Empty;


This file used String consistently before this change. It is using mix of string and String now. It does not look like an improvement.

(multiple places - not commenting on all of them)

It looks like VS didn't catch all instances. Will fix.

jkotas · 2018-04-06T04:32:41Z

src/mscorlib/shared/System/Lazy.cs

        {
            var spinWait = new SpinWait();
-            while (!ReferenceEquals(_state, null))
+            while (!(_state is null))


The other places that check _state are still using ReferenceEquals. Using is here while keeping the rest on the ReferenceEquals plan does not look like an improvement.

For comparing an object to a non-null object, is there any benefit of using ReferenceEquals over ==? If not, I can change the other calls as well.

== may use overriden operator Equals if there is one.

ReferenceEquals is explicit about your intent: that you want to compare references, and not use any overridden operators.

jkotas · 2018-04-06T04:33:47Z

src/mscorlib/shared/System/Lazy.cs

        {
            var spinWait = new SpinWait();
-            while (!ReferenceEquals(_state, null))
+            while (!(_state is null))


Why is this not != null like the other place?

VS defaulted to this as part of the "Null check can be simplified" fix. I will update to use !=

ahsonkhan · 2018-04-06T04:40:37Z

@dotnet-bot test Windows_NT x64 Checked corefx_baseline
@dotnet-bot test Ubuntu x64 Checked corefx_baseline

joshfree · 2018-04-06T05:55:23Z

As this change isn't required to ship 2.1 RC, please hold off on merging this until the master branch opens for 2.2 PRs later this month

ghost · 2018-04-06T14:49:46Z

src/mscorlib/shared/System/Memory.cs

 using System.Runtime.InteropServices;
-using EditorBrowsableAttribute = System.ComponentModel.EditorBrowsableAttribute;
-using EditorBrowsableState = System.ComponentModel.EditorBrowsableState;
+using System.ComponentModel;


I preferred it the original way. System.ComponentModel is not an obvious namespace to import into a file like Memory.cs - spelling out the types makes it clear why we're importing it.

…leanup

…17451) * Fix IDE0034 C# expression can be simplified (use of default) * Remove unnecessary using statements * IDE0012 C# Name can be simplified. String -> string * IDE0041 C# Null check can be simplified * Fix code formatting (limited to Span/System.Memory files). * Address PR feedback - IDE0012 Name can be simplified (String -> string) * Add back the necessary using statements and fix typo * Fix comment typo and add necessary using statements Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

…17451) * Fix IDE0034 C# expression can be simplified (use of default) * Remove unnecessary using statements * IDE0012 C# Name can be simplified. String -> string * IDE0041 C# Null check can be simplified * Fix code formatting (limited to Span/System.Memory files). * Address PR feedback - IDE0012 Name can be simplified (String -> string) * Add back the necessary using statements and fix typo * Fix comment typo and add necessary using statements Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>

…17451) * Fix IDE0034 C# expression can be simplified (use of default) * Remove unnecessary using statements * IDE0012 C# Name can be simplified. String -> string * IDE0041 C# Null check can be simplified * Fix code formatting (limited to Span/System.Memory files). * Address PR feedback - IDE0012 Name can be simplified (String -> string) * Add back the necessary using statements and fix typo * Fix comment typo and add necessary using statements Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>

…17451) * Fix IDE0034 C# expression can be simplified (use of default) * Remove unnecessary using statements * IDE0012 C# Name can be simplified. String -> string * IDE0041 C# Null check can be simplified * Fix code formatting (limited to Span/System.Memory files). * Address PR feedback - IDE0012 Name can be simplified (String -> string) * Add back the necessary using statements and fix typo * Fix comment typo and add necessary using statements Signed-off-by: dotnet-bot-corefx-mirror <dotnet-bot@microsoft.com>

ahsonkhan added 5 commits April 5, 2018 19:02

Fix IDE0034 C# expression can be simplified (use of default)

68e858d

Remove unnecessary using statements

6e3177d

IDE0012 C# Name can be simplified. String -> string

e161121

IDE0041 C# Null check can be simplified

f839163

Fix code formatting (limited to Span/System.Memory files).

be6920b

ahsonkhan added the area-System.Memory label Apr 6, 2018

ahsonkhan self-assigned this Apr 6, 2018

ahsonkhan mentioned this pull request Apr 6, 2018

Code cleanup and formatting for System.Memory src and test dotnet/corefx#28875

Merged

dotnet-bot added the 2 - In Progress label Apr 6, 2018

jkotas reviewed Apr 6, 2018

View reviewed changes

ahsonkhan added this to the 2.2.0 milestone Apr 6, 2018

ghost reviewed Apr 6, 2018

View reviewed changes

ahsonkhan added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 11, 2018

ahsonkhan added 3 commits May 21, 2018 12:09

Merge branch 'master' of https://github.com/dotnet/coreclr into CodeC…

0384464

…leanup

Address PR feedback - IDE0012 Name can be simplified (String -> string)

61370f3

Add back the necessary using statements and fix typo

695dc8d

ahsonkhan removed the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label May 21, 2018

Fix comment typo and add necessary using statements

72c2798

ahsonkhan merged commit f31097f into dotnet:master May 22, 2018

ahsonkhan deleted the CodeCleanup branch May 22, 2018 01:04

jkotas mentioned this pull request Jun 3, 2018

Fix blittable array instead of copying dotnet/corefx#30099

Merged

jkotas mentioned this pull request Jun 27, 2018

Moved SafeWaitHandle and cancellationToken to shared #18662

Merged

lewurm mentioned this pull request Feb 1, 2019

[2018-08] Bump corert mono/mono#12721

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Code cleanup and formatting for System.Memory files.#17451

Code cleanup and formatting for System.Memory files.#17451
ahsonkhan merged 9 commits intodotnet:masterfrom
ahsonkhan:CodeCleanup

ahsonkhan commented Apr 6, 2018 •

edited

Loading

Uh oh!

jkotas Apr 6, 2018

Uh oh!

ahsonkhan Apr 6, 2018

Uh oh!

jkotas Apr 6, 2018

Uh oh!

ahsonkhan Apr 6, 2018

Uh oh!

jkotas Apr 6, 2018

Uh oh!

jkotas Apr 6, 2018

Uh oh!

ahsonkhan Apr 6, 2018

Uh oh!

ahsonkhan commented Apr 6, 2018

Uh oh!

joshfree commented Apr 6, 2018

Uh oh!

ghost Apr 6, 2018 •

edited by ghost

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ahsonkhan commented Apr 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkotas Apr 6, 2018

Choose a reason for hiding this comment

Uh oh!

ahsonkhan Apr 6, 2018

Choose a reason for hiding this comment

Uh oh!

jkotas Apr 6, 2018

Choose a reason for hiding this comment

Uh oh!

ahsonkhan Apr 6, 2018

Choose a reason for hiding this comment

Uh oh!

jkotas Apr 6, 2018

Choose a reason for hiding this comment

Uh oh!

jkotas Apr 6, 2018

Choose a reason for hiding this comment

Uh oh!

ahsonkhan Apr 6, 2018

Choose a reason for hiding this comment

Uh oh!

ahsonkhan commented Apr 6, 2018

Uh oh!

joshfree commented Apr 6, 2018

Uh oh!

ghost Apr 6, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ahsonkhan commented Apr 6, 2018 •

edited

Loading

ghost Apr 6, 2018 •

edited by ghost

Loading