Code Cleanup · Pull Request #26363 · dotnet/coreclr

ghost · 2019-08-25T23:04:18Z

All refactorings and code fixes were done by the IDE.

Test

…null-check

stephentoub · 2019-08-26T13:13:58Z

                culture: null);

-            return result != null ? (int)result : 0;
+            return (int?) result ?? 0;


These are not the same; the latter produces IL that involves nullable. Please revert this and all similar changes.

stephentoub · 2019-08-26T13:15:56Z


            IEqualityComparer<TKey>? comparer = _comparer;
-            uint hashCode = (uint)((comparer == null) ? key.GetHashCode() : comparer.GetHashCode(key));
+            uint hashCode = (uint)(comparer?.GetHashCode(key) ?? key.GetHashCode());


I don't think this is an improvement. The previous code was easy to understand: if comparer is null, do one thing, otherwise do something else. This takes longer to visually parse and understand. Please revert this.

stephentoub · 2019-08-26T13:17:19Z

        }
    }  // class Convert
-}  // namespace
+}  // namespace


Please revert this file.

stephentoub · 2019-08-26T13:18:50Z

                m_creator = creator;
                m_uniqueId = uniqueId;
-                m_level = creator != null ? creator.m_level + 1 : 0;
+                m_level = creator?.m_level + 1 ?? 0;


This is hard to understand. Please revert.

stephentoub · 2019-08-26T13:19:43Z

        {
            ManifestBuilder? manifest = null;
-            bool bNeedsManifest = source != null ? !source.SelfDescribingEvents : true;
+            bool bNeedsManifest = !source?.SelfDescribingEvents ?? true;


This is harder to understand. Please revert.

stephentoub · 2019-08-26T13:20:01Z

        public void AddBinary(string? value)
        {
-            DataCollector.ThreadInstance.AddBinary(value, value == null ? 0 : value.Length * 2);
+            DataCollector.ThreadInstance.AddBinary(value, value?.Length * 2 ?? 0);


This is harder to understand. Please revert.

stephentoub · 2019-08-26T13:20:12Z

-            return Compare(string1, offset1, string1 == null ? 0 : string1.Length - offset1,
-                           string2, offset2, string2 == null ? 0 : string2.Length - offset2, options);
+            return Compare(string1, offset1, string1?.Length - offset1 ?? 0,
+                           string2, offset2, string2?.Length - offset2 ?? 0, options);


These are harder to understand. Please revert.

stephentoub · 2019-08-26T13:21:05Z

        public void Add<T>(T value, IEqualityComparer<T>? comparer)
        {
-            Add(comparer != null ? comparer.GetHashCode(value) : (value?.GetHashCode() ?? 0));
+            Add(comparer?.GetHashCode(value) ?? (value?.GetHashCode() ?? 0));


This is harder to understand. Please revert.

stephentoub · 2019-08-26T13:22:39Z

                else
                {
-                    char ch = builder[builder.Length - 1];
+                    char ch = builder[^1];


Please revert all use of ^1 in this file. This file ends up getting compiled into some downlevel assemblies that do not have access to Index.

stephentoub · 2019-08-26T13:23:06Z

                else
                {
-                    if (!PathInternal.IsDirectorySeparator(builder[builder.Length - 1]) && !PathInternal.IsDirectorySeparator(path[0]))
+                    if (!(PathInternal.IsDirectorySeparator(builder[^1]) || PathInternal.IsDirectorySeparator(path[0])))


Please revert this.

stephentoub · 2019-08-26T13:23:53Z

            {
                uint result = Interop.Kernel32.GetLongPathNameW(
-                    ref inputBuilder.GetPinnableReference(terminate: true), ref outputBuilder.GetPinnableReference(), (uint)outputBuilder.Capacity);
+                    ref inputBuilder.GetPinnableReference(terminate: true), ref outputBuilder.GetPinnableReference(),


Why did you change this?

stephentoub · 2019-08-26T13:24:11Z

                return false;

-            char c = path[path.Length - 1];
+            char c = path[^1];


Please revert this file. Same reason as earlier.

stephentoub · 2019-08-26T13:24:35Z

-            if (!hasValue) return other == null;
-            if (other == null) return false;
-            return value.Equals(other);
+            return !hasValue ? other == null : other != null && value.Equals(other);


This is harder to understand. Please revert this.

stephentoub · 2019-08-26T13:27:33Z

-            return source != null ?
-                source.InternalRegister(callback, state, useSynchronizationContext ? SynchronizationContext.Current : null, useExecutionContext ? ExecutionContext.Capture() : null) :
-                default; // Nothing to do for tokens than can never reach the canceled state. Give back a dummy registration.
+            return source?.InternalRegister(callback, state, useSynchronizationContext ? SynchronizationContext.Current : null, useExecutionContext ? ExecutionContext.Capture() : null) ?? default; // Nothing to do for tokens than can never reach the canceled state. Give back a dummy registration.


Please put the default and everything after it back on its own line. This new formatting creates an incredibly long line.

stephentoub · 2019-08-26T13:38:55Z

            int len = threadsToPrint.Count;
            Console.WriteLine("getting info for {0} threads", len);
-            for (int i = 0; i < len; i++)
+            foreach (var t in threadsToPrint)


var => whatever type this actually is

… into null-check" This reverts commit 67cef41, reversing changes made to deda7bb.

This reverts commit deda7bb.

This reverts commit add7e53.

This reverts commit 96d11b4.

This reverts commit e710c78.

This reverts commit d12cef3.

This reverts commit df97df7.

This reverts commit b09e08e.

ghost · 2019-08-26T17:18:47Z

I see the issue; I am not using the version of C# this program is meant to be in. I am going to undo my changes and redo them

This reverts commit add7e53.

This reverts commit a1f19e5.

* adding ci legs to release 3.1 branch * removing regex and adding branches as regex was not working Remove unnecessary !s for [DoesNotReturn] (#26350) Remove unused test file introduced in #25948 (#26349) Fix BinaryReader.ReadChars for fragmented Streams (#26324) BinaryReader.ReadChars incorrectly read more than necessary from the underlying Stream when multi-byte characters straddled the read chunks. Fixes https://github.com/dotnet/corefx/issues/40455 Update ReadOnlyDictionary.cs Simplify null checks Update parse-hb-log.cs Update Nullable.cs Optimizations Use Expression Update Update PathHelper.Windows.cs Fix convert Update Nullable.cs Update StackFrameHelper.cs Revert "Merge branch 'null-check' of https://github.com/pi1024e/coreclr into null-check" This reverts commit 67cef41, reversing changes made to deda7bb. Revert "Update PathHelper.Windows.cs" This reverts commit deda7bb. Revert "Update" This reverts commit add7e53. Revert "Use Expression" This reverts commit 96d11b4. Revert "Revert "Use Expression"" This reverts commit e710c78. Revert "Revert "Revert "Use Expression""" This reverts commit d12cef3. Revert "Simplify null checks" This reverts commit df97df7. Revert "Update ReadOnlyDictionary.cs" This reverts commit b09e08e. Revert "Update" This reverts commit add7e53. Revert "Optimizations" This reverts commit a1f19e5. Update objectnative.cpp

…null-check

pi1024e and others added 13 commits August 25, 2019 14:12

Update ReadOnlyDictionary.cs

b09e08e

Simplify null checks

df97df7

Fix convert

a238d62

Update Nullable.cs

dc0ed0a

Update StackFrameHelper.cs

ff7d59d

Update parse-hb-log.cs

b345d43

Update Nullable.cs

4b86c73

Optimizations

a1f19e5

Use Expression

96d11b4

Update

add7e53

Merge pull request #1 from pi1024e/Test

efa6806

Test

Update PathHelper.Windows.cs

deda7bb

Merge branch 'null-check' of https://github.com/pi1024e/coreclr into …

67cef41

…null-check

jkotas requested a review from stephentoub August 26, 2019 12:22

stephentoub reviewed Aug 26, 2019

View reviewed changes

Comment thread src/tools/GCLogParser/parse-hb-log.cs

stephentoub reviewed Aug 26, 2019

View reviewed changes

jkotas mentioned this pull request Aug 26, 2019

Update ReadOnlyDictionary.cs [DO NOT MERGE] #26359

Closed

pi1024e added 8 commits August 26, 2019 13:13

Revert "Merge branch 'null-check' of https://github.com/pi1024e/coreclr…

60951ab

… into null-check" This reverts commit 67cef41, reversing changes made to deda7bb.

Revert "Update PathHelper.Windows.cs"

09dbbd0

This reverts commit deda7bb.

Revert "Update"

f34ee41

This reverts commit add7e53.

Revert "Use Expression"

e710c78

This reverts commit 96d11b4.

Revert "Revert "Use Expression""

d12cef3

This reverts commit e710c78.

Revert "Revert "Revert "Use Expression"""

6c94b0b

This reverts commit d12cef3.

Revert "Simplify null checks"

90fcfb0

This reverts commit df97df7.

Revert "Update ReadOnlyDictionary.cs"

a7eade7

This reverts commit b09e08e.

pi1024e added 2 commits August 26, 2019 13:19

Revert "Update"

1876be6

This reverts commit add7e53.

Revert "Optimizations"

99c0c10

This reverts commit a1f19e5.

ghost closed this Aug 26, 2019

Update objectnative.cpp

f8176dd

ghost reopened this Aug 26, 2019

Anipik and others added 2 commits August 26, 2019 13:27

Merge branch 'null-check' of https://github.com/pi1024e/coreclr into …

0dac9d0

…null-check

ghost closed this Aug 26, 2019

ghost deleted the null-check branch August 26, 2019 17:31

This pull request was closed.

Conversation

ghost commented Aug 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants